767809 - Exiting full screen video keeps browser in full screen mode

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Martijn Wargers (dead)]

Steps to reproduce:
- Visit a video (like where the url points to)
- Go into full screen
- Tap on the Android back button

Expected result:
- Browser goes out of full screen mode

Actual result:
- Browser is still in full screen mode

I can see this bug in current trunk build, but not in the latest Aurora build.

Comment 1

[Cristian Nicolae (:xti)]

I cannot reproduce this issue on the latest Nightly. Should I do anything else then steps mentioned above? 

--
Firefox 16.0a1 (2012-06-27)
Device: Galaxy Nexus
OS: Android 4.0.2

Comment 2

[Martijn Wargers (dead)]

I can't reproduce either anymore, so marking wfm.

Comment 3

[Martijn Wargers (dead)]

Hrm, I can still reproduce it.
I guess this is happening when you have only 1 tab open with the video and no 'back' history.

Comment 4

[Cristian Nicolae (:xti)]

(In reply to Martijn Wargers [:mw22] (QA - IRC nick: mw22) from comment #3)
> Hrm, I can still reproduce it.
> I guess this is happening when you have only 1 tab open with the video and
> no 'back' history.

I was able to reproduce it twice so far, following your suggestion, but still is hardly to make it happens. I will try to find a regression range

Comment 5

[Cristian Nicolae (:xti)]

Last good build: 06/14 m-c

Bad build: 06/15 m-c

A possible regression range is: 
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3f408698a03f&tochange=da8c6039c25e

--
Device: Galaxy Nexus
OS: Android 4.0.2

Comment 6

[Justin Wood (:Callek)]

I just hit what sounds like this on my galaxy S III.

steps I did:

* load http://blip.tv/day9 one of his newer videos with flash installed
* press and hold in black area where video should be... select to enter full screen mode

you are now in JUST a black screen

* press android back button

you are now in a full screen version of the whole page.

* more back button presses bring you back to android desktop. reenter app...menu....quit and we are back to normal after restart

while in bad mode no urlbar access to create more tabs etc

Comment 7

[Justin Wood (:Callek)]

that was on aurora 15 from today. also just hit a weird typing bug while entering last comment someone remind me on circ to describe it ;-)

Comment 8

[Matt Brubeck (:mbrubeck)]

I get the following warnings in the log:

I/GeckoApp( 3005): Got message: ToggleChrome:Hide
I/dalvikvm( 3005): Could not find method android.view.View.setSystemUiVisibility, referenced from method org.mozilla.gecko.GeckoApp$31.run
W/dalvikvm( 3005): VFY: unable to resolve virtual method 777: Landroid/view/View;.setSystemUiVisibility (I)V
W/System.err( 3005): android.view.ViewRoot$CalledFromWrongThreadException: Only the original thread that created a view hierarchy can touch its views.
W/System.err( 3005): 	at android.view.ViewRoot.checkThread(ViewRoot.java:2942)
W/System.err( 3005): 	at android.view.ViewRoot.invalidateChild(ViewRoot.java:642)
W/System.err( 3005): 	at android.view.ViewRoot.invalidateChildInParent(ViewRoot.java:668)
W/System.err( 3005): 	at android.view.ViewGroup.invalidateChild(ViewGroup.java:2511)
W/System.err( 3005): 	at android.view.View.invalidate(View.java:5276)
W/System.err( 3005): 	at android.view.View.setFlags(View.java:4639)
W/System.err( 3005): 	at android.view.View.setVisibility(View.java:3113)
W/System.err( 3005): 	at org.mozilla.gecko.BrowserToolbar.hide(BrowserToolbar.java:546)
W/System.err( 3005): 	at org.mozilla.gecko.BrowserApp.setFullScreen(BrowserApp.java:578)
W/System.err( 3005): 	at org.mozilla.gecko.GeckoAppShell.setFullScreen(GeckoAppShell.java:1371)
W/System.err( 3005): 	at org.mozilla.gecko.GeckoAppShell.nativeRun(Native Method)
W/System.err( 3005): 	at org.mozilla.gecko.GeckoAppShell.nativeRun(Native Method)
W/System.err( 3005): 	at org.mozilla.gecko.GeckoAppShell.runGecko(GeckoAppShell.java:483)
W/System.err( 3005): 	at org.mozilla.gecko.GeckoThread.run(GeckoThread.java:105)

This code is running on the main thread because GeckoApp.setFullScreen posts to mMainHandler.  It looks like it should be on a different thread instead (or we need to move some code from elsewhere to the main thread)?

Comment 9

[Matt Brubeck (:mbrubeck)]

Attachment: patch

No wait, my last comment was incorrect.  The problem is that BrowserApp.setFullScreen does *not* run its code on the UI thread.  This patch just wraps that code in mMainHandler.post to fix this bug.

Comment 10

[Matt Brubeck (:mbrubeck)]

It looks like the problematic code was introduced by bug 753102, though I'm not sure why the bug didn't start happening until a few days after that patch landed.  Maybe it was exposed by some other patch.

Comment 11

[Matt Brubeck (:mbrubeck)]

Comment on attachment 643052 [details] [diff] [review]
patch

https://hg.mozilla.org/integration/mozilla-inbound/rev/6312d382ffc3

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Regression from bug 753102

User impact if declined: No way to exit full-screen mode after entering it.

Testing completed (on m-c, etc.): Landed on inbound 7/18

Risk to taking this patch (and alternatives if risky): This is a very low-risk mobile-only patch that just moves some code back onto the UI thread, after it was accidentally moved off of the UI thread.  This code is only run when entering and exiting full screen on Android.

String or UUID changes made by this patch: None.

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/6312d382ffc3

Comment 14

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0565f384e382
https://hg.mozilla.org/releases/mozilla-beta/rev/62dc8862639c

Comment 15

[Adrian Tamas (:AdrianT)]

Unable to reproduce the issue on:

Firefox Mobile 16.0b5
Samsung Galaxy R (Android 2.3.4)

Marking as verified on Firefox Mobile 16