Last Comment Bug 700678 - Exit full screen mode when the app goes into the background
: Exit full screen mode when the app goes into the background
Status: VERIFIED FIXED
[mentor=margaret][lang=js]
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All Android
: P4 normal (vote)
: Firefox 19
Assigned To: pushkar.singh93
:
Mentors:
: 800879 (view as bug list)
Depends on: 688082
Blocks: 800673
  Show dependency treegraph
 
Reported: 2011-11-08 08:45 PST by :Margaret Leibovic
Modified: 2012-12-17 09:53 PST (History)
11 users (show)
ryanvm: in‑testsuite-
ioana.chiorean: in‑moztrap+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
verified


Attachments
Bug 700678 : Escaping FullScreen when app goes in background (426 bytes, patch)
2012-10-30 13:13 PDT, pushkar.singh93
margaret.leibovic: review-
Details | Diff | Review
Bug 700678 - Exit full screen mode when the app goes into the background (1.02 KB, patch)
2012-10-31 04:40 PDT, pushkar.singh93
margaret.leibovic: review+
Details | Diff | Review
checkin-needed (1.03 KB, patch)
2012-10-31 12:05 PDT, pushkar.singh93
no flags Details | Diff | Review

Description :Margaret Leibovic 2011-11-08 08:45:59 PST
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 2012-10-10 03:16:28 PDT
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 2012-10-10 14:00:48 PDT
I agree with the fact the full screen should be exited when the app goes to the background :)
Comment 3 Aaron Train [:aaronmt] 2012-10-12 07:27:23 PDT
*** Bug 800879 has been marked as a duplicate of this bug. ***
Comment 4 pushkar.singh93 2012-10-24 17:08:59 PDT
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 2012-10-25 10:10:26 PDT
(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 2012-10-25 10:17:26 PDT
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 2012-10-26 18:56:30 PDT
I built the code and its working fine, I am now going to tackle the main bug.
Comment 8 pushkar.singh93 2012-10-29 13:14:58 PDT
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 2012-10-29 13:59:07 PDT
(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 2012-10-30 13:13:55 PDT
Created attachment 676741 [details] [diff] [review]
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 2012-10-30 15:52:00 PDT
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 2012-10-31 04:40:05 PDT
Created attachment 676972 [details] [diff] [review]
Bug 700678 - Exit full screen mode when the app goes into the background

This patch contains all the required changes.
Comment 13 pushkar.singh93 2012-10-31 08:12:39 PDT
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 2012-10-31 11:39:01 PDT
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 2012-10-31 11:44:52 PDT
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 2012-10-31 12:05:24 PDT
Created attachment 677106 [details] [diff] [review]
checkin-needed

The Final Patch!
Comment 17 :Margaret Leibovic 2012-11-01 09:44:49 PDT
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 2012-11-01 11:00:28 PDT
Amazing! Thank You!
Comment 19 Ryan VanderMeulen [:RyanVM] 2012-11-01 18:36:34 PDT
https://hg.mozilla.org/mozilla-central/rev/840a9b8ec6e1
Comment 20 Cristian Nicolae (:xti) 2012-11-06 08:26:54 PST
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 2012-12-17 09:53:36 PST
A TC was added in MozTrap for this on 19 phones, 19 tablets, 20 phones, 20 tablets

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

Note You need to log in before you can comment on or make changes to this bug.