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)
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.
Assignee | ||
Updated•10 years ago
|
Assignee: alive → gduan
Flags: needinfo?(gduan)
Assignee | ||
Comment 2•10 years ago
|
||
WIP,
1. add fullscreen_layout
2. modify css
Updated•10 years ago
|
Whiteboard: [systemsfe]
Assignee | ||
Comment 3•10 years ago
|
||
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)
Assignee | ||
Updated•10 years ago
|
Attachment #8474989 -
Attachment is obsolete: true
Reporter | ||
Comment 4•10 years ago
|
||
(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 5•10 years ago
|
||
Comment on attachment 8475661 [details]
PR to master
I think you need to change layoutManager as well.
Attachment #8475661 -
Flags: feedback?(alive) → feedback+
Assignee | ||
Comment 6•10 years ago
|
||
Comment on attachment 8475661 [details]
PR to master
update layoutManager and test. could you review it again? Thanks.
Attachment #8475661 -
Flags: review?(alive)
Comment 7•10 years ago
|
||
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.
Comment 8•10 years ago
|
||
(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. :)
Comment 9•10 years ago
|
||
(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 10•10 years ago
|
||
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 11•10 years ago
|
||
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)
Comment 12•10 years ago
|
||
(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)
Comment 13•10 years ago
|
||
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)
Assignee | ||
Comment 14•10 years ago
|
||
>
> 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.
Assignee | ||
Comment 15•10 years ago
|
||
> 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)
Assignee | ||
Comment 16•10 years ago
|
||
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)
Comment 17•10 years ago
|
||
[Blocking Requested - why for this release]: This is blocking a blocker: bug 1055198.
blocking-b2g: --- → 2.1?
Reporter | ||
Updated•10 years ago
|
Keywords: dev-doc-needed
Comment 18•10 years ago
|
||
Comment on attachment 8475661 [details]
PR to master
Almost there! A few comments on github.
Attachment #8475661 -
Flags: review?(etienne)
Assignee | ||
Comment 19•10 years ago
|
||
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 20•10 years ago
|
||
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)
Assignee | ||
Comment 21•10 years ago
|
||
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)
Assignee | ||
Comment 22•10 years ago
|
||
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 23•10 years ago
|
||
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)
Reporter | ||
Comment 24•10 years ago
|
||
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.
Reporter | ||
Comment 25•10 years ago
|
||
Updated•10 years ago
|
blocking-b2g: 2.1? → ---
Comment 26•10 years ago
|
||
Unable to verify because this is a backend code change.
QA Whiteboard: [QAnalyst-verify-]
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-verify-] → [QAnalyst-verify-][QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Updated•10 years ago
|
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.
Description
•