Skip to content

Fullscreen/unfullscreen ASAP; fire events at animation frame timing#92

Merged
foolip merged 1 commit into
masterfrom
fullscreen-asap
Jun 15, 2017
Merged

Fullscreen/unfullscreen ASAP; fire events at animation frame timing#92
foolip merged 1 commit into
masterfrom
fullscreen-asap

Conversation

@foolip

@foolip foolip commented May 31, 2017 •
edited by pr-preview Bot
Loading

Copy link
Copy Markdown
Member

This means that changes to document.fullscreenElement and other state
will be observable as soon as the resize itself is (e.g. via
window.innerWidth) and before resize or scroll events are fired.

The fullscreenchange event is still delayed to animation frame timing.

This also includes a slight change when /resize/ is true in "exit
fullscreen". By changing /doc/ to /topLevelDoc/ in this case, we can
make sure that we always fully unfullscreen all documents in this case.
This makes a "fully exit fullscreen" corner case unnecessary.

Fixes #74.

Preview | Diff

@foolip foolip mentioned this pull request May 31, 2017
Closed
@foolip

foolip commented May 31, 2017

Copy link
Copy Markdown
Member Author

This depends on whatwg/html#2627. @upsuper, would you mind reviewing both? I'm fine with not making animation frame tasks a generic concept if you think that's better, it wouldn't affect the important changes here.

@foolip foolip mentioned this pull request Jun 1, 2017
Closed
@foolip

foolip commented Jun 7, 2017

Copy link
Copy Markdown
Member Author

Ping @upsuper.

@upsuper

upsuper commented Jun 7, 2017

Copy link
Copy Markdown
Member

Sorry for the delay. Please give me another day. I'll try to review it tomorrow.

@upsuper

upsuper commented Jun 7, 2017

Copy link
Copy Markdown
Member

And yeah, I'd probably prefer we don't make animation frame tasks a generic concept. Having each thing in its own queue makes order of them clearer, and would also allow us to specify the order if we need / want, at least for this kind of things which can potentially affect rendering.

@foolip

foolip commented Jun 14, 2017

Copy link
Copy Markdown
Member Author

Thanks for the review, @upsuper. I've now pushed an update that defines the "run the fullscreen rendering steps" already references by HTML, which means this doesn't depend on any HTML changes.

The implementation I hope to land in Blink is more similar to the "animation frame tasks" model I had, but currently it is indistinguishable. I might want to revisit whatwg/html#2627 later, but it's not important today.

Care to take another look? If it's OK with you, I plan to write the tests for this in https://chromium-review.googlesource.com/521162, so you wouldn't need to do that review.

@foolip
Fullscreen/unfullscreen ASAP; fire events at animation frame timing
a62b790 
This means that changes to document.fullscreenElement and other state
will be observable as soon as the resize itself is (e.g. via
window.innerWidth) and before resize or scroll events are fired.

The fullscreenchange event is still delayed to animation frame timing.

This also includes a slight change when /resize/ is true in "exit
fullscreen". By changing /doc/ to /topLevelDoc/ in this case, we can
make sure that we always fully unfullscreen all documents in this case.
This makes a "fully exit fullscreen" corner case unnecessary.

Fixes #74.
@foolip foolip force-pushed the fullscreen-asap branch from 2ab6bad to a62b790 Compare June 15, 2017 08:49
@foolip foolip changed the title Fullscreen/unfullscreen ASAP; fire events in animation frame tasks git showFullscreen/unfullscreen ASAP; fire events in animation frame tasks Jun 15, 2017
@foolip foolip changed the title git showFullscreen/unfullscreen ASAP; fire events in animation frame tasks Fullscreen/unfullscreen ASAP; fire events at animation frame timing Jun 15, 2017
@foolip foolip merged commit 4208e5a into master Jun 15, 2017
@foolip foolip deleted the fullscreen-asap branch June 15, 2017 08:49
@foolip

foolip commented Jun 15, 2017

Copy link
Copy Markdown
Member Author

To repeat, tests will be added in https://chromium-review.googlesource.com/521162

foolip added a commit to whatwg/html that referenced this pull request Jun 15, 2017
@foolip
Link to "run the fullscreen rendering steps" in Fullscreen
c503df8 
Introduced in whatwg/fullscreen#92.

Drive-by: re-wrap lines to 100 columns.
@foolip foolip mentioned this pull request Jun 15, 2017
Merged
foolip added a commit to whatwg/html that referenced this pull request Jun 15, 2017
@foolip
Link to "run the fullscreen rendering steps" in Fullscreen
ca65ca4 
Introduced in whatwg/fullscreen#92.

Fixes the fullscreen part of #707.

Drive-by: re-wrap lines to 100 columns.
@chromium-wpt-export-bot chromium-wpt-export-bot mentioned this pull request Jun 20, 2017
Merged
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 20, 2017
@foolip @chromium-wpt-export-bot
Reland "Sync requestFullscreen() and exitFullscreen() algorithms with…
8061570 
… 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
@foolip @chromium-wpt-export-bot
Reland "Sync requestFullscreen() and exitFullscreen() algorithms with…
9c9f267 
… 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
@foolip
Reland "Sync requestFullscreen() and exitFullscreen() algorithms with…
81e33d3 
… 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}
@foolip foolip mentioned this pull request Feb 1, 2018
Closed
@foolip foolip mentioned this pull request Aug 29, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@upsuper upsuper upsuper approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@foolip @upsuper