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

Status: VERIFIED FIXED

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

Description

[Oana Horvath [:ohorvath]]

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.

Comment 1

[Nevin Chen(Not active on Bugzilla)]

Hi Joe, Wesly
Please help prioritize this.

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

CustomTabsActivity needs to tell GeckoView to exit fullscreen in onBackPressed().

Comment 3

[Nevin Chen(Not active on Bugzilla)]

Please help prioritize. keep the ni so I won't forget

Comment 4

[Wesly Huang (EPM)]

let's bring this to the 59.5 sprint planning.

Comment 5

[Nevin Chen(Not active on Bugzilla)]

Attachment: Bug 1427771 - Exit fullscreen with back key is pressed.

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

Comment 6

[Nevin Chen(Not active on Bugzilla)]

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?

Comment 7

[Nevin Chen(Not active on Bugzilla)]

Hi snorp
Could you please help advise with comment 6?
Thanks!

Comment 8

[Nevin Chen(Not active on Bugzilla)]

Attachment: pwa_test_site.zip

This is the test PWA website I used to test WebAppActivity. I think it also have the same problem.

Comment 9

[James Willcox (:snorp) ([e-mail]) (he/him)]

(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?

Comment 10

[Dylan Roeh (:droeh) (he/him)]

Yep, I'll look into it.

Comment 11

[Dylan Roeh (:droeh) (he/him)]

Attachment: Fix GeckoSession.exitFullScreen()

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.

Comment 12

[James Willcox (:snorp) ([e-mail]) (he/him)]

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.

Comment 13

[Dylan Roeh (:droeh) (he/him)]

Attachment: Fix GeckoSession.exitFullScreen()

Ah, you're right, that handleFullScreen call was redundant.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
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

Comment 15

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/0ec85117458e

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 17

[Dylan Roeh (:droeh) (he/him)]

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

Comment 18

[Liz Henry (:lizzard) (relman/hg->git project)]

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!

Comment 19

[Sorina Florean [:sflorean]]

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.

Comment 20

[Raul Gurzau (:RaulG)]

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

Comment 21

[Sorina Florean [:sflorean]]

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.