Removing steps on fullscreen element leave a dangling fullscreen flag #217
Comments
|
@foolip, do you by chance know the history of "Step 7.2" (see Chrome's implementation) and why it's no longer in the spec? WebKit will follow Chrome's behavior as otherwise it affects YouTube, but it would be good to figure this out as, right now, the we both don't follow the spec. |
…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
…gi?id=253121 REGRESSION(257542@main): Video is misaligned on YouTube site's PiP player 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
|
Sorry @nt1m @marcoscaceres, I missed this back when it was filed. I'll go through all open issues to see what else I've neglected. Step 7.2 in Chromium comes from https://source.chromium.org/chromium/chromium/src/+/14eeb1d5b6369a4fefa57973bf6b646469f72e35 by @dtapuska. It was never landed in the spec, but it was in #128, where I argued that we shouldn't have that step. I don't remember the details, but in the end we landed the PR without that step, with #133 filed as a follow-up. Since that step is still what shipped, I think adding it to the spec is the correct first step for sure. |
If you have a fullscreen element, then move it to a different parent, it triggers the removing steps which does at step 3:
But in the exit fullscreen algorithm, the fullscreen flag is only removed at step 14.3. This is problematic, because that step runs in parallel, so step 3.3 of the removing steps which removes the fullscreen element from the top layer will run before.
That means we end up stopping the exit fullscreen at step 11, so we end up never removing the fullscreen flag, which is problematic if the removed element is re-appended in the document.
Chromium handles it specifically at step 7. of exit fullscreen, by invoking the "unfullscreen doc's fullscreen element" as a step 7.2.
The text was updated successfully, but these errors were encountered: