In requestFullscreen(), handle the pending element moving #87
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes #33. Drive-by: capitalization in lt attributes to match dfn.
|
A test that already tests this was added in web-platform-tests/wpt#4250: Firefox Nightly already passes this test. It was an oversight that this test was included before the spec change, I've gone through the PR and checked for other things not yet reflected in the spec, but can't spot anything. |
annevk
approved these changes
May 16, 2017
|
Merged, but can you confirm if this will work for Gecko, @upsuper? Maybe the tests doesn't exercise some minor difference between the spec and what Gecko does. |
|
There is an issue here that, it seems when this happens, the browser window would end up being in fullscreen state while the content is not? It doesn't sound like a helpful state, but this is probably a separate issue. Anyway, I think it works for Gecko, and I think except having the issue above, Gecko should have been doing something like this now, but I'm not completely sure. |
|
Yeah, that is a problem, and IIRC you have a fix for it in Gecko? I've filed #88 but probably won't get back to it myself anytime soon since it's such a corner case. |
|
Gecko doesn't have this problem. The algorithm is something like, when we are trying to apply fullscreen state to pending document from window change, if there is nothing successfully gets fullscreened eventually, we revert the window state. I don't know if something like this is specable... |
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this pull request
Jun 20, 2017
… the spec" This relands https://codereview.chromium.org/2573773002/, which was reverted in https://codereview.chromium.org/2654083006/. Relevant spec changes since December 2016: * whatwg/fullscreen#68 * whatwg/fullscreen#72 * whatwg/fullscreen#85 * whatwg/fullscreen#87 * whatwg/fullscreen#90 * whatwg/fullscreen#92 Updated tests for whatwg/fullscreen#92: * document-exit-fullscreen-nested-manual.html asserts that fullscreenElement changes are not synchronous. * document-exit-fullscreen-timing-manual.html and element-request-fullscreen-timing-manual.html assert that fullscreenElement changes before the resize event. (They still fail in content_shell because there is no resize, but pass in chromium if run manually.) * element-request-fullscreen-and-exit-iframe-manual.html asserts that the order of "run the fullscreen steps" (firing events) is now frame tree order. Bug: 402376 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I9c01b237ecfd7d74b28e3dbafcacdefe43416cdf Reviewed-on: https://chromium-review.googlesource.com/521162 Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Reviewed-by: John Mellor <johnme@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: Emil A Eklund <eae@chromium.org> Cr-Commit-Position: refs/heads/master@{#480702} WPT-Export-Revision: 26d298d134b1407ffb035481dea00e95e66a4de9
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this pull request
Jun 20, 2017
… the spec" This relands https://codereview.chromium.org/2573773002/, which was reverted in https://codereview.chromium.org/2654083006/. Relevant spec changes since December 2016: * whatwg/fullscreen#68 * whatwg/fullscreen#72 * whatwg/fullscreen#85 * whatwg/fullscreen#87 * whatwg/fullscreen#90 * whatwg/fullscreen#92 Updated tests for whatwg/fullscreen#92: * document-exit-fullscreen-nested-manual.html asserts that fullscreenElement changes are not synchronous. * document-exit-fullscreen-timing-manual.html and element-request-fullscreen-timing-manual.html assert that fullscreenElement changes before the resize event. (They still fail in content_shell because there is no resize, but pass in chromium if run manually.) * element-request-fullscreen-and-exit-iframe-manual.html asserts that the order of "run the fullscreen steps" (firing events) is now frame tree order. Bug: 402376 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I9c01b237ecfd7d74b28e3dbafcacdefe43416cdf Reviewed-on: https://chromium-review.googlesource.com/521162 Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Reviewed-by: John Mellor <johnme@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: Emil A Eklund <eae@chromium.org> Cr-Commit-Position: refs/heads/master@{#480702} WPT-Export-Revision: 26d298d134b1407ffb035481dea00e95e66a4de9
MXEBot
pushed a commit
to mirror/chromium
that referenced
this pull request
Jun 21, 2017
… the spec" This relands https://codereview.chromium.org/2573773002/, which was reverted in https://codereview.chromium.org/2654083006/. Relevant spec changes since December 2016: * whatwg/fullscreen#68 * whatwg/fullscreen#72 * whatwg/fullscreen#85 * whatwg/fullscreen#87 * whatwg/fullscreen#90 * whatwg/fullscreen#92 Updated tests for whatwg/fullscreen#92: * document-exit-fullscreen-nested-manual.html asserts that fullscreenElement changes are not synchronous. * document-exit-fullscreen-timing-manual.html and element-request-fullscreen-timing-manual.html assert that fullscreenElement changes before the resize event. (They still fail in content_shell because there is no resize, but pass in chromium if run manually.) * element-request-fullscreen-and-exit-iframe-manual.html asserts that the order of "run the fullscreen steps" (firing events) is now frame tree order. Bug: 402376 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I9c01b237ecfd7d74b28e3dbafcacdefe43416cdf Reviewed-on: https://chromium-review.googlesource.com/521162 Commit-Queue: Philip Jägenstedt <foolip@chromium.org> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Reviewed-by: John Mellor <johnme@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: Emil A Eklund <eae@chromium.org> Cr-Commit-Position: refs/heads/master@{#480862}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #33.
Drive-by: capitalization in lt attributes to match dfn.
Preview | Diff