Change tab/navigate should exit DOM full-screen mode

RESOLVED FIXED in Firefox 10

Status

()

Firefox
General
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: cpearce, Assigned: cpearce)

Tracking

({dev-doc-complete})

unspecified
Firefox 10
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

6 years ago
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.
(Assignee)

Updated

6 years ago
Assignee: nobody → chris
(Assignee)

Comment 1

6 years ago
We must also exit full-screen mode when the browser window loses focus (a la ALT + TAB).
(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.
(Assignee)

Comment 3

6 years ago
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

6 years ago
(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.
(Assignee)

Updated

6 years ago
Duplicate of this bug: 697636
(Assignee)

Comment 6

6 years ago
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?
(Assignee)

Comment 7

6 years ago
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.
> 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?
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?
(Assignee)

Comment 10

6 years ago
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.
Yes; like I said we should auto-deny requests after OnPageHide.
(Assignee)

Comment 12

6 years ago
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.
Attachment #571251 - Flags: review?(bzbarsky)
(Assignee)

Updated

6 years ago
Depends on: 691947
(Assignee)

Comment 13

6 years ago
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.
Attachment #571536 - Flags: review?(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.
Attachment #571536 - Flags: review?(dao) → review+
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
Attachment #571251 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 16

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a2d01d8c57d3
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7e0950aec83
Whiteboard: [inbound]
Target Milestone: --- → Firefox 10
https://hg.mozilla.org/mozilla-central/rev/a2d01d8c57d3
https://hg.mozilla.org/mozilla-central/rev/e7e0950aec83
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
For the record, this is documented there:
https://developer.mozilla.org/en/DOM/Using_full-screen_mode#Things_your_users_want_to_know
Keywords: dev-doc-complete

Updated

6 years ago
Depends on: 729872
You need to log in before you can comment on or make changes to this bug.