Closed Bug 1427771 Opened 2 years ago Closed 2 years ago

Blank screen when youtube exits from full-screen in custom tabs

Categories

(Firefox for Android :: Custom Tabs, defect, P2)

ARM
Android
defect

Tracking

()

VERIFIED FIXED
Firefox 60
Tracking Status
fennec + ---
firefox58 --- unaffected
firefox59 --- verified
firefox60 --- verified

People

(Reporter: oana.horvath, Assigned: cnevinchen, NeedInfo)

Details

(Whiteboard: [FNC][SPT59.5][BL])

Attachments

(3 files, 1 obsolete file)

Device:
HTC Desire 820 (Android 6.0.1)
Google Pixel (Android 8.0)
Build: Nightly 59.0a1 (2018-01-03);

Steps to reproduce:
1. Open youtube in a custom tab (e.g. from gmail).
2. Play a video in fullscreen mode.
3. Drag the notification bar down to show the device's buttons. Tap the back button.

Expected result:
The video exits fullscreen.

Actual result:
A blank dark screen covers the page.

Note: not reproducing if you exit by tapping the video fullscreen button.
Hi Joe, Wesly
Please help prioritize this.
Flags: needinfo?(wehuang)
Flags: needinfo?(jcheng)
Priority: -- → P2
CustomTabsActivity needs to tell GeckoView to exit fullscreen in onBackPressed().
tracking-fennec: ? → +
Flags: needinfo?(cnevinchen)
Please help prioritize. keep the ni so I won't forget
let's bring this to the 59.5 sprint planning.
Flags: needinfo?(wehuang)
Assignee: nobody → cnevinchen
Flags: needinfo?(cnevinchen)
Whiteboard: [FNC][SPT59.5][BL]
Comment on attachment 8942067 [details]
Bug 1427771 - Exit fullscreen with back key is pressed.

https://reviewboard.mozilla.org/r/212282/#review218082

::: mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java:273
(Diff revision 1)
>      @Override
>      public void onBackPressed() {
> -        if (mCanGoBack) {
> +        final boolean fullScreen = ActivityUtils.isFullScreen(this);
> +        if (fullScreen) {
> +            handleFullScreen(false);
> +            mGeckoSession.exitFullScreen();

Youtube stays in full screen after I call mGeckoSession.exitFullScreen().

Maybe I use the wrong mehtod?
Hi snorp
Could you please help advise with comment 6?
Thanks!
Flags: needinfo?(snorp)
Attached file pwa_test_site.zip
This is the test PWA website I used to test WebAppActivity. I think it also have the same problem.
(In reply to Nevin Chen [:nechen] from comment #6)
> Comment on attachment 8942067 [details]
> Bug 1427771 - Exit fullscreen with back key is pressed.
> 
> https://reviewboard.mozilla.org/r/212282/#review218082
> 
> :::
> mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.
> java:273
> (Diff revision 1)
> >      @Override
> >      public void onBackPressed() {
> > -        if (mCanGoBack) {
> > +        final boolean fullScreen = ActivityUtils.isFullScreen(this);
> > +        if (fullScreen) {
> > +            handleFullScreen(false);
> > +            mGeckoSession.exitFullScreen();
> 
> Youtube stays in full screen after I call mGeckoSession.exitFullScreen().
> 
> Maybe I use the wrong mehtod?

Nope you're doing it right. Maybe it's broken? Dylan can you check what's going with fullscreen and custom tabs?
Flags: needinfo?(snorp) → needinfo?(droeh)
Yep, I'll look into it.
Flags: needinfo?(droeh)
This just takes Nevin's existing patch and adds a fix to GeckoSession.exitFullScreen(), which was currently sending a message that nothing was listening for.
Attachment #8944851 - Flags: review?(snorp)
Comment on attachment 8944851 [details] [diff] [review]
Fix GeckoSession.exitFullScreen()

Review of attachment 8944851 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java
@@ +268,5 @@
>      @Override
>      public void onBackPressed() {
> +        final boolean fullScreen = ActivityUtils.isFullScreen(this);
> +        if (fullScreen) {
> +            handleFullScreen(false);

I don't think you need this, right? I think you should get an onFullScreen() after exitFullScreen() is called. If that doesn't happen we should fix it so it does.
Attachment #8944851 - Flags: review?(snorp) → review-
Ah, you're right, that handleFullScreen call was redundant.
Attachment #8944851 - Attachment is obsolete: true
Attachment #8944913 - Flags: review?(snorp)
Attachment #8944913 - Flags: review?(snorp) → review+
Pushed by droeh@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ec85117458e
Fix broken exitFullScreen() in GeckoSession and call it in onBackPressed in custom tabs and PWAs. r=snorp
https://hg.mozilla.org/mozilla-central/rev/0ec85117458e
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Please request Beta approval on this when you get a chance.
Flags: needinfo?(cnevinchen)
Flags: needinfo?(cnevinchen) → needinfo?(droeh)
Comment on attachment 8944913 [details] [diff] [review]
Fix GeckoSession.exitFullScreen()

Approval Request Comment
[Feature/Bug causing the regression]: 1370605
[User impact if declined]: Back button will not work as expected when in full screen mode for PWAs and custom tabs
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Just a minor change with no obvious potential to crash, etc.
[String changes made/needed]: No
Flags: needinfo?(droeh)
Attachment #8944913 - Flags: approval-mozilla-beta?
Summary: Blank screen when youtube exists from full-screen in custom tabs → Blank screen when youtube exits from full-screen in custom tabs
Comment on attachment 8944913 [details] [diff] [review]
Fix GeckoSession.exitFullScreen()

Regression in 59. Let's uplift this for beta 6. 

I should note though, when we say "verified in nightly" we mean that QA has tested and marked the bug as verified, which I don't think has happened here. 

Ioana or Sorina, can you verify the fix on beta once it lands in 59? Thanks!
Flags: needinfo?(ioana.chiorean)
Attachment #8944913 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Verified as fixed in build 60.0a1 from 01-29. 
Device: Google Pixel (Android 8.0).
Blank screen is not displayed anymore and the video exit correctly from full-screen.
Flags: needinfo?(ioana.chiorean)
Verified as fixed in build 59.0b13.
Devices: HTC 10(Android 8.1.0) and Google Pixel(Android 8.0).
Blank screen is not displayed when video exit from full-screen.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.