1823284 - Widget minimization on windows exits DOM fullscreen. (was: Problem with Youtube full screen at version 111.0)

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[FireFoxy]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101 Firefox/111.0

Steps to reproduce:

Opened Youtube played any video with fullscreen , then used Win+D shortcut to return to desktop , after then returned back to the Firefox

Actual results:

Menu and address bar is hidden unless you move cursor

Expected results:

It should be as it was last time ( works at v110 btw )

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Audio/Video: Playback' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Alice0775 White]

This is reproduced not only in Youtube videos, but also in stand-alone videos.

Bad build:
Browser sizemode becomes window fullscreen instead of video fullscreen.
Taskbar is not hidden (This is an old bug and seems unrelated to this one.).

Good build:
video fullscreen is restored.
Taskbar is not hidden (this is an old bug).

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=baf3448d460ba11e7118acca260e9781f9d29259&tochange=0577733e69fa9efbcac58d9ad61995e2b72b7a66

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

:emilio, since you are the author of the regressor, bug 1809757, could you take a look? Also, could you set the severity field?

For more information, please visit auto_nag documentation.

Comment 4

[Emilio Cobos Álvarez [:emilio]]

So from the widget point of view this looks roughly as expected, in the sense that we're sending the right messages:

• The window is minimized, so we send an willexitfullscreen message, then exit fullscreen.
• Then it's restored, so we send an willenterfullscreen, then enter fullscreen.

It seems when we exit widget fullscreen we also exit DOM fullscreen, but we don't restore this.

So all-in-all I'd expect other platforms to behave like this as well.

Minimal STR:

• Go to https://commondatastorage.googleapis.com/gtv-videos-bucket/sample/BigBuckBunny.mp4
• Make video fullscreen.
• Press Windows+M to minimize window.
• Restore window.

We could go back to omitting some of those minimize / restore pairs (which is what the early-returns in fullscreen were doing), but that feels wrong on the widget side at least.

Comment 5

[Edgar Chen [:edgar]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)

It seems when we exit widget fullscreen we also exit DOM fullscreen,

Yes, when widget exits fullscreen, we also exit DOM fullscreen.

but we don't restore this.

I am not sure if we want to restore the DOM fullscreen as we could not tell the willexitfullscreen/willenterfullscreen message is caused by minimizing/restoring window or user press F11 explicitly to exit/enter browser fullscreen. Furthermore, Chrome doesn't exit DOM fullscreen while window is minimized.

Comment 7

[Ray Kraesig [:rkraesig]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)

So from the widget point of view this looks roughly as expected, in the sense that we're sending the right messages:

• The window is minimized, so we send an willexitfullscreen message, then exit fullscreen.
• Then it's restored, so we send an willenterfullscreen, then enter fullscreen.

(In reply to Edgar Chen [:edgar] from comment #5)

Furthermore, Chrome doesn't exit DOM fullscreen while window is minimized.

I think I agree with Chrome's behavior here. As far as the content is concerned — and possibly even the chrome, outside of TaskbarConcealer — minimizing shouldn't affect the fullscreen state.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)

So all-in-all I'd expect other platforms to behave like this as well.

And indeed (as seen in the recently-merged duplicate bug), this also happens on Gtk. :emilio, should this be DOM: Core & HTML?

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit auto_nag documentation.

Comment 9

[Simon R]

To carry over information from the now-duplicate bug, and post some new findings:

My read here is that there seem to be two seperate bugs/changesets causing the same regression at different times on different platforms.

on LINUX, mozregression yielded:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=81c9cc81146b08ed739c7218b206d4bcfed1d9b3&tochange=bf01dc27a9f3cad7d59ec3c65cd007b1338ba474
as the culprit. the issue seems to have existed there for a long time. I know emilio said he doesnt think its the cause but that was the conclusion of my tests with mozregression.

on WINDOWS, mozregression yielded:
https://hg.mozilla.org/integration/autoland/rev/0577733e69fa9efbcac58d9ad61995e2b72b7a66
as the cause, and the regression is much more recent.

I'd confirm this by building without those changesets, but I cant for the life of me get the bootstrap script to work right now so that'll have to wait.
(as an aside, sorry for the dupe, but i literally seached full screen and this bug didnt show up :/)

Comment 10

[Edgar Chen [:edgar]]

The communication between the widget and the DOM side is through the FullscreenWillChange and FullscreenChanged callbacks. The DOM side uses these callbacks to synchronize its fullscreen state with the widget. This is necessary to handle cases where the user directly toggles the fullscreen state of the widget, for example, by using a keyboard shortcut for "Toggle fullscreen mode" defined in the platform settings on Linux.

However, from the DOM side, it is not possible to determine whether the callback was triggered by the user explicitly toggling the fullscreen state or by minimizing/restoring the window, so it may be more appropriate to handle this in the widget and possibly not notify the FullscreenWillChange and FullscreenChanged callbacks when the window is being minimized/restored. I will check whether that might cause any side effects.

Comment 11

[Emilio Cobos Álvarez [:emilio]]

So on Windows, Android, and GTK at least, FullscreenWillChange / FullscreenChanged seem rather useless as they're always basically synchronous around the SizeModeChanged.

We could pass the old sizemode to SizeModeChanged quite easily now I believe, and the DOM could avoid exiting fullscreen in from/to-minimized changes.

Edgar, would that not work for any reason I'm missing? Maybe we should tweak the cocoa notifications to behave like other platforms and remove some of the complexity here.

Comment 12

