Closed
Bug 1031519
Opened 9 years ago
Closed 7 years ago
(immersive fullscreen) Hide soft buttons on fullscreen request, show them on swipe.
Categories
(Firefox for Android Graveyard :: Theme and Visual Design, enhancement)
Tracking
(firefox50 fixed)
RESOLVED
FIXED
Firefox 50
Tracking | Status | |
---|---|---|
firefox50 | --- | fixed |
People
(Reporter: bugs, Assigned: twointofive, Mentored)
References
Details
(Whiteboard: [good first bug])
Attachments
(1 file, 3 obsolete files)
Currently https://developer.mozilla.org/en-US/docs/Web/API/Element.requestFullScreen preserves the soft buttons at the bottom of the page. Reasonable, since you need to be able to get out. Apparently the stock browser shows them on swipe, which is rather nice for, say, a truly fullscreen video. Ideally, it'd be nice if this API had some way to flag "always show" vs "show on swipe" but since that doesn't exist, "show on swipe" is nicer since you can always allow space for the buttons in your app so that they don't interrupt gameplay, if you need that.
Updated•9 years ago
|
Component: Graphics, Panning and Zooming → Theme and Visual Design
Almost one year later and this simple, yet big improvement for the user experience, hasn't even been addressed in any way other than with a component classification and duplicate redirection.
For further comparison, following are the Fullscreen behaviors compared between Chrome, Opera and Firefox: - Chrome: Video goes into fullscreen, statusbar and navigation keys hide automatically. Swipe down from top displays these controls. http://i.cubeupload.com/e79NfO.png - Opera: Video goes into fullscreen, statusbar and navigation keys hide automatically. Swipe down from top displays these controls. http://i.cubeupload.com/wVG1jQ.png - Firefox: Video goes into fullscreen, statusbar hides, but the navigation menu remains visible at all times, unable to hide by any means. http://i.cubeupload.com/E47aSV.png Unable to play YouTube videos as a result of the lack of Android version in the UAS (user agent string), YouTube cannot tell if the Android handset is capable if playing its videos because of this missing vital piece of information. Because of this I had to use a different video in the example. Firefox is the only one that does not have its Fullscreen api working correctly, it should follow the instictive behavior that is present in the other browsers and apps that offer similar content.
Comment 4•8 years ago
|
||
Seems like what you have to do is to change this line [1] from > View.SYSTEM_UI_FLAG_LOW_PROFILE to > View.SYSTEM_UI_FLAG_HIDE_NAVIGATION But KitKat introduced some new, useful features, so right below that line we should add, > if (Versions.feature19Plus) { > newVis |= View.SYSTEM_UI_FLAG_LAYOUT_STABLE | > View.SYSTEM_UI_FLAG_LAYOUT_FULLSCREEN | > View.SYSTEM_UI_FLAG_LAYOUT_HIDE_NAVIGATION | > View.SYSTEM_UI_FLAG_IMMERSIVE_STICKY; > } (newVis should be made non-final) [1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/util/ActivityUtils.java?rev=abe43c30d78d#27
Updated•8 years ago
|
Mentor: michael.l.comella, nchen
Whiteboard: [good first bug]
Comment 5•8 years ago
|
||
Hi,I want to work on this bug as my first bug.Could anyone help me with the build process and the location of this particular piece of code.Thanks!
Comment 6•8 years ago
|
||
Made newVis non - final and added the respective line for feature19Plus. Kindly review this patch and suggest modification
Comment 7•8 years ago
|
||
Ignore the previous attachment. Have attached the correct patch. Made newVis non - final and added the respective line for feature19Plus. Kindly review this patch and suggest modification
Attachment #8706296 -
Attachment is obsolete: true
Comment 8•7 years ago
|
||
Comment on attachment 8713984 [details] [diff] [review] ActivityUtils_v1.java ># HG changeset patch ># User Deepthi Venkitaramanan <deepthivenkitaramanan@gmail.com> ># Parent 5acc2a44834ce0614f98466475e674517daf0041 >Bug 1031519 - made newVis non final.In kitkat, for Version 19Plus, newViz = View.SYSTEM_UI_FLAG_HIDE_NAVIGATION instead of View.SYSTEM_UI_FLAG_LOW_PROFILE > >diff --git a/mobile/android/base/java/org/mozilla/gecko/util/ActivityUtils.java b/mobile/android/base/java/org/mozilla/gecko/util/ActivityUtils.java >--- a/mobile/android/base/java/org/mozilla/gecko/util/ActivityUtils.java >+++ b/mobile/android/base/java/org/mozilla/gecko/util/ActivityUtils.java >@@ -16,20 +16,26 @@ > private ActivityUtils() { > } > > public static void setFullScreen(Activity activity, boolean fullscreen) { > // Hide/show the system notification bar > Window window = activity.getWindow(); > > if (Versions.feature16Plus) { >- final int newVis; >+ int newVis; > if (fullscreen) { > newVis = View.SYSTEM_UI_FLAG_FULLSCREEN | > View.SYSTEM_UI_FLAG_LOW_PROFILE; >+ if (Versions.feature19Plus) { >+ newVis |= View.SYSTEM_UI_FLAG_LAYOUT_STABLE | >+ View.SYSTEM_UI_FLAG_LAYOUT_FULLSCREEN | >+ View.SYSTEM_UI_FLAG_LAYOUT_HIDE_NAVIGATION | >+ View.SYSTEM_UI_FLAG_IMMERSIVE_STICKY; >+ } > } else { > newVis = View.SYSTEM_UI_FLAG_VISIBLE; > } > > window.getDecorView().setSystemUiVisibility(newVis); > } else { > window.setFlags(fullscreen ? > WindowManager.LayoutParams.FLAG_FULLSCREEN : 0,
Attachment #8713984 -
Flags: review?(nchen)
Attachment #8713984 -
Flags: review?(michael.l.comella)
Comment 9•7 years ago
|
||
Comment on attachment 8713984 [details] [diff] [review] ActivityUtils_v1.java Review of attachment 8713984 [details] [diff] [review]: ----------------------------------------------------------------- I think mcomella should be reviewing this.
Attachment #8713984 -
Flags: review?(nchen)
Comment 10•7 years ago
|
||
Comment on attachment 8713984 [details] [diff] [review] ActivityUtils_v1.java Review of attachment 8713984 [details] [diff] [review]: ----------------------------------------------------------------- Can you explain how these layout flags combine to produce the desired output? I would expect `View.SYSTEM_UI_FLAG_HIDE_NAVIGATION`, not `View.SYSTEM_UI_FLAG_LAYOUT_HIDE_NAVIGATION`.
Updated•7 years ago
|
Assignee: nobody → deepthivenkitaramanan
Updated•7 years ago
|
Flags: needinfo?(deepthivenkitaramanan)
Comment 11•7 years ago
|
||
Only after I read you comment, I realized that i misunderstood what jchen said. I am attaching the updated patch here. please review the same.
Flags: needinfo?(deepthivenkitaramanan)
Comment 12•7 years ago
|
||
Attachment #8713984 -
Attachment is obsolete: true
Attachment #8713984 -
Flags: review?(michael.l.comella)
Attachment #8722811 -
Flags: review?(michael.l.comella)
Comment 13•7 years ago
|
||
Sorry for the delay, Deepthi – I'll get to this tomorrow.
Comment 14•7 years ago
|
||
Related: I've been wanting to clean up the fullscreen interactions for a while. See bug 1137283.
See Also: → 1137283
Comment 15•7 years ago
|
||
Comment on attachment 8722811 [details] [diff] [review] hide_soft_buttons.patch Review of attachment 8722811 [details] [diff] [review]: ----------------------------------------------------------------- I tried to make sense of these flags using the documentation but I'm resolved to the fact that they make no sense. I found [1] which provides the layout flags you mentioned so I'm good on the added api 19+ implementation. However, on pre-19, when the user taps the screen to reintroduce the controls, they don't go away after some time period. This means with the new configuration, the video size will annoyingly change and not change back when the user taps the screen (e.g. accidentally). However, in LOW_PROFILE, the controls will stay obscured but accessible and there won't be an annoying resize, which I think feels more polished. Can we set FULLSCREEN + LOW_PROFILE on 16-19, and the new immersive mode on 19+? btw, sorry this took so long, Deepthi. I'll try to be better in the future. [1]: https://developer.android.com/training/system-ui/immersive.html#sticky ::: mobile/android/base/java/org/mozilla/gecko/util/ActivityUtils.java @@ +25,4 @@ > if (fullscreen) { > newVis = View.SYSTEM_UI_FLAG_FULLSCREEN | > + View.SYSTEM_UI_FLAG_HIDE_NAVIGATION; > + if (Versions.feature19Plus) { nit: Can we indent this to the same level of newVis?
Attachment #8722811 -
Flags: review?(michael.l.comella) → feedback+
Updated•7 years ago
|
Summary: Hide soft buttons on fullscreen request, show them on swipe. → (immersive fullscreen) Hide soft buttons on fullscreen request, show them on swipe.
Comment 18•7 years ago
|
||
hey, I want to fix this bug. this'll be my first bug. what do i do from here?
Flags: needinfo?(michael.l.comella)
Comment 19•7 years ago
|
||
hey, I want to fix this bug. this'll be my first bug. what do i do from here?
Comment 20•7 years ago
|
||
(In reply to amol1994mane from comment #19) > hey, I want to fix this bug. this'll be my first bug. what do i do from here? Hi, amol. We generally recommend having one bug at a time for new contributors. It looks like you've been making progress in bug 1262285 – would you also like to look at bug 1081437? Comment in that bug if you'd like to try it.
Flags: needinfo?(michael.l.comella)
Comment 21•7 years ago
|
||
The results on my Amazon Fire (5g, 7") do not align with previous statements. Soft buttons do actually hide; however, space for them is left behind. AND the Play/seek controls are present and will not hide. (The seek bar just gets a bit smaller, like how th old YouTube player worked on Desktop when not full screen.) Two workarounds to fix the Play/Seek controls, but not the soft buttons. First is to switch to a Chrome User-Agent. The downside is that I get the default HTML5 video controls instead of the YouTube ones--hence no CC or gear icon for the setting resolution. Second is to switch to Desktop mode. This keeps the YouTube video controls. The downside is that, if I use those controls, I must then tap elsewhere for them to hide. The show/hide animation is also glitchy. Both still have the problem that space for the soft buttons is still present.
Assignee | ||
Comment 22•7 years ago
|
||
This is Deepthi Venkitaramanan's patch with feedback comments addressed. Review commit: https://reviewboard.mozilla.org/r/65224/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/65224/
Attachment #8772395 -
Flags: review?(michael.l.comella)
Updated•7 years ago
|
Attachment #8772395 -
Flags: review?(michael.l.comella) → review+
Comment 23•7 years ago
|
||
Comment on attachment 8772395 [details] Bug 1031519 - Use immersive fullscreen when available. https://reviewboard.mozilla.org/r/65224/#review63060 I tested this on html5demos.com/video GS4 (4.4 w/ hardware buttons, API 19), Galaxy Nexus (4.3), & Nexus 5 (4.4+) – it performs my expected behavior. Galaxy Nexus: soft buttons are dimmed but visible. Tapping the screen does not make them visible but hitting back will exit fullscreen mode. Swiping from the top does nothing. GS4: no soft buttons so video is fullscreen. Hardware back can be used to exit fullscreen. Swiping from the top will make the notification bar appear, but it will shortly disappear. N5: Fullscreen video, no soft buttons. Swiping from the top will make the notification & navigation bars appear temporarily. When the navigation bar appears, hitting back will leave fullscreen. - I feel like I'd prefer the behavior of tapping the screen to make the video controls appear but it does come at the cost of taking up screen real estate. However, looking at the Youtube video player, it provides a leave-fullscreen button on tap, which fixes my concern. Perhaps, like the Youtube case, we should rely on the video provider to provide a fullscreen exit option in the player on tap.
Comment 25•7 years ago
|
||
I made a push to our try test servers (above). Once the push goes green, you can add the "checkin-needed" keyword [1] to get your patch checked in. Note that all patches added via checkin-needed keyword need an associated green try run. Let me know if you need help reading the results. Thanks Tom! [1]: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Getting_the_patch_checked_into_the_tree
Updated•7 years ago
|
Assignee: nobody → twointofive
Updated•7 years ago
|
Attachment #8722811 -
Attachment is obsolete: true
Comment 26•7 years ago
|
||
(In reply to Terrell Kelley from comment #21) > The results on my Amazon Fire (5g, 7") do not align with previous statements. This is without the patch applied, right? The new patch should remove the soft buttons on devices with 4.4+ (see comment 23). Let me (or Tom! :D) know if you still have issues.
Comment 27•7 years ago
|
||
Pushed by twointofive@gmail.com: https://hg.mozilla.org/integration/autoland/rev/830a8be8be2a Use immersive fullscreen when available. r=mcomella
Comment 28•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/830a8be8be2a
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•