Closed Bug 698836 Opened 13 years ago Closed 13 years ago

Add full screen mode

Categories

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

All
Android
defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: Margaret, Assigned: Margaret)

References

Details

(Whiteboard: [QA+])

Attachments

(1 file, 2 obsolete files)

We should use the new fullscreen API for this. (This bug depends on bug 695460, since we would add a context menu item to get to full screen mode.)
Assignee: nobody → margaret.leibovic
Priority: -- → P4
Attached patch wip (obsolete) (deleted) — Splinter Review
This generally works, but it has problems if you leave Fennec while you're in full screen mode, so I need to add something that deals with that case.
Attachment #572132 - Flags: feedback?(blassey.bugs)
(This patch is written on top of the patch in bug 699667)
Depends on: 699667
Comment on attachment 572132 [details] [diff] [review] wip Review of attachment 572132 [details] [diff] [review]: ----------------------------------------------------------------- mostly good, but hiding chrome whenever we go into fullscreen needs to be fixed for this to land ::: embedding/android/GeckoApp.java @@ +991,5 @@ > + WindowManager.LayoutParams.FLAG_FULLSCREEN : 0, > + WindowManager.LayoutParams.FLAG_FULLSCREEN); > + > + // Hide/show the browser toolbar > + mBrowserToolbar.setVisibility(fullscreen ? View.GONE : View.VISIBLE); I don't think we should be hiding chrome here. Instead, browser.js should be listening for the fullscreen event (see https://mxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp#4486, http://mxr.mozilla.org/mozilla-central/source/mobile/chrome/content/browser.js#330 and mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#1671) and that handler should send a "Window:HideChrome" message to java to execute this code.
Attachment #572132 - Flags: feedback?(blassey.bugs) → feedback-
Attached patch patch (obsolete) (deleted) — Splinter Review
Attachment #572132 - Attachment is obsolete: true
Attachment #572665 - Flags: review?(blassey.bugs)
Comment on attachment 572665 [details] [diff] [review] patch there are two actions happening here, let's not get them crossed up. You're first patch was basically right for handling SetFullscreen on the window, just remove the "mBrowserToolbar.setVisibility(mFullScreen ? View.VISIBLE : View.GONE);" line. For the fullscreen event on the document (unfortunately, it is confusingly named), this essentially means "hide the chrome". You're second patch essentially handles that correctly, but it should only call "mBrowserToolbar.setVisibility(mFullScreen ? View.VISIBLE : View.GONE);" in that handler. Also, to try to limit the confusion, could you name the message sent for the document going fullscreen to "ToggleChrome:Hide/Show" or something along those lines. We loose the context of it being related to the document when it becomes a message sent through this interface.
Attachment #572665 - Flags: review?(blassey.bugs) → review-
Attached patch patch v2 (deleted) — Splinter Review
I hope I understand this now!
Attachment #572665 - Attachment is obsolete: true
Attachment #572679 - Flags: review?(blassey.bugs)
Attachment #572679 - Flags: review?(blassey.bugs) → review+
Blocks: 688082
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Flags: in-litmus?(fennec)
Whiteboard: [QA+]
These patches were backed while investigating Talos failures. Now that tests are green again, we will need to reland.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
20111114041052 http://hg.mozilla.org/projects/birch/rev/859ecdfe0168 Samsung Galaxy SII (Android 2.3.4)
Status: RESOLVED → VERIFIED
Added testcase for Fennec 11.0 Catch-All Test Run and Fennec 12.0 Catch-All Test Run : - https://litmus.mozilla.org/show_test.cgi?id=44849 - https://litmus.mozilla.org/show_test.cgi?id=44850
Flags: in-litmus?(fennec) → in-litmus+
Depends on: 719557
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: