1620341 - Fission window stays full screen after closing embedded YouTube video while full screen

Status: VERIFIED FIXED

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

Description

[Chris Peterson [:cpeterson]]

Steps to reproduce

1. In a Fission window, open multiple tabs.
2. In one tab, load https://wpsmackdown.com/embed-youtube-video-wordpress/
3. Search the page for "If you’re in a hurry, watch our YouTube video embed tutorial:"
4. Play the embedded YouTube video called "YouTube Video Embed in WordPress".
5. Click the YouTube player's full screen button.
6. While the video is playing in full screen, press Ctrl+W to close the video tab.

Expected results

The video tab should close and the browser window should return to non-full screen mode.

Actual results

The video tab closes, but the browser window stays full screen. The window's other tabs are full screen. You can't see the address bar and some keyboard shortcuts no longer respond, such as opening the File menu.

This full screen bug does not happen for first-party videos on youtube.com or in non-Fission windows.

I'm testing on Windows 10.

Comment 1

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

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Chris Peterson [:cpeterson]]

I can reproduce this bug on both Windows 10 (closing the full screen YouTube tab with Ctrl+W) and macOS 10.15 (closing with Cmd+W). Nika says she can't reproduce on Linux.

Comment 3

[Adam Stevenson [:adamopenweb]]

Hi Chris, do you have steps for us to enable a Fission window?

Comment 4

[Chris Peterson [:cpeterson]]

