The transition for entering and exiting fullscreen is too long

RESOLVED FIXED in Firefox 42

Status

()

Core
Widget
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jaws, Assigned: jaws)

Tracking

unspecified
mozilla42
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(1 attachment)

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.
Created attachment 8636318 [details] [diff] [review]
Patch
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Attachment #8636318 - Flags: ui-review?(philipp)
Also need approval from security guys.
Flags: needinfo?(dveditz)
(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.

Comment 4

3 years ago
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...
Flags: needinfo?(jaws)
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.
Flags: needinfo?(philipp)
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?
Flags: needinfo?(quanxunzhen)
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.
Flags: needinfo?(quanxunzhen)
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.
Flags: needinfo?(dveditz)
Thanks!
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/df33f085e1a6
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Backed out for making bug 1181446 basically permafail on Win7.
https://hg.mozilla.org/integration/mozilla-inbound/rev/b92398507d91
Status: RESOLVED → REOPENED
status-firefox42: fixed → ---
Resolution: FIXED → ---
Target Milestone: mozilla42 → ---
And Win8.
checkin-needed now that bug 1181446 is fixed.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5010f1531d25
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.