1134192 - Opening a new tab while playing a video in fullscreen will open the tab in some sort of fullscreen mode

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Flaviu Cos]

Attachment: new opened tab.png

Environment: 
Device: Lenovo Yoga Tab 10 (Android 4.4.2);
Build: Nightly 38.0a1 (2015-02-17);

Steps to reproduce:
1. Launch fennec;
2. Launch a video (e.g.: http://people.mozilla.com/~nhirata/html_tp/elephants-dream.webm );
3. Make the video fullscreen;
4. Tap on the three dot menu from the bottom right corner;
5. Tap on 'New Tab' option from the Options menu.

Expected result:
A new tab is successfully opened.

Actual result:
A new tab is opened is some sort of fullscreen mode.

Notes:
Please check the attached screenshot.

This bug is specific to those devices that have the three dot menu at the bottom right corner;

Comment 1

[cmtalbert]

Looks like this happens on release and on beta 10? Kevin, do you think this needs to be addressed earlier than the next cycle or is the next cycle ok in your view?

Comment 2

[Kevin Brosnan [Ex-Mozilla]]

This is non-standard UI for this ROM. I don't have devices that place the 3 dot menu in the bottom. Nominated. This seems like a plus at best.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

This is also reproducible on phones with a hardware menu button (verified by blassey). We need to handle this code path.

Comment 4

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

Attachment: Galaxy chat w/ menu over fullscreen video - gap at bottom

I can't reproduce on my galaxy chat but instead get a space below the fullscreen video and when the menu is closed, the fullscreen video is scrollable. O_o (My proposed solutions below should fix this...)

In any case, we'll probably get a lot of egde case issues with accessing the menu during fullscreen mode so my two suggestions are:
  1) Disable the menu during fullscreen mode. This is similar to how we disable the menu when in editing mode on phones. 
  2) Close fullscreen mode when the menu button is pressed.

I'm leaning #2 because it doesn't limit functionality and once the user does it a few times, they'll get the idea. What say you, Finkle?

Comment 5

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

(In reply to Michael Comella (:mcomella) from comment #4)
> I can't reproduce on my galaxy chat but instead get a space below the
> fullscreen video and when the menu is closed, the fullscreen video is
> scrollable.

After disabling the dynamic toolbar, I no longer have this issue so it is likely due to [1]:

mDynamicToolbar.setVisible(true, VisibilityTransition.ANIMATE);

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.java?rev=1e087e6607f1#2894

Comment 6

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

Comment on attachment 8568029 [details]
Galaxy chat w/ menu over fullscreen video - gap at bottom

(In reply to Michael Comella (:mcomella) from comment #5)
> (In reply to Michael Comella (:mcomella) from comment #4)
> > I can't reproduce on my galaxy chat but instead get a space below the
> > fullscreen video and when the menu is closed, the fullscreen video is
> > scrollable.
> 
> After disabling the dynamic toolbar, I no longer have this issue so it is
> likely due to [1]:
> 
> mDynamicToolbar.setVisible(true, VisibilityTransition.ANIMATE);

I removed this line and still have the issue so I filed bug 1135780.

It seems painful to deal with this edge case so I'm just going to disable the menu in fullscreen mode for now. We can later create a new menu with a "Close fullscreen" menu item if that's desirable (mentor bug!).

Comment 7

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

Attachment: MozReview Request: bz://1134192/mcomella

/r/4193 - Bug 1134192 - Add ActivityUtils.isFullScreen. r=mfinkle
/r/4195 - Bug 1134192 - Prevent the options menu from opening in fullscreen mode. r=mfinkle

Pull down these commits:

hg pull review -r 3db64ab65d30f43b0d20294988cec676049f6bd1

Comment 8

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

Note that the commits in comment 7 maintains the incorrect API division from bug 1135796 to ensure we are consistent. I'm unsure why this hasn't already broken things (at least that we've noticed).

I tested that fullscreen mode in webm video (comment 0) and reader mode (i.e. menu still accessible) behaves as expected.

I'll let Finkle answer the NI in the review.

Comment 9

[Kevin Brosnan [Ex-Mozilla]]

The grey bar is a different bug. bug 1032179

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8568147 [details]
MozReview Request: bz://1134192/mcomella

https://reviewboard.mozilla.org/r/4191/#review3379

Disabling the menu when in Fullscreen seems like a safe approach. As you point out, we already do this when editing a URL.

I kinda liked the idea of leaving fullscreen on menu button press, but if it's causing issues, we can revisit it later. Let's see how the disbaled menu feels for now.

Comment 11

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

remote:   https://hg.mozilla.org/integration/fx-team/rev/5eab6df483fb
remote:   https://hg.mozilla.org/integration/fx-team/rev/0a3ff6c99a9b

Comment 12

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

(In reply to Mark Finkle (:mfinkle) from comment #10)
> I kinda liked the idea of leaving fullscreen on menu button press, but if
> it's causing issues, we can revisit it later. Let's see how the disbaled
> menu feels for now.

bug 1136325.

Comment 13

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

Comment on attachment 8568147 [details]
MozReview Request: bz://1134192/mcomella

Approval Request Comment
[Feature/regressing bug #]: N/A

[User impact if declined]:
  Users with hardware menu buttons can cause the browser to get into an inconsistent state by opening and using the menu during fullscreen mode. Unpolished and potentially makes the browser unusable.

[Describe test coverage new/current, TreeHerder]: None

[Risks and why]: We could prevent the menu from opening on all devices if the added isFullScreen method does not work correctly, but other than that, there would be no effect. Low risk.

[String/UUID change made/needed]: None

Comment 14

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

https://hg.mozilla.org/mozilla-central/rev/5eab6df483fb
https://hg.mozilla.org/mozilla-central/rev/0a3ff6c99a9b

Comment 15

[Liz Henry (:lizzard)]

Comment on attachment 8568147 [details]
MozReview Request: bz://1134192/mcomella

Approving for beta and aurora since it seems low-risk.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f18e2aecea6d
https://hg.mozilla.org/releases/mozilla-beta/rev/b23a690fa325

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ec26b08dcc08
https://hg.mozilla.org/releases/mozilla-aurora/rev/9a4542438b40

Comment 18

[Flaviu Cos]

Attachment: settings menu.png

On Lenovo Yoga Tab 10 (Android 4.4.2):
The settings menu is disabled while in fullscreen but there is still one case when it is accessible:
while in fullscreen mode swipe down from the top in order to display the android notifications bar then the settings menu becomes available again.

Should we consider this bug as fixed and wait for bug 1136325 or should I reopen it?

Comment 19

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

(In reply to Flaviu Cos, QA [:flaviu] from comment #18)
> Should we consider this bug as fixed and wait for bug 1136325 or should I
> reopen it?

Nice find. This is less important to fix (weird edge case) so I filed a mentored bug 1139013.

Comment 20

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

Didn't mean to reopen - this edge case will be fixed in the bug mentioned in comment 19.

Comment 21

[Flaviu Cos]

Verified as fixed in builds:
- 39.0a1 2015-03-05;
- 38.0a2 2015-03-05;
- 37 beta 2;
Device: Lenovo Yoga Tab 10 (Android 4.4.2).

Comment 23

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

Attachment: MozReview Request: Bug 1134192 - Add ActivityUtils.isFullScreen. r=mfinkle

Comment 24

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

Attachment: MozReview Request: Bug 1134192 - Prevent the options menu from opening in fullscreen mode. r=mfinkle