(In reply to Adam Stevenson [:adamopenweb] from comment #3)

Hi Chris, do you have steps for us to enable a Fission window?

Set pref fission.autostart = true in Nightly and restart Nightly. When Fission is enabled, you can open a non-Fission window for comparison using the "File > New Non-Fission Window" menu item.

Comment 5

[Alastor Wu [:alwu]]

It looks like an issue for Fullscreen API, change component to DOM.

Comment 6

[Chris Peterson [:cpeterson]]

Tracking for Fission Nightly (M6)

Comment 7

[Chris Peterson [:cpeterson]]

I can still reproduce this full screen bug in 78 Nightly.

Comment 8

[Alphan Chen [:alchen]]

Let me summarize how does it works in this scarino.

In non-fission case,
[non-fission-1] @DOMFullscreenParent.jsm
handleEvent(MozDOMFullscreen:Exited)
https://searchfox.org/mozilla-central/rev/3ce874dc2703831af3e5ef3a1d216ffd08057fa5/browser/actors/DOMFullscreenParent.jsm#88
--> call fullScreen.cleanDomFullscreen()

[non-fission-2] @browser-fullScreenAndPointerLock.js
cleanupDomFullscreen(aActor)
https://searchfox.org/mozilla-central/rev/3ce874dc2703831af3e5ef3a1d216ffd08057fa5/browser/base/content/browser-fullScreenAndPointerLock.js#490
--> Send "DOMFullscreen:CleanUp" MSG without remoteFrameBC. (https://searchfox.org/mozilla-central/rev/3ce874dc2703831af3e5ef3a1d216ffd08057fa5/browser/base/content/browser-fullScreenAndPointerLock.js#557)
--> At this time, we also restore the chrome UI from fullscreen.

[non-fission-3] @DOMFullscreenChild.jsm
receiveMessage(DOMFullscreen:CleanUp)
--> No remoteFrameBC. Don't set this. _isNotTheRequestSource (https://searchfox.org/mozilla-central/rev/3ce874dc2703831af3e5ef3a1d216ffd08057fa5/browser/actors/DOMFullscreenChild.jsm#55)

[non-fission-4] @DOMFullscreenChild.jsm
handleEvent(MozDOMFullscreen:Exited)
--> Send "DOMFullscreen:Exit" MSG to parent. (https://searchfox.org/mozilla-central/rev/3ce874dc2703831af3e5ef3a1d216ffd08057fa5/browser/actors/DOMFullscreenChild.jsm#103)

[non-fission-5] @DOMFullscreenParent.jsm
receiveMessage(DOMFullscreen:Exit)
--> call windowUtils.remoteFrameFullscreenReverted()(https://searchfox.org/mozilla-central/rev/3ce874dc2703831af3e5ef3a1d216ffd08057fa5/browser/actors/DOMFullscreenParent.jsm#44)

Comment 9

[Alphan Chen [:alchen]]

In fission case,
[fission-1] @ DOMFullscreenParent.jsm
handleEvent(MozDOMFullscreen:Exited)
https://searchfox.org/mozilla-central/rev/3ce874dc2703831af3e5ef3a1d216ffd08057fa5/browser/actors/DOMFullscreenParent.jsm#88
--> call fullScreen.cleanDomFullscreen()

[fission-2] @ browser-fullScreenAndPointerLock.js
cleanupDomFullscreen(aActor)
https://searchfox.org/mozilla-central/rev/3ce874dc2703831af3e5ef3a1d216ffd08057fa5/browser/base/content/browser-fullScreenAndPointerLock.js#490
--> Send "DOMFullscreen:CleanUp" MSG with remoteFrameBC. (https://searchfox.org/mozilla-central/rev/3ce874dc2703831af3e5ef3a1d216ffd08057fa5/browser/base/content/browser-fullScreenAndPointerLock.js#545)
--> At this time, we don't restore the chrome UI.

[fission-3] @ DOMFullscreenChild.jsm
receiveMessage(DOMFullscreen:CleanUp)
--> There is remoteFrameBC. Set this. _isNotTheRequestSource as true.(https://searchfox.org/mozilla-central/rev/3ce874dc2703831af3e5ef3a1d216ffd08057fa5/browser/actors/DOMFullscreenChild.jsm#44)
--> Call windowUtils.exitFullscreen() if there is fullscreenElement in this document. (https://searchfox.org/mozilla-central/rev/3ce874dc2703831af3e5ef3a1d216ffd08057fa5/browser/actors/DOMFullscreenChild.jsm#53)

[fission-4] @ DOMFullscreenChild.jsm
handleEvent(MozDOMFullscreen:Exited)
--> Since 'this._isNotTheRequestSource' is true, we send "DOMFullscreen:Exited" MSG to parent. (https://searchfox.org/mozilla-central/rev/3ce874dc2703831af3e5ef3a1d216ffd08057fa5/browser/actors/DOMFullscreenChild.jsm#95)

[fission-5] @ DOMFullscreenParent.jsm
receiveMessage(DOMFullscreen:Exited)
--> Call fullScreen.cleanDomFullscreen() (https://searchfox.org/mozilla-central/rev/3ce874dc2703831af3e5ef3a1d216ffd08057fa5/browser/actors/DOMFullscreenParent.jsm#48)
--> Then we will have the same sequence from [non-fission-2] to [non-fission-5]

Comment 10

[Alphan Chen [:alchen]]

It is easy to reproduce on mac and windows.
However, sometimes it is not that easy after adding some logs.
As a result, I think it is somehow a timing issue.

I meet two cases that will cause the symptom as this bug described.

[1] in [fission-5], the this.browsingContext.top.embedderElement is null. So we return early and the chrome UI keeps fullscreen state.
https://searchfox.org/mozilla-central/rev/3ce874dc2703831af3e5ef3a1d216ffd08057fa5/browser/actors/DOMFullscreenParent.jsm#19

[2] after [fission-2], we receive "MozDOMFullscreen:Exited" event[fission-4] first. At that time, we send MSG ""DOMFullscreen:Exit"" to parent. https://searchfox.org/mozilla-central/rev/3ce874dc2703831af3e5ef3a1d216ffd08057fa5/browser/actors/DOMFullscreenChild.jsm#103
Then in [fission-5], we also early return due to the same reason as the previous case(this.browsingContext.top.embedderElement is null).
https://searchfox.org/mozilla-central/rev/3ce874dc2703831af3e5ef3a1d216ffd08057fa5/browser/actors/DOMFullscreenParent.jsm#19

Comment 11

[Alphan Chen [:alchen]]

Attachment: Bug 1620341 - Handle the special case when closing tab which has a cross-origin embedded YouTube video

Comment 12

[Alphan Chen [:alchen]]

Try.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=93b5dff1b1e2513cbd1e026937f2458f914e5bc5

Comment 13

[Alphan Chen [:alchen]]

Attachment: Bug 1620341 - Add didDestroy() in DOMFullscreenParent.jsm to avoid the window stays in fullscreen problem

Comment 14

[Alphan Chen [:alchen]]

Here(attachment 9150072 [details]) we have another implementation based on the same idea.

Comment 15

[Alphan Chen [:alchen]]

(In reply to Alphan Chen [:alchen] from comment #14)

Here(attachment 9150072 [details]) we have another implementation based on the same idea.
I verified this on Mac.

I will try this implementation on windows later.

Try
https://treeherder.mozilla.org/#/jobs?repo=try&revision=eca6ff1f0f82abea5e676268881feaeae9bcadb1

Comment 16

[Chris Peterson [:cpeterson]]

I tested the Try builds from comment 12 and comment 15 on Windows 10. The bug seems to be fixed in both builds. 👍🏻

Comment 17

[Alphan Chen [:alchen]]

I create a test. The test can catch the problem when fission is enabled.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5e698718742a5f56bd07d71833c544426d5a640d

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9173573143df
Add didDestroy() in DOMFullscreenParent.jsm to avoid the window stays in fullscreen problem r=Gijs,smaug

Comment 19

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/9173573143df

Comment 20

[Chris Peterson [:cpeterson]]

I verified this fix works in 79 Nightly build 2020-06-09.

We don't need to uplift this fix to 78 Beta because this bug only affects Fission (which is disabled in Beta and Release).