1185775 - The transition for entering and exiting fullscreen is too long

Status: RESOLVED FIXED

(Core :: Widget, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

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.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

Comment 2

[Xidorn Quan [:xidorn] UTC+11]

Also need approval from security guys.

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

(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

[Jet Villegas (inactive)]

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.

Comment 5

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Jared, do you have a build with this?
Thanks!

Also, I'm not sure to what degree Jets comment changes anything here...

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

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

Comment 7

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

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.

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

(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.

Comment 9

[Daniel Veditz [:dveditz]]

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?

Comment 10

[Xidorn Quan [:xidorn] UTC+11]

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).

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

Based on comment #10 I think we can push forward with the change that this patch is making.

Comment 12

[Bernardo Rittmeyer [:rittme]]

Comment on attachment 8636318 [details] [diff] [review]
Patch

Review of attachment 8636318 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me!

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

Xidorn, are we good to land this then?

Comment 14

[Xidorn Quan [:xidorn] UTC+11]

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.

Comment 15

[Daniel Veditz [:dveditz]]

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.

Comment 16

[Jared Wein [:jaws] (please needinfo? me)]

Thanks!

Comment 17

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/df33f085e1a6

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/df33f085e1a6

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Backed out for making bug 1181446 basically permafail on Win7.
https://hg.mozilla.org/integration/mozilla-inbound/rev/b92398507d91

Comment 20

[Ryan VanderMeulen [:RyanVM]]

And Win8.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/b92398507d91

Comment 22

[Jared Wein [:jaws] (please needinfo? me)]

checkin-needed now that bug 1181446 is fixed.

Comment 23

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5010f1531d25

Comment 24

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5010f1531d25