Closed
Bug 1352009
Opened 8 years ago
Closed 8 years ago
Investigate whether we need to do anything to support larger aspect ratios
Categories
(Firefox for Android Graveyard :: General, defect, P1)
Tracking
(fennec+, firefox53+ wontfix, firefox54+ verified, firefox55+ verified)
VERIFIED
FIXED
Firefox 55
People
(Reporter: sebastian, Assigned: mkaply)
References
Details
Attachments
(3 files)
153.78 KB,
image/png
|
Details | |
59 bytes,
text/x-review-board-request
|
sebastian
:
review+
gchang
:
approval-mozilla-aurora-
gchang
:
approval-mozilla-beta+
Sylvestre
:
approval-mozilla-release-
|
Details |
235.03 KB,
image/png
|
Details |
Google published this confusing blog post:
https://android-developers.googleblog.com/2017/03/update-your-app-to-take-advantage-of.html
From the posting it's not clear to me whether we should add something like this to our manifest:
<meta-data android:name="android.max_aspect" android:value="2.1" />
The posting also mentions "android:resizeableActivity" - but we can't set this without using the 24/25/26 SDK first. This is something we should set for multiscreen anyways - as soon as we can.
Comment 1•8 years ago
|
||
This is Firefox for Android on a Samsung Galaxy S8 with an aspect ratio of 18.5:9, in FHD+ resolution (2220x1080). The black bars at the top and bottom of the screen are there in both other resolutions supported by the device. Some other apps (such as Chrome) don't have those black bars.
Reporter | ||
Comment 4•8 years ago
|
||
We added a max_aspect value of 2.0 in bug 1345014. Seems like we want to use an even higher value for now. And later enable resizeableActivity (after bug 1352015).
See Also: → 1345014
Assignee | ||
Comment 5•8 years ago
|
||
Yeah, it appears we just need to keep upping that value. Weird.
I wonder if we should just set this to some comical value like like 5. I think I'd rather see at worst a slightly-busted UI than these black bars.
Flags: needinfo?(snorp)
Assignee | ||
Comment 7•8 years ago
|
||
> I wonder if we should just set this to some comical value like like 5. I think I'd rather see at worst a slightly-busted UI than these black bars.
If it was going to bust the UI, it should bust at the low end to, right? Shouldn't we theoretically be able to set it to 5 and see how it looks on existing devices?
Comment 8•8 years ago
|
||
I just upgraded to Samsung's latest release and now there's a workaround: using the app switcher, I can force Firefox to full screen, which eliminates the black bars and looks great. The setting persists across app restarts.
Reporter | ||
Comment 9•8 years ago
|
||
(In reply to Mike Kaply [:mkaply] from comment #7)
> > I wonder if we should just set this to some comical value like like 5. I think I'd rather see at worst a slightly-busted UI than these black bars.
>
> If it was going to bust the UI, it should bust at the low end to, right?
> Shouldn't we theoretically be able to set it to 5 and see how it looks on
> existing devices?
Yeah, let's do it. If we could we'd set resizeableActivity ("support whatever") anyways.
Assignee | ||
Comment 10•8 years ago
|
||
There's finally some public info about this:
https://android-developers.googleblog.com/2017/03/update-your-app-to-take-advantage-of.html
They are recommending 2.1.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 12•8 years ago
|
||
And we should at least uplift this to 54. I originally didn't uplift because you had to change a user setting on the G6 to see the problem, but with it more common, we should at least get it to 54.
Reporter | ||
Comment 13•8 years ago
|
||
mozreview-review |
Comment on attachment 8860999 [details]
Bug 1352009 - Bump max_aspect_ratio per Android guidelines.
https://reviewboard.mozilla.org/r/133030/#review135786
Attachment #8860999 -
Flags: review?(s.kaspari) → review+
Comment 14•8 years ago
|
||
Pushed by mozilla@kaply.com:
https://hg.mozilla.org/integration/autoland/rev/6bcf19f3164c
Bump max_aspect_ratio per Android guidelines. r=sebastian
Comment 15•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Comment 16•8 years ago
|
||
[triage]
per comment12 we're tagging 54+.
sebastian, could you request the uplift to 54?
Reporter | ||
Updated•8 years ago
|
Assignee: nobody → mozilla
Reporter | ||
Comment 17•8 years ago
|
||
Comment on attachment 8860999 [details]
Bug 1352009 - Bump max_aspect_ratio per Android guidelines.
Approval Request Comment
[Feature/Bug causing the regression]: New phones with different aspect ratios have been released.
[User impact if declined]: On those phones Fennec will not scale to use all screen space (See attached screenshot).
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: I do not have one of those phones.
[Needs manual test from QE? If yes, steps to reproduce]: Just launching Fennec on one of those phones.
[List of other uplifts needed for the feature/fix]: Just this one.
[Is the change risky?]: Very very low.
[Why is the change risky/not risky?]: We are only bumping the max supported aspect ratio in the manifest from 2.0 to 2.1
[String changes made/needed]: -
Attachment #8860999 -
Flags: approval-mozilla-beta?
Attachment #8860999 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 18•8 years ago
|
||
Would it be worthwhile asking for release approval since we're doing a Fennec point release anyway? Especially given that the S8 just came out?
Reporter | ||
Comment 19•8 years ago
|
||
With the risk being super low I'd say yes. I'll request.
Reporter | ||
Comment 20•8 years ago
|
||
Comment on attachment 8860999 [details]
Bug 1352009 - Bump max_aspect_ratio per Android guidelines.
See comment 17. With the risk being so low we'd like to get it into the next dot release.
Attachment #8860999 -
Flags: approval-mozilla-release?
Comment 21•8 years ago
|
||
I've verified that Nightly renders full-screen. Thank you all for taking care of this so quickly!
Comment 22•8 years ago
|
||
[Tracking Requested - why for this release]:
status-firefox53:
--- → affected
tracking-firefox53:
--- → ?
tracking-firefox54:
--- → ?
tracking-firefox55:
--- → ?
Comment 24•8 years ago
|
||
Comment on attachment 8860999 [details]
Bug 1352009 - Bump max_aspect_ratio per Android guidelines.
Fix an issue related to different aspect ratios and was verified. Beta54+. Should be in 54 beta 4.
Attachment #8860999 -
Flags: approval-mozilla-beta?
Attachment #8860999 -
Flags: approval-mozilla-beta+
Attachment #8860999 -
Flags: approval-mozilla-aurora?
Attachment #8860999 -
Flags: approval-mozilla-aurora-
Comment 25•8 years ago
|
||
needs rebasing for beta
grafting 413576:6bcf19f3164c "Bug 1352009 - Bump max_aspect_ratio per Android guidelines. r=sebastian"
merging mobile/android/base/AndroidManifest.xml.in
warning: conflicts while merging mobile/android/base/AndroidManifest.xml.in! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
Flags: needinfo?(mozilla)
Comment 26•8 years ago
|
||
This is running into conflicts because bug 1345014 only landed on m-c. Given that this patch is just updating the one line that patch added, it seems reasonable and fair to just land that patch first so this grafts cleanly onto it. Our other option would be to create a Beta patch in this bug that effectively does the exact same thing. From a blame standpoint, the former option seems better.
Flags: needinfo?(mozilla)
Comment 27•8 years ago
|
||
bugherder uplift |
Assignee | ||
Comment 28•8 years ago
|
||
Might as well pull this change into 53.0.2 as well...
Comment 29•8 years ago
|
||
We have a bunch going on for 53.0.2 and I'd like to not complicate that build with any further risk so we can get it out the door. I'll keep it tracked in case we end up with yet another fennec dot release but otherwise it will go out with 54.
Comment 30•8 years ago
|
||
Let's leave this to ride with 54. I would love to have some verification of the fix in 54, before we get much later in beta.
Flags: qe-verify+
Flags: needinfo?(mihai.ninu)
Flags: needinfo?(ioana.chiorean)
Comment 31•8 years ago
|
||
Hi Liz,
Since I couldn't get access to a Samsung Galaxy S8 (yet), tried this issue on a Samsung Galaxy S7 EDGE (closest device to S8) (Android 7.0) and on a LG V20 (Android 7.0).
No issues to report in both cases using 54.0b8 build and latest 55.0a1 build.
Flags: needinfo?(mihai.ninu)
Flags: needinfo?(ioana.chiorean)
Comment 32•8 years ago
|
||
Comment on attachment 8860999 [details]
Bug 1352009 - Bump max_aspect_ratio per Android guidelines.
As liz said in comment #30
Attachment #8860999 -
Flags: approval-mozilla-release? → approval-mozilla-release-
Comment 33•7 years ago
|
||
Got my hands on a S8. Played around with it on this and works good. Both Nightly and Beta.
Updated•4 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
•