Closed Bug 1054126 Opened 10 years ago Closed 10 years ago

Support "fullscreen_layout" field in manifest

Categories

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v2.1 fixed)

RESOLVED FIXED
2.1 S3 (29aug)
Tracking Status
b2g-v2.1 --- fixed

People

(Reporter: mikehenrty, Assigned: gduan)

References

Details

(Keywords: dev-doc-needed, Whiteboard: [systemsfe])

Attachments

(1 file, 1 obsolete file)

To quote an email from Vivien: "The first thing I would like to discuss and introduced is a 'fullscreen_layout' flags in the manifest. It let applications be sized at 100% or the width and 100% of the height of the device, ignoring any chrome UI that may cover it." We need this for bug 1037255 in order to allow video applications to underlay the software home button, and to be able to show/hide this button without changing the aspect ratio of the displayed video.
Could you take this one? Thanks.
Flags: needinfo?(gduan)
Assignee: alive → gduan
Flags: needinfo?(gduan)
No longer blocks: 1037255
Attached file PR to master (obsolete) (deleted) —
WIP, 1. add fullscreen_layout 2. modify css
Whiteboard: [systemsfe]
Attached file PR to master (deleted) —
Hi Alive, please let me know if this approach is ok. I've tried to rebase on https://github.com/etiennesegonzac/gaia/compare/softbutton-media-experiment and it works fine.
Attachment #8475661 - Flags: feedback?(alive)
Attachment #8474989 - Attachment is obsolete: true
(In reply to George Duan [:gduan] [:喬智] from comment #3) > Created attachment 8475661 [details] > PR to master > > Hi Alive, > please let me know if this approach is ok. I've tried to rebase on > https://github.com/etiennesegonzac/gaia/compare/softbutton-media-experiment > and it works fine. Don't worry about Etienne's proof of concept. We are no longer taking that approach. Your patch will make it possible to do bug 1055198 without using media events.
Comment on attachment 8475661 [details] PR to master I think you need to change layoutManager as well.
Attachment #8475661 - Flags: feedback?(alive) → feedback+
Comment on attachment 8475661 [details] PR to master update layoutManager and test. could you review it again? Thanks.
Attachment #8475661 - Flags: review?(alive)
I've looked a bit at the PR and to me it seems like a bad idea to cover the keyboard, permission screen etc. with the soft home button. If we take the video app as an example, in my understanding the app would only call mozRequestFullScreen (and thus hide the soft home button) when you play a video. Let's say that you could edit the video name in the list view (or some other action that shows the keyboard), in that case parts of the keyboard would be covered by the soft home button.
(In reply to Markus Nilsson from comment #7) > I've looked a bit at the PR and to me it seems like a bad idea to cover the > keyboard, permission screen etc. with the soft home button. From the UX perspective, the soft home button and bar should never cover the keyboard. Nor should the keyboard appear on top of the soft home button and bar. I tend to think of the soft home button / bar as part of the hardware as opposed to part of the display. The only exception would be in full-screen video playback where the soft home button bar appears above the video playback and video is partially visible through the bar when it is displayed. But the keyboard and dialogs should not use the home button bar space nor should they transition over the home button bar space. NI me if this doesn't make sense. :)
(In reply to Markus Nilsson from comment #7) > I've looked a bit at the PR and to me it seems like a bad idea to cover the > keyboard, permission screen etc. with the soft home button. > > If we take the video app as an example, in my understanding the app would > only call mozRequestFullScreen (and thus hide the soft home button) when you > play a video. Let's say that you could edit the video name in the list view > (or some other action that shows the keyboard), in that case parts of the > keyboard would be covered by the soft home button. I agree, thankfully it should be an easy change :) George, to clarify: an app opting in for the fullscreen_layout is asking for the software home button to be overlayed on top of the app frame (we should probably put the #software-home-buttons into opacity:0.5 in this case BTW). But in the system app we should still make sure that the various system element (soft home, keyboard, prompts) won't overlap.
Comment on attachment 8475661 [details] PR to master Looks like there is something I never know about fullscreen_layout spec... George and Etienne, could you co-work here?
Attachment #8475661 - Flags: review?(alive) → review?(etienne)
Comment on attachment 8475661 [details] PR to master Commented on the PR, mainly about removing code which is nice :) There's one issue left: when you lock the phone while a fullscreen_layout app is open the lockscreen isn't displayed correctly. Alive what's the best way to handle this case? layoutManager checking System.locked? Will System.locked be set early enough? Also, we probably don't want to switch the video app to fullscreen_layout as part of this patch (that's bug 1055198).
Attachment #8475661 - Flags: review?(etienne)
Flags: needinfo?(alive)
(In reply to Etienne Segonzac (:etienne) from comment #11) > Comment on attachment 8475661 [details] > PR to master > > Commented on the PR, mainly about removing code which is nice :) > > There's one issue left: when you lock the phone while a fullscreen_layout > app is open the lockscreen isn't displayed correctly. > > Alive what's the best way to handle this case? layoutManager checking > System.locked? Will System.locked be set early enough? > > Also, we probably don't want to switch the video app to fullscreen_layout as > part of this patch (that's bug 1055198). System.locked is true when system app has the lockscreen-request-unlock event from the unlock handle. What's in my mind: * As you said check lockscreen window state(system.locked) in layout manager * Check "top level window"(it's lockscreen window now) state(isFullScreenLayout) in layout manager Both of them needs to touch window management so I don't think which is perfect. We have a plan to implement hierarchy manager to reflect the top level window(that is to say, not app window only) in long term.
Flags: needinfo?(alive)
Hey George, anything I can do to help? :) There's a bit of pressure on this bug but your patch is almost ready!
Flags: needinfo?(gduan)
> > System.locked is true when system app has the lockscreen-request-unlock > event from the unlock handle. > What's in my mind: > * As you said check lockscreen window state(system.locked) in layout manager > * Check "top level window"(it's lockscreen window now) When I try to getTopMostWindow() in lockscreen, the result is still video app. It might be a bug. I will use the first option. > state(isFullScreenLayout) in layout manager > > Both of them needs to touch window management so I don't think which is > perfect. We have a plan to implement hierarchy manager to reflect the top > level window(that is to say, not app window only) in long term.
> System.locked is true when system app has the lockscreen-request-unlock > event from the unlock handle. > What's in my mind: > * As you said check lockscreen window state(system.locked) in layout manager When screen off, I see no one call lockscreenWindow._resize, so the layoutManager.height will not affect the size of lockScreen.window, unless we do resize when in lockscreenWindowManager.openApp.
Flags: needinfo?(gduan)
Comment on attachment 8475661 [details] PR to master Hi Etienne, could you check my approach again? Besides what you addressed in github, I also call resize in lockscreenWindowManager.openApp, that will help us to get height of layoutManager. Sorry for merging two commits..
Attachment #8475661 - Flags: review?(etienne)
[Blocking Requested - why for this release]: This is blocking a blocker: bug 1055198.
blocking-b2g: --- → 2.1?
Comment on attachment 8475661 [details] PR to master Almost there! A few comments on github.
Attachment #8475661 - Flags: review?(etienne)
Comment on attachment 8475661 [details] PR to master Hi Etienne, thanks for your patience, could you check 2nd commit again? Thanks.
Attachment #8475661 - Flags: review?(etienne)
Comment on attachment 8475661 [details] PR to master Thanks for the quick turn around! Small naming issue left and we need to address this comment: https://github.com/mozilla-b2g/gaia/pull/23021#issuecomment-53550400
Attachment #8475661 - Flags: review?(etienne)
Hi Etienne, I've updated modal dialog and time/date/value-selector, but I think we should also update context_menu and other css style?
Flags: needinfo?(etienne)
Comment on attachment 8475661 [details] PR to master Hi Etienne, I update most of dialog, please check 3nd commit, Thanks.
Attachment #8475661 - Flags: review?(etienne)
Comment on attachment 8475661 [details] PR to master Looks like the PR doesn't have the last live changes from this morning on IRC. But with those + the small nits on the PR addressed we're good to go! Thank you so much for the quick turnaround!
Attachment #8475661 - Flags: review?(etienne) → review+
Flags: needinfo?(etienne)
We got the r+, we got a green try run: https://tbpl.mozilla.org/?rev=3cb119a0e7502d604c9b7510f18d20f94e7dff01&tree=Gaia-Try Great job, George! I'm going to go ahead and land this, since bug 1055198 depends on it and I'd love to have this "baking" on master for a day or so before FL.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
blocking-b2g: 2.1? → ---
Depends on: 1060693
Depends on: 1064327
Depends on: 1064648
Unable to verify because this is a backend code change.
QA Whiteboard: [QAnalyst-verify-]
QA Whiteboard: [QAnalyst-verify-] → [QAnalyst-verify-][QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-verify-][QAnalyst-Triage?] → [QAnalyst-verify-][QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: