1290882 - Exit fullscreen snackbar references back button even though back is hidden in immersive fullscreen

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Audio/Video, defect)

Description

[Teodora Vermesan (:TeoVermesan)]

Attachment: Screenshot from 2016-08-01 15:23:25.png

Steps to reproduce:
1. Go to http://base-n.de/webm/out9.webm
2. Enter fullscreen
3. Try to exit fullscreen

Expected results:
- Back button is visible and by tapping it, the video exits fullscreen mode

Actual results:
- Back button is not visible.
- See attached screenshot

Comment 1

[Tom Klein]

Fullscreen behavior was changed by bug 1031519 - once in fullscreen I'm able to swipe down from the top to reveal the back button, and then tapping it exits fullscreen mode.  Does that work for you?

IIRC the first time I entered immersive fullscreen with that change it gave some instructions on swiping from the top to display the soft keys; maybe that's not always happening the first time?

Comment 2

[Tom Klein]

Teodora, do you get the expected behavior of swipe-from-the-top revealing the soft keys in fullscreen mode and tapping back exiting fullscreen, or is that broken on your device?

Comment 3

[Teodora Vermesan (:TeoVermesan)]

Swiping-from-the-top will reveal the soft keys.
But, after pressing the fullscreen button, a snackbar is displayed: "Press back to exit full-screen mode". The soft keys are not displayed, only after swipe from the top.
Maybe a snackbar that inform users to swipe to reveal the back button should be displayed.

Comment 4

[Tom Klein]

> Maybe a snackbar that inform users to swipe to reveal the back button should
> be displayed.

Thanks Teo.  Mike, any thoughts on this?

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

There are several layers coding, which makes this confusing. There is:
 * The system – it displays the "swipe from the top" the first time immersive fullscreen mode is used
 * Fennec, the app – What we decide to show (currently, a snackbar saying, "Press BACK to exit")
 * The default controls for HTML5 players that our platform provides – provides a "leave fullscreen" button (present in the example in 1)
 * The overridden controls for HTML5 players that the site provides – sometimes provides "leave fullscreen" (e.g. youtube), sometimes does not (http://html5demos.com/video)

> Maybe a snackbar that inform users to swipe to reveal the back button should
> be displayed.

I think we have two options when entering immersive fullscreen mode for videos:
 1) Display a snackbar every time that informs the user to "tap back" on devices with hardware keys and "swipe from the top & hit back" on devices with software keys
 2) Don't display anything and rely on the system's first intro & the video player controls to help users leave fullscreen mode

#1 has issues:
 * The messaging in the snackbar will be duplicated by the system's tutorial the first time immersive full screen is entered
 * The patch for bug 1031519 is in FF50 and we'd have strings to add, so we'd have to back it out of 50 so the strings can ride the trains

#2 also has a few issues:
 * There'd be no help on pages where the built-in video controls don't provide a button to leave fullscreen
 * On http://html5demos.com/video, the built-in controls are disabled by default (though you can enable them with long-press) – other pages may also do this? Note that on this page you have to long-press to go into full screen in the first place.

Anthony, do you think we should go for option #1 or option #2? If #1, do you have a preference on copy?

[1]: then again, if the system thinks the user can handle a one-time tutorial, maybe we should agree with them?

Comment 6

[gibbering.baboons]

Immersive fullscreen has been a part of android for three years now.  Users should know how it works as it is the same in every app out there.  Please don't do anything to delay it further in Firefox.  It is hugely overdue and reflects poorly on Firefox.

Comment 7

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Before we proceed further, I notice that the media controls in the Aurora screenshot are out of place. If the back, home, and recents navigation bar is missing, our video controls should occupy that space. Is this a known issue/ fixed?

Pressing the "exit fullscreen button" is the primary way in our UI for the user to exit video, does this work Teodora?

Comment 8

[Tom Klein]

(In reply to Anthony Lam (:antlam) from comment #7)
> Before we proceed further, I notice that the media controls in the Aurora
> screenshot are out of place. If the back, home, and recents navigation bar
> is missing, our video controls should occupy that space. Is this a known
> issue/ fixed?

If I can jump in, I think the Aurora screenshot shows the nav buttons in "low profile" (dimmed) mode, which is what we still use instead of immersive with the bug 1031519 patch on older devices - the little dots in place of back, home, and recents are just hard to see in the screenshot I think.  But as far as I know that's the expected behavior.

Comment 9

[Teodora Vermesan (:TeoVermesan)]

Attachment: Screenshot from 2016-08-19 10:55:22.png

Tested using:
Builds: Firefox for Android 50.0a2 and 51.0a1 (2016-08-18) and Firefox for Android 49 Beta 4.
One A2001 (Android 5.1.1) with software on-screen navigation bar enabled

Comment 10

[Teodora Vermesan (:TeoVermesan)]

Exiting fullscreen mode works ok on all branches with both methods: fullscreen button and pressing the back button.

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Anthony, the issue is that a contradictory snackbar is shown when we enter fullscreen mode that says to "Press back to exit", even on devices where the back button is hidden.

Two options I saw were to fix the snackbar to be accurate to device type (comment 5, option 1) or remove it entirely (option 2). Thoughts?

Sorry if I was unclear.

Comment 12

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(In reply to Michael Comella (:mcomella) [not actively working on fennec: expect slow responses] from comment #11)
> Anthony, the issue is that a contradictory snackbar is shown when we enter
> fullscreen mode that says to "Press back to exit", even on devices where the
> back button is hidden.
> 
> Two options I saw were to fix the snackbar to be accurate to device type
> (comment 5, option 1) or remove it entirely (option 2). Thoughts?
> 
> Sorry if I was unclear.

By "Hidden" do you mean dimmed like Teodora is indicating in comment 9? If so, I think that's fine. I'm not too worried about it.

If not, could you show me a screenshot of this "snackbar"?

Comment 13

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Screenshot of the issue

(In reply to Anthony Lam (:antlam) from comment #12)
> If not, could you show me a screenshot of this "snackbar"?

This screenshot is taken on Nightly, on a N5, which has had its software menu button entirely removed (i.e. not hidden or "low profile") upon entering fullscreen mode (this fullscreen mode is known as "immersive fullscreen" in android lingo).

Comment 14

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(In reply to Michael Comella (:mcomella) [not actively working on fennec: expect slow responses] from comment #13)
> Created attachment 8786787 [details]
> Screenshot of the issue
> 
> (In reply to Anthony Lam (:antlam) from comment #12)
> > If not, could you show me a screenshot of this "snackbar"?
> 
> This screenshot is taken on Nightly, on a N5, which has had its software
> menu button entirely removed (i.e. not hidden or "low profile") upon
> entering fullscreen mode (this fullscreen mode is known as "immersive
> fullscreen" in android lingo).

In these cases, could we just not show this then?

Comment 15

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Spoke on Vidyo: we're just going to remove the "Press back to exit" snackbar entirely. The old video player didn't have an exit fullscreen button in the built-in video controls but now we have one so we theorize the snackbar is no longer necessary.

Comment 16

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Bug 1290882 - Remove 'Press back to exit' snackbar when entering fullscreen mode.

We decided to remove it because:
 * Modern video controls include an exit fullscreen button
 * After bug 1031519, you have to swipe down from the top of the screen to
display the soft back button so the copy does not make sense.

Note that the `MozShowFullScreenWarning` event was removed previously in the
platform in [1] so we remove the listener attachment here without replacing it.

[1]: https://hg.mozilla.org/integration/fx-team/rev/a6a5f79e630d

Review commit: https://reviewboard.mozilla.org/r/80228/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/80228/

Comment 17

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Let's try to get this into the version it affects.

Comment 18

[Andrzej Hunt :ahunt]

Comment on attachment 8793514 [details]
Bug 1290882 - Remove 'Press back to exit' snackbar when entering fullscreen mode.

https://reviewboard.mozilla.org/r/80228/#review78900

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dae8eaf6a289
Remove 'Press back to exit' snackbar when entering fullscreen mode. r=ahunt

Comment 20

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8793514 [details]
Bug 1290882 - Remove 'Press back to exit' snackbar when entering fullscreen mode.

Approval Request Comment
[Feature/regressing bug #]: bug 1031519
[User impact if declined]: Users will see a popup with a conflicting message on devices with soft keyboards.

[Describe test coverage new/current, TreeHerder]: Tested locally.
[Risks and why]: Low - we're removing minimal amounts of seemingly simple code so I don't expect any issues.
[String/UUID change made/needed]: Removed a string

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/dae8eaf6a289

Comment 22

[Francesco Lodolo [:flod]]

(In reply to Michael Comella (:mcomella) [not actively working on fennec: expect slow responses] from comment #20)
> [String/UUID change made/needed]: Removed a string

If you want to land this in aurora and beta, please create a patch that doesn't touch browser.properties
This patch would create unnecessary churn in localization tools and dashboards.

Comment 23

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Mike, it seems the l10n team would like the patch to be redone so as to minimize localization impact. Please renominate the updated patch, I'll cancel the flags on the current one.

Comment 24

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: (51 branch patch) Remove 'Press back to exit' snackbar when entering fullscreen mode

Comment 25

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: 50 branch patch

Comment 26

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8796705 [details] [diff] [review]
(51 branch patch) Remove 'Press back to exit' snackbar when entering fullscreen mode

(See comment 20)

Comment 27

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8796713 [details] [diff] [review]
50 branch patch

(See comment 20)

Comment 28

[Teodora Vermesan (:TeoVermesan)]

'Press BACK to leave full-screen mode' snackbar is not displayed when entering fullscreen mode, so:
Verified as fixed using:
Device: One A2001 (Android 6.0.1)
Build: Firefox for Android 52.0a1 (2016-10-02)

Comment 29

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8796713 [details] [diff] [review]
50 branch patch

Fix was verified on Nightly52, Beta50+

Comment 30

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f1d381c06728

Comment 31

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/f81892b5a9c8

Comment 32

[u421692]

Verified as fixed on Nexus 6P (Android 7.0) on Fennec 50 Beta 4