Last Comment Bug 685402 - Change tab/navigate should exit DOM full-screen mode
: Change tab/navigate should exit DOM full-screen mode
Status: RESOLVED FIXED
: dev-doc-complete
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: -- normal with 1 vote (vote)
: Firefox 10
Assigned To: Chris Pearce (:cpearce)
:
Mentors:
Depends on: 691947 729872
Blocks: 545812
  Show dependency treegraph
 
Reported: 2011-09-07 20:35 PDT by Chris Pearce (:cpearce)
Modified: 2012-02-23 05:23 PST (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch 1 v1: Detect navigation (9.95 KB, patch)
2011-11-01 23:41 PDT, Chris Pearce (:cpearce)
bzbarsky: review+
Details | Diff | Review
Patch 2 v1: Exit full-screen mode upon window deactivate or tab change/open/close (2.95 KB, patch)
2011-11-02 19:16 PDT, Chris Pearce (:cpearce)
dao+bmo: review+
Details | Diff | Review

Description Chris Pearce (:cpearce) 2011-09-07 20:35:41 PDT
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) 2011-10-03 16:32:49 PDT
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) 2011-10-12 12:55:00 PDT
(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) 2011-10-12 13:29:44 PDT
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] 2011-10-13 16:45:43 PDT
(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 5 Chris Pearce (:cpearce) 2011-10-26 18:24:22 PDT
*** Bug 697636 has been marked as a duplicate of this bug. ***
Comment 6 Chris Pearce (:cpearce) 2011-10-27 15:19:47 PDT
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) 2011-10-27 18:06:12 PDT
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 [:bz] 2011-10-27 19:17:37 PDT
> 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 [:bz] 2011-10-27 20:40:31 PDT
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) 2011-10-27 20:51:11 PDT
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 [:bz] 2011-10-27 21:05:08 PDT
Yes; like I said we should auto-deny requests after OnPageHide.
Comment 12 Chris Pearce (:cpearce) 2011-11-01 23:41:31 PDT
Created attachment 571251 [details] [diff] [review]
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) 2011-11-02 19:16:34 PDT
Created attachment 571536 [details] [diff] [review]
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] 2011-11-03 00:51:00 PDT
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 [:bz] 2011-11-03 20:22:29 PDT
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 18 Jean-Yves Perrier [:teoli] 2011-11-21 23:48:57 PST
For the record, this is documented there:
https://developer.mozilla.org/en/DOM/Using_full-screen_mode#Things_your_users_want_to_know

Note You need to log in before you can comment on or make changes to this bug.