[Edgar Chen [:edgar]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #11)

Edgar, would that not work for any reason I'm missing? Maybe we should tweak the cocoa notifications to behave like other platforms and remove some of the complexity here.

Ah, okay. I think that should also work. Thanks! The FullscreenWillChange for native fullscreen on Cocoa happens at a different timing which is determined by the OS, but I think it should be fine to delay it a bit until we handle SizeModeChanged. I will try this on Cocoa first.

Comment 13

[Edgar Chen [:edgar]]

Cocoa behaves differently when minimizing in fullscreen. It exits fullscreen to a normal window and then minimizes (previously it did not allow minimizing while in fullscreen), but it notifies SizeModeChanged with nsSizeMode_Minimized directly. When restoring, it does not enter fullscreen again, and notifies SizeModeChanged with nsSizeMode_Normal.

DOM could detect that and sync DOM fullscreen state while get the nsSizeMode_Normal, but which means DOM will still be in fullscreen state when minimizing. Or maybe Cocoa could notify nsSizeMode_Normal when it returns to a normal window before minimizing, so that the DOM can sync the fullscreen state with the widget earlier.

Bradley, is there any specific reason for not notifying nsSizeMode_Normal in this case, or is it possible to have nsSizeMode_Normal notified before minimizing?

Comment 14

[Brad Werth [:bradwerth]]

(In reply to Edgar Chen [:edgar] from comment #13)

Cocoa behaves differently when minimizing in fullscreen. It exits fullscreen to a normal window and then minimizes (previously it did not allow minimizing while in fullscreen), but it notifies SizeModeChanged with nsSizeMode_Minimized directly. When restoring, it does not enter fullscreen again, and notifies SizeModeChanged with nsSizeMode_Normal.

DOM could detect that and sync DOM fullscreen state while get the nsSizeMode_Normal, but which means DOM will still be in fullscreen state when minimizing. Or maybe Cocoa could notify nsSizeMode_Normal when it returns to a normal window before minimizing, so that the DOM can sync the fullscreen state with the widget earlier.

Bradley, is there any specific reason for not notifying nsSizeMode_Normal in this case, or is it possible to have nsSizeMode_Normal notified before minimizing?

I made Cocoa go straight to nsSizeMode_Minimized from fullscreen because I was working under the assumption that one JS call = one sizemodechange event. But we could instead adopt the idea that leaving fullscreen always generates a nsSizeMode_Normal and then maybe another sizemodechange if that's the destination. There's only one test that checks this, browser_fullscreen_from_minimize.js and it could be changed to expect two sizemode events for that transition.

Comment 15

[Suhaib Mujahid [:suhaib]]

:echen is there any update here?

Comment 16

[Edgar Chen [:edgar]]

Attachment: Bug 1823284 - Avoid waiting for window resizing if it is already at expected dimensions; r?smaug!

Comment 17

[Edgar Chen [:edgar]]

Attachment: Bug 1823284 - [Cocoa] Allow Dispatching sizemodechange event for intermediate transition; r?bradwerth!

Depends on D175213

Comment 18

[Edgar Chen [:edgar]]

Attachment: Bug 1823284 - Use `SizeModeChanged` notification to handle fullscreen change; r?smaug!,bradwerth!

Depends on D175214

Comment 19

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/jobs?repo=try&revision=54da00a428af39a710ee916044bad54f6ca01fca

Comment 20

[Brad Werth [:bradwerth]]

Attachment: Bug 1823284 - [Cocoa] Make nsCocoaWindow restore minimized window to former state.

This makes Cocoa windows behave similarly to other widget windows in that
restoring a minimized window will return it to its pre-minimized state. It
does this by inserting an intermediate transition to normal/windowed mode,
but that transition does not fire a sizemodechange event.

Comment 23

[Phabricator Automation]

Comment on attachment 9328406 [details]
Bug 1823284 - [Cocoa] Make nsCocoaWindow restore minimized window to former state.

Revision D175384 was moved to bug 1828706. Setting attachment 9328406 [details] to obsolete.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/65747f283949
Avoid waiting for window resizing if it is already at expected dimensions; r=smaug
https://hg.mozilla.org/integration/autoland/rev/54bce7b5d558
Use `SizeModeChanged` notification to handle fullscreen change; r=geckoview-reviewers,rkraesig,stransky,bradwerth,smaug,m_kato

Comment 26

[Norisz Fay [:noriszfay]]

Backed out for causing failures on element-request-fullscreen-timing.html, test_fullscreen-api.html

Backout link

Push with failures - mochitest
Failure log - mochitest

Push with failures - wpt
Failure log - wpt

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a62bae7b4710
Avoid waiting for window resizing if it is already at expected dimensions; r=smaug
https://hg.mozilla.org/integration/autoland/rev/84f653809273
Use `SizeModeChanged` notification to handle fullscreen change; r=geckoview-reviewers,rkraesig,stransky,bradwerth,smaug,m_kato

Comment 28

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/a62bae7b4710
https://hg.mozilla.org/mozilla-central/rev/84f653809273

Comment 29

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:edgar, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox113 to wontfix.

For more information, please visit auto_nag documentation.

Comment 32

[Edgar Chen [:edgar]]

Comment on attachment 9328089 [details]
Bug 1823284 - Use SizeModeChanged notification to handle fullscreen change; r?smaug!,bradwerth!

Beta/Release Uplift Approval Request

• User impact if declined: Document exits fullscreen after minimizing the window.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): The patch doesn't involve many changes (mostly code removal), and there are many tests for fullscreen. However, since it needs to tweak each platform, I'm hesitant to set to low.
• String changes made/needed: None
• Is Android affected?: No

Comment 33

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9328087 [details]
Bug 1823284 - Avoid waiting for window resizing if it is already at expected dimensions; r?smaug!

Approved for 113.0b9.

Comment 34

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/bd7382a111cd
https://hg.mozilla.org/releases/mozilla-beta/rev/43e4aa86fa0b

Comment 35

[Hani Yacoub, Desktop QA]

Verified as fixed on Windows 10 x64 and on Ubuntu 20.04 x64.

Comment 36

[Intermittent Failures Robot]

8 failures in 3177 pushes (0.003 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 6
• mozilla-central: 2

Platform and build breakdown:

• macosx1015-64-qr: 4
  • opt: 4
• macosx1015-64-shippable-qr: 4
  • opt: 4

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1823284&startday=2023-04-24&endday=2023-04-30&tree=all