Closed Bug 1134192 Opened 5 years ago Closed 5 years ago

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

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 39
Tracking Status
firefox35 --- wontfix
firefox36 --- wontfix
firefox37 --- verified
firefox38 --- verified
firefox39 --- verified
fennec 37+ ---

People

(Reporter: cos_flaviu, Assigned: mcomella)

References

(Depends on 1 open bug, Blocks 2 open bugs)

Details

Attachments

(4 files, 2 obsolete files)

Attached image 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;
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?
Flags: needinfo?(kbrosnan)
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.
tracking-fennec: --- → ?
Flags: needinfo?(kbrosnan)
This is also reproducible on phones with a hardware menu button (verified by blassey). We need to handle this code path.
Assignee: nobody → michael.l.comella
tracking-fennec: ? → 37+
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?
Flags: needinfo?(mark.finkle)
(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 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!).
Attachment #8568029 - Attachment is obsolete: true
/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
Attachment #8568147 - Flags: review?(mark.finkle)
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.
Flags: needinfo?(mark.finkle)
The grey bar is a different bug. bug 1032179
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.
Attachment #8568147 - Flags: review?(mark.finkle) → review+
(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 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
Attachment #8568147 - Flags: approval-mozilla-beta?
Attachment #8568147 - Flags: approval-mozilla-aurora?
Comment on attachment 8568147 [details]
MozReview Request: bz://1134192/mcomella

Approving for beta and aurora since it seems low-risk.
Attachment #8568147 - Flags: approval-mozilla-beta?
Attachment #8568147 - Flags: approval-mozilla-beta+
Attachment #8568147 - Flags: approval-mozilla-aurora?
Attachment #8568147 - Flags: approval-mozilla-aurora+
Attached image 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?
Flags: needinfo?(michael.l.comella)
(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.
Status: RESOLVED → REOPENED
Flags: needinfo?(michael.l.comella)
Resolution: FIXED → ---
Didn't mean to reopen - this edge case will be fixed in the bug mentioned in comment 19.
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
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).
Attachment #8568147 - Attachment is obsolete: true
Attachment #8619508 - Flags: review+
Attachment #8619509 - Flags: review+
You need to log in before you can comment on or make changes to this bug.