700678 - Exit full screen mode when the app goes into the background

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P4)

Description

[:Margaret Leibovic]

Follow up from bug 688082 comment 9:

Looking at the ways we exit fullscreen mode on desktop makes me think we should add a few more to mobile:
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#3915

Places we could toggle out of fullscreen mode:
* In GeckoApp.onPause  (app is deactivated)
* In GeckoApp.handleAddTab, handleCloseTab and handleSelectTab ("TabOpen", "TabClose" and "TabSelect" events in desktop)
* In GeckoApp.onPrepareOptionsMenu when the menu is about to be shown (?? not sure about this one)

Comment 1

[:Margaret Leibovic]

Revisiting this bug, almost everything I said in comment 0 is out of date. I'm not sure what I was thinking about tab events, because I'm not sure how we could be encountering these tab events while in full screen mode. We have much more limited UI than desktop, so this might be a case where we don't need to do as much as desktop.

However, I think it does make sense to exit full screen mode when the application goes into the background, since it can be confusing if you come back and you forgot that you were in full screen mode. Also, if you're in full screen mode, put the app in the background, then open a link from another app, that new tab will be in full screen mode, which is even more confusing. 

To test full screen mode, I usually just put this video in full screen mode:
http://clips.vorwaerts-gmbh.de/big_buck_bunny.webm

Comment 2

[Connor Roberts]

I agree with the fact the full screen should be exited when the app goes to the background :)

Comment 4

[pushkar.singh93]

Hi, I am a new contributor and I'm interested in working on this bug. If nobody else is working then I am eager to try my hands on this with some guidance. Can u give me some clue on how to start? A lot thanks!

Comment 5

[:Margaret Leibovic]

(In reply to pushkar.singh93 from comment #4)
> Hi, I am a new contributor and I'm interested in working on this bug. If
> nobody else is working then I am eager to try my hands on this with some
> guidance. Can u give me some clue on how to start? A lot thanks!

Welcome! First of all, do you have a development environment set up for building fennec? If not, you should follow the directions here:
https://wiki.mozilla.org/Mobile/Fennec/Android#Building_Fennec

To write a patch for this bug, you're going to need to add some code that exits full screen mode when we go into the background. I marked this bug as a Java bug, but actually this can all be done on the JS side of things. We already listen for the application going into the background here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#6412

In there, we can check to see if the currently selected tab is in fullscreen mode, and if so, we should exit it. To do this, you'll need the selected tab's content document, which you can get at with BrowserApp.selectedTab.browser.contentDocument. From there, you can check fullscreen mode with document.mozFullScreen, and exit fullscreen mode with document.mozCancelFullScreen().

I would encourage you to use mxr.mozilla.org to search around for code related to "fullscreen" to get a sense of how this all works.

Also, this will only exit DOM fullscreen mode, not fullscreen plugins, so we may want to file a separate bug for that.

Comment 6

[:Margaret Leibovic]

Also, please feel free to ask questions in #mobile on irc.mozilla.org. There's usually a bunch of friendly developers hanging out in there!

Comment 7

[pushkar.singh93]

I built the code and its working fine, I am now going to tackle the main bug.

Comment 8

[pushkar.singh93]

Hi!, I wanted to ask when can I talk to you over IRC. I think IRC will be great for clearing small doubts and queries.

Comment 9

[:Margaret Leibovic]

(In reply to pushkar.singh93 from comment #8)
> Hi!, I wanted to ask when can I talk to you over IRC. I think IRC will be
> great for clearing small doubts and queries.

For those following along, we worked this out on IRC :)

Comment 10

[pushkar.singh93]

Attachment: Bug 700678 : Escaping FullScreen when app goes in background

Checking the condition for app, when in background and having full Screen mode then we exit full-screen mode for the app.

Comment 11

[:Margaret Leibovic]

Comment on attachment 676741 [details] [diff] [review]
Bug 700678 : Escaping FullScreen when app goes in background

This looks like it's on the right track, but there are a few issues. First of all, just using |document| isn't guaranteed to give you the right document. You want to check the content document of the currently selected tab. To do that, you can just declare a local variable, like so:

let doc = BrowserApp.selectedTab.browser.contentDocument;

Then use |doc| instead of |document|.

Secondly, it appears your patch isn't formatted correctly, so I couldn't apply it to my tree. You should read these pages about making a patch:
https://developer.mozilla.org/en-US/docs/Creating_a_patch
https://developer.mozilla.org/en-US/docs/Creating_a_patch_that_can_be_checked_in

Comment 12

[pushkar.singh93]

Attachment: Bug 700678 - Exit full screen mode when the app goes into the background

This patch contains all the required changes.

Comment 13

[pushkar.singh93]

I am really sorry for the last patch, actually I instead of using Hg for diff just directly used the shell command diff to generate the universal patch. Sorry for the mistake.

Comment 14

[:Margaret Leibovic]

Comment on attachment 676972 [details] [diff] [review]
Bug 700678 - Exit full screen mode when the app goes into the background

This is great, thanks!

Comment 15

[:Margaret Leibovic]

I was going to land this for you, but mozilla-inbound is closed at the moment.

To land your patch, you can also upload a new version of the patch with "r=margaret" added to the end of the commit message, then add "checkin-needed" to the keyword field, and then a kind soul should come by and land the patch for you :)

Comment 16

[pushkar.singh93]

Attachment: checkin-needed

The Final Patch!

Comment 17

[:Margaret Leibovic]

Pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/840a9b8ec6e1

This will be merged to mozilla-central later, and it will be part of Firefox 19. Thanks!

Comment 18

[pushkar.singh93]

Amazing! Thank You!

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/840a9b8ec6e1

Comment 20

[Cristian Nicolae (:xti)]

The app exits from full screen mode when it's moved to background. However, the app will freeze when it's reopened (bug 809055).

Closing bug as verified fixed on:

Firefox 19.0a1 (2012-11-06)
Device: Galaxy S2
OS: Android 4.0.3

Comment 21

[Ioana Chiorean]

A TC was added in MozTrap for this on 19 phones, 19 tablets, 20 phones, 20 tablets

https://moztrap.mozilla.org/manage/case/5945