Closed Bug 1185775 Opened 5 years ago Closed 5 years ago
The transition for entering and exiting fullscreen is too long
This is a different issue than bug 1185208. The current settings of "400 400" for both entering and exiting fullscreen are too long. This is especially noticeable if a video has already begun playing before fullscreen is entered, as about 1 second of frames are hidden from the screen. We should cut this number in half, as it will make it feel snappier as well as reduce the amount of frames not shown on the screen.
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Attachment #8636318 - Flags: ui-review?(philipp)
Also need approval from security guys.
(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #2) > Also need approval from security guys. I'm not sure why, with the transition we didn't remove the permission dialog from the top of the window.
Note that we're also removing the mouse-click to approve the fullscreen action. After that lands, we should look at this again to see if the delay is still an issue. The visible transition was the security trade-off that allows us to remove the 'confirm' click.
Jared, do you have a build with this? Thanks! Also, I'm not sure to what degree Jets comment changes anything here...
You can test this out by going to about:config and changing full-screen-api.transition-duration.enter and full-screen-api.transition-duration.leave to "200 200" for both values. The first number is the fade-to-black, and the second number is the "fade-to-video/fade-to-webpage". You can test this on http://camendesign.com/code/video_for_everybody/test.html
Flags: needinfo?(jaws) → needinfo?(philipp)
Comment on attachment 8636318 [details] [diff] [review] Patch Thanks, that definitely looks better! Related: the transition is broken on Windows hidpi screens. I filed bug 1186384 about it.
Attachment #8636318 - Flags: ui-review?(philipp) → ui-review+
Attachment #8636318 - Flags: review?(bernardo)
(In reply to Jet Villegas (:jet) from comment #4) > Note that we're also removing the mouse-click to approve the fullscreen > action. After that lands, we should look at this again to see if the delay > is still an issue. The visible transition was the security trade-off that > allows us to remove the 'confirm' click. So when going fullscreen, there won't be a "allow"/"deny" dialog at the top anymore? Will there be some indicator that "example.com is now fullscreen"? The transition is still very noticeable, just not "slow" anymore. I still think we should do this.
How does changing this transition time interact with the other planned changes for full-screen? Are there builds with the new clickless announcement? In some of the mock-ups they shortened the time the announcement stayed on screen to account for the transition. If you're shortening the transition are you lengthening the announcement visibility?
I don't recall we have planned to shorten the announcement time to account for the transition. The announcement currently stays on screen for 3s, and I'm going to keep this time, and make it appear after the transition finishes as designed (currently the 3s starts since the fade-back starts).
Based on comment #10 I think we can push forward with the change that this patch is making.
Comment on attachment 8636318 [details] [diff] [review] Patch Review of attachment 8636318 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me!
Attachment #8636318 - Flags: review?(bernardo) → review+
Xidorn, are we good to land this then?
This change looks good to me, but I still hope to wait for reply from dveditz for security consideration. But I guess it should be fine.
I'm provisionally OK with this change, but I agree w/Jet in comment 4 that after the rest of the planned changes land we need to look at the effect as a whole and make sure we're happy or if we need to tweak it.
Backed out for making bug 1181446 basically permafail on Win7. https://hg.mozilla.org/integration/mozilla-inbound/rev/b92398507d91
Merge of backout: https://hg.mozilla.org/mozilla-central/rev/b92398507d91
You need to log in before you can comment on or make changes to this bug.