REGRESSION(257542@main): Video is misaligned on YouTube site's PiP pl…

…ayer after transitioning from full screen https://bugs.webkit.org/show_bug.cgi?id=253121 rdar://105713729 Reviewed by Ryosuke Niwa. There is a bug with the fullscreen spec that leaves a dangling fullscreen flag when moving elements: whatwg/fullscreen#217 This causes fullscreen styles to unintentionally apply on the YouTube site even though the player element which has moved in the DOM tree, has exited fullscreen. To fix this, we follow Chromium's pattern of running an extra "unfullscreen element" step in the synchronous exit fullscreen steps when the element to be exited is disconnected. * LayoutTests/imported/w3c/web-platform-tests/fullscreen/model/move-fullscreen-element-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/fullscreen/model/move-fullscreen-element.html: Added. * Source/WebCore/dom/FullscreenManager.cpp: (WebCore::FullscreenManager::exitFullscreen): Canonical link: https://commits.webkit.org/260985@main
nt1m · Mar 1, 2023 · d8b3533 · d8b3533
1 parent f022496
commit d8b3533
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 1 deletion.
diff --git a/...sts/imported/w3c/web-platform-tests/fullscreen/model/move-fullscreen-element-expected.txt b/...sts/imported/w3c/web-platform-tests/fullscreen/model/move-fullscreen-element-expected.txt
@@ -0,0 +1,4 @@
+Fullscreen element
+
+PASS Moving the fullscreen element should not leave the fullscreen flag
+
diff --git a/LayoutTests/imported/w3c/web-platform-tests/fullscreen/model/move-fullscreen-element.html b/LayoutTests/imported/w3c/web-platform-tests/fullscreen/model/move-fullscreen-element.html
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+<title>Moving the fullscreen element should not leave the fullscreen flag</title>
+<link rel="author" title="Tim Nguyen" href="https://github.com/nt1m">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+<script src="../trusted-click.js"></script>
+
+<div id="fullscreen-element">Fullscreen element</div>
+
+<div id="new-parent"></div>
+
+<script>
+    promise_test(async () => {
+        const fullscreenElement = document.getElementById("fullscreen-element");
+        await trusted_request(fullscreenElement);
+        assert_true(fullscreenElement.matches(":fullscreen"), "Element has fullscreen flag");
+        assert_equals(document.fullscreenElement, fullscreenElement, "Element is fullscreen element");
+        document.getElementById("new-parent").appendChild(fullscreenElement);
+        assert_false(fullscreenElement.matches(":fullscreen"), "Element no longer has fullscreen flag after being moved");
+        assert_false(!!document.fullscreenElement, "There is no more fullscreen element, since fullscreen flag was removed");
+        await fullScreenChange();
+        assert_false(fullscreenElement.matches(":fullscreen"), "Element no longer has fullscreen flag after fullscreen change event");
+        assert_false(!!document.fullscreenElement, "There is no more fullscreen element, since fullscreen flag was removed");
+    });
+</script>
+</html>
diff --git a/Source/WebCore/dom/FullscreenManager.cpp b/Source/WebCore/dom/FullscreenManager.cpp
@@ -334,8 +334,11 @@ void FullscreenManager::exitFullscreen(RefPtr<DeferredPromise>&& promise)
     }
 
     auto element = exitingDocument->fullscreenManager().fullscreenElement();
-    if (element && !element->isConnected())
+    if (element && !element->isConnected()) {
         addDocumentToFullscreenChangeEventQueue(exitingDocument);
+        element->setFullscreenFlag(false);
+        element->removeFromTopLayer();
+    }
 
     m_pendingExitFullscreen = true;