The default bug view has changed. See this FAQ.

Implement "You've entered full-screen" warning for Fennec

VERIFIED FIXED

Status

()

Firefox for Android
General
VERIFIED FIXED
6 years ago
8 months ago

People

(Reporter: cpearce, Assigned: Margaret)

Tracking

unspecified
Points:
---
Dependency tree / graph
Bug Flags:
in-litmus +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [QA+])

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
We need to do something sensible for the DOM full-screen API on fennec (bug 545812). The DOM full-screen API uses nsGlobalWindow::SetFullScreen(), and that works appropriately on Fennec apparently, so the base API should Just Work (once bug 684620 lands to proxy calls to nsGlobalWindow::SetFullScreen() to the chrome process)

Then we need to:
1. Implement a "you've entered full-screen, press the back button to exit" warning drop down.
2. Implement exiting full-screen when back button is pressed.

I'm not sure how to do (2). I can't initiate leaving full-screen mode from chrome since there (currently) appears to be no way to get hold of a corresponding PBrowserParent from any of the C++ entry points in the chrome process.
#1 could be implemented via the "fullscreen" event and displaying a popup (toaster) alert. Might be handy if we knew the "fullscreen" event was triggered via DOM and not some other reason. Maybe an event.reason or event.detail ?

#2 seems like a hole in the e10s/IPC framework.
Just saw "mozfullscreenchange" event. That'll do.
(Assignee)

Comment 3

6 years ago
I'm implementing #2 for native fennec in bug 698836. Since we're not developing XUL fennec anymore, we could probably move this bug to the native fennec component and make it about implementing #1.
(Reporter)

Comment 4

6 years ago
Sounds reasonable.
Component: General → General
Product: Fennec → Fennec Native
Summary: UI considerations for DOM full-screen API for Fennec → Implement "You've entered full-screen" warning for Fennec
Version: Trunk → unspecified
(Assignee)

Comment 5

6 years ago
And I can take it :)
Assignee: nobody → margaret.leibovic
(Assignee)

Updated

6 years ago
Depends on: 698836
No longer depends on: 684620
(Assignee)

Comment 6

6 years ago
Created attachment 572688 [details] [diff] [review]
patch

This depends on the patch in bug 698836.

We should probably ask a UX person about exactly what the string should say.
Attachment #572688 - Flags: review?(mark.finkle)
(Reporter)

Comment 7

6 years ago
Comment on attachment 572688 [details] [diff] [review]
patch

The warning on desktop Firefox is "Press ESC to exit full-screen mode".

Is there any way to enter window full-screen mode in Fennec (i.e. having chrome set |window.fullscreen=true|? In desktop Firefox you can do this by pressing F11, so we listen for the "mozfullscreenchange" event instead. This way we can also catch the case where the user is in window/browser full-screen mode and switches to DOM full-screen mode. Maybe it makes sense to do that here too?
(In reply to Chris Pearce (:cpearce) (Mozilla Corporation) from comment #7)
> Comment on attachment 572688 [details] [diff] [review] [diff] [details] [review]
> patch
> 
> The warning on desktop Firefox is "Press ESC to exit full-screen mode".
> 
> Is there any way to enter window full-screen mode in Fennec (i.e. having
> chrome set |window.fullscreen=true|? In desktop Firefox you can do this by
> pressing F11, so we listen for the "mozfullscreenchange" event instead. This
> way we can also catch the case where the user is in window/browser
> full-screen mode and switches to DOM full-screen mode. Maybe it makes sense
> to do that here too?

Bug 698836 adds a context menu for <video> allowing the window to go fullscreen, but it does not use window.fullScreen. It uses document.mozRequestFullScreen.

So you are saying that we should be using "mozfullscreenchange" event instead of "fullscreen" so we know anytime the system tries to use DOM fullscreen API, even if the window is already fullscreen?
Comment on attachment 572688 [details] [diff] [review]
patch


>diff --git a/mobile/locales/en-US/chrome/browser.properties b/mobile/locales/en-US/chrome/browser.properties

>+alertFullScreenToast=You've entered full-screen mode. Press the back button to exit.

Let's mimic the desktop string for now:

"Press BACK to leave full-screen mode"

===========
We should think about listening for "mozfullscreenchange" although, I'm not sure it gives us anything over "fullscreen" yet - given our simple implementation and chrome.

Looking at the ways we exit fullscreen mode on desktop makes me think we should add a few more to mobile:
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#3915

Places we could toggle out of fullscreen mode:
* In GeckoApp.onPause  (app is deactivated)
* In GeckoApp.handleAddTab, handleCloseTab and handleSelectTab ("TabOpen", "TabClose" and "TabSelect" events in desktop)
* In GeckoApp.onPrepareOptionsMenu when the menu is about to be shown (?? not sure about this one)

These could be a followup bug, after we talk to UX.

r+, but make the string change. the rest needs more thought and can be a followup.
Attachment #572688 - Flags: review?(mark.finkle) → review+
(Reporter)

Comment 10

6 years ago
(In reply to Mark Finkle (:mfinkle) from comment #8)
> So you are saying that we should be using "mozfullscreenchange" event
> instead of "fullscreen" so we know anytime the system tries to use DOM
> fullscreen API, even if the window is already fullscreen?

If there aren't any places that set window.fullscreen on mobile, then adding your listener to the "fullscreen" event is ok. The "fullscreen" event fires synchronously with the transition to full-screen mode, whereas the mozfullscreenchange event fires asynchronously after the fact. So if there's no uses of the window.fullscreen chrome API that could confuse matters, you're better off listening to "fullscreen" so you can get your warning up faster.

On desktop we also show the warning whenever there's alpha-numeric key input as a safeguard against password phishing. I guess it makes sense to show the warning on text input on mobile too; the bad guys could still fake the fennec UI going to paypal.com or whatever and phish for passwords.
(Assignee)

Updated

6 years ago
Blocks: 700678
(Assignee)

Updated

6 years ago
Blocks: 700679
(Assignee)

Comment 11

6 years ago
(In reply to Mark Finkle (:mfinkle) from comment #9)
> >+alertFullScreenToast=You've entered full-screen mode. Press the back button to exit.
> 
> Let's mimic the desktop string for now:
> 
> "Press BACK to leave full-screen mode"

s/leave/exit/ as per comment 7?

Also, I filed bug 700678 and 700679 as follow-ups.
(In reply to Margaret Leibovic [:margaret] from comment #11)
> (In reply to Mark Finkle (:mfinkle) from comment #9)
> > >+alertFullScreenToast=You've entered full-screen mode. Press the back button to exit.
> > 
> > Let's mimic the desktop string for now:
> > 
> > "Press BACK to leave full-screen mode"
> 
> s/leave/exit/ as per comment 7?
> 
> Also, I filed bug 700678 and 700679 as follow-ups.

Desktop uses "leave":
http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/browser.dtd#93
(Assignee)

Comment 13

6 years ago
(In reply to Mark Finkle (:mfinkle) from comment #12)
> (In reply to Margaret Leibovic [:margaret] from comment #11)
> > (In reply to Mark Finkle (:mfinkle) from comment #9)
> > > >+alertFullScreenToast=You've entered full-screen mode. Press the back button to exit.
> > > 
> > > Let's mimic the desktop string for now:
> > > 
> > > "Press BACK to leave full-screen mode"
> > 
> > s/leave/exit/ as per comment 7?
> > 
> > Also, I filed bug 700678 and 700679 as follow-ups.
> 
> Desktop uses "leave":
> http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/
> browser/browser.dtd#93

Oh, okay, I should have looked up the string myself. Thanks!
(Assignee)

Comment 14

6 years ago
https://hg.mozilla.org/projects/birch/rev/79690fc1fbf4
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Updated

6 years ago
Flags: in-litmus?(fennec)
Whiteboard: [QA+]
What does fullscreen actually do on mobile? Does it disable the displayport/viewport features and just make the document fill the screen exactly? Does it disable zooming?
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #15)
> What does fullscreen actually do on mobile? Does it disable the
> displayport/viewport features and just make the document fill the screen
> exactly? Does it disable zooming?

It simply removes the browser chrome and also allows the content to fill the entire device screen. Meaning, it hides the "status bar" across the top of the device where you normally see network, battery and status indicators.
more...

It does not affect zooming and panning. The content is given the entire device screen area, but zooming an panning would still function.
Arguably we should disable panning and zooming in fullscreen mode.
(separate bug of course)

Comment 20

6 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #18)
> Arguably we should disable panning and zooming in fullscreen mode.

I'm not sure I understand why?
The use-cases for fullscreen generally involve the app filling the entire screen with content designed to fit the screen. For them, panning and zooming is unneeded and unwanted.
If a poorly designed app makes content overflow the screen in fullscreen mode, it might make sense to reenable panning and zooming.
These patches were backed while investigating Talos failures.  Now that tests are green again, we will need to reland.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
backout was backed out https://hg.mozilla.org/projects/birch/rev/6f925b45a547
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago5 years ago
Resolution: --- → FIXED
20111114041052
http://hg.mozilla.org/projects/birch/rev/859ecdfe0168
Samsung Galaxy SII (Android 2.3.4)
Status: RESOLVED → VERIFIED

Comment 26

5 years ago
Test case added: https://litmus.mozilla.org/show_test.cgi?id=43039
Flags: in-litmus?(fennec) → in-litmus+
You need to log in before you can comment on or make changes to this bug.