685402 - Change tab/navigate should exit DOM full-screen mode

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

When in DOM full-screen mode, navigating to a new page (i.e. clicking on a link) or changing tab to another document, should exit DOM full-screen mode.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

We must also exit full-screen mode when the browser window loses focus (a la ALT + TAB).

Comment 2

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

(In reply to Chris Pearce (:cpearce) (Mozilla Corporation) from comment #1)
> We must also exit full-screen mode when the browser window loses focus (a la
> ALT + TAB).

Is there not a way that we can stay in fullscreen mode even if the browser window loses focus. Flash recently added multi-monitor support to allow full-screen to stay in full-screen even when not in focus, and Silverlight has offered this feature for a much longer time.

I use it to pin videos on one monitor while doing work on another monitor.

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

The worry is that if we don't exit full-screen mode when we lose focus, the full-screen page could then spoof the user's desktop without them being aware of it.

Comment 4

[David Chan [:dchan]]

(In reply to Chris Pearce (:cpearce) (Mozilla Corporation) from comment #3)
> The worry is that if we don't exit full-screen mode when we lose focus, the
> full-screen page could then spoof the user's desktop without them being
> aware of it.

I'm having a hard time thinking of attacks if the desktop is spoofed. I believe the proposal is to restrict a majority of the keyboard input while in full-screen mode. If that is the case, I can see a clickjacking-like attack, but not a password/phishing attack in full-screen mode
* spoof the desktop
* convince user to click on the fake desktop, executing some action

This would be slightly more convincing than a normal clickjacking attack due to the browser chrome being hidden.

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

Boris: Is the best way to detect navigation while in full-screen mode to add an nsIWebProgressListener when full-screen is requested (in nsDocument::RequestFullScreen), and exit full-screen on document start loads and location change. We can remove the listener on exit full-screen. Or is there a better way?

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

Or can we add a hook to nsDocument::OnPageHide() to exit full-screen?

We can add a blur listener in browser.js to ensure we exit full-screen on change tab or alt+tab.

Comment 8

[Boris Zbarsky [:bzbarsky]]

> Or can we add a hook to nsDocument::OnPageHide() to exit full-screen?

Do you want to exit full-screen when the navigation starts, or when the old page is hidden?

OnPageHide is somewhat better, I think, because it would do the right thing if a subframe which has a full-screen element is removed from the DOM, right?

Comment 9

[Boris Zbarsky [:bzbarsky]]

One other thought.  If you exit full-screen OnPageHide, you can also enforce the invariant that a hidden document auto-denies fullscreen requests.  If you exit it on navigation start, then you have to worry about a new fullscreen request between then and actual unload, right?

Comment 10

[Chris Pearce [:cpearce (Not reading bugmail)]]

OnPageHide seems to work, and it does indeed catch the case when full-screen is requested after navigation but before unload.

We still need to explicitly handle the case where full-screen is requested after unload, as in Jesse's test case in bug 697636, that still fails with the OnPageHide change.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Yes; like I said we should auto-deny requests after OnPageHide.

Comment 12

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch 1 v1: Detect navigation

Part 1, exit full-screen mode on page hide. This happens when a docshell navigates.

Comment 13

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch 2 v1: Exit full-screen mode upon window deactivate or tab change/open/close

Part 2 (of 2), exit full-screen upon open new tab, change tab, close tab, or window deactivate.

Comment 14

[Dão Gottwald [:dao]]

Comment on attachment 571536 [details] [diff] [review]
Patch 2 v1: Exit full-screen mode upon window deactivate or tab change/open/close

>+    gBrowser.tabContainer.addEventListener("TabOpen", this.exitDomFullScreen, false);
>+    gBrowser.tabContainer.addEventListener("TabClose", this.exitDomFullScreen, false);
>+    gBrowser.tabContainer.addEventListener("TabSelect", this.exitDomFullScreen, false);

You can omit the third argument here.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 571251 [details] [diff] [review]
Patch 1 v1: Detect navigation

> nsWeakPtr nsDocument::sFullScreenDoc = nsnull;
>+nsWeakPtr nsDocument::sFullScreenRootDoc = nsnull;

Worth documenting what these are.

>+GetRootDocument(nsIDocument* aDoc)

I think this should use GetParentDocument until it hits null, just like the other code that manages full-screen state does.  Otherwise GetRootDocument on the document of a display:none iframe will return null, for example, which seems broken to me.

r=me with that

Comment 16

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a2d01d8c57d3
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7e0950aec83

Comment 17

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/a2d01d8c57d3
https://hg.mozilla.org/mozilla-central/rev/e7e0950aec83

Comment 18

[Jean-Yves Perrier [:teoli]]

For the record, this is documented there:
https://developer.mozilla.org/en/DOM/Using_full-screen_mode#Things_your_users_want_to_know