Closed
Bug 1128431
Opened 9 years ago
Closed 9 years ago
'Start browsing' link from onboarding v1.5 is not visible on small screen devices
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox38 verified, firefox39 verified, fennec38+)
VERIFIED
FIXED
Firefox 39
People
(Reporter: CristinaM, Assigned: ally)
References
Details
Attachments
(5 files)
58.03 KB,
image/png
|
Details | |
1.47 MB,
image/jpeg
|
Details | |
1.03 MB,
image/jpeg
|
Details | |
44.33 KB,
image/png
|
Details | |
3.71 KB,
patch
|
liuche
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Environment: Device: Samsung Galaxy R (Android 2.3.4) Build: Firefox for Android 38.0a1 (2015-02-01) Steps to reproduce: 1. Make sure your Firefox profile is clean (Android Settings -> Apps -> Manage applications -> Firefox -> Force Stop & Clear data); 2. Launch Firefox. Expected result: Onboarding screen is displayed containing 'Sign in to Nightly' button and 'Start browsing' link. Actual results: 'Start browsing' link is not visible. Notes: Please see the attachment.
Updated•9 years ago
|
tracking-fennec: --- → ?
Comment 1•9 years ago
|
||
Given the bugs filed for "small screen" layout issues, I think we should make a few small screen layout resources.
Updated•9 years ago
|
Assignee: nobody → liuche
tracking-fennec: ? → 38+
Updated•9 years ago
|
QA Contact: ioana.chiorean
Updated•9 years ago
|
Assignee: liuche → ally
Updated•9 years ago
|
Assignee | ||
Comment 2•9 years ago
|
||
Digging into this, we're pretty sure this a layout bug. Ruled out - altering or adding weight to the missing element does not have an effect - altering the weights of the parents - scrollview has correct number & type of children - there are no listviews present to interfere - layout emulators show missing textview in correct place - reordering the elements. the bottom thing is cut off or not present and there is still no scrollbar --- I notice that the padding between the hint(firstrun_welcome_subtext) & the following child is honored over wrap_content of the bottom children: android:paddingTop="20dp" android:paddingBottom="30dp" Works (scrollbar when needed + visible element) - hardcording the height instead of wrap content - keeping wrap content & hardcoding a minHeight at 92dp+ works, but we feel that this is a band-aid and that there is something bigger going on. diff --git a/mobile/android/base/resources/layout/firstrun_welcome_fragment.xml b/mobile/android/base/resources/layout/firstrun_welcome_fragment.xml --- a/mobile/android/base/resources/layout/firstrun_welcome_fragment.xml +++ b/mobile/android/base/resources/layout/firstrun_welcome_fragment.xml @@ -54,15 +54,16 @@ style="@style/Widget.Firstrun.Button" android:background="@drawable/firstrun_button_background" android:layout_gravity="center" android:text="@string/firstrun_welcome_button_account"/> <TextView android:id="@+id/welcome_browse" android:layout_width="@dimen/firstrun_content_width" android:layout_height="wrap_content" + android:minHeight="92dp" android:padding="20dp" android:gravity="center" android:textAppearance="@style/TextAppearance.FirstrunRegular.Link" android:text="@string/firstrun_welcome_button_browser"/> </LinearLayout> </LinearLayout> </ScrollView> So now we have a band-aid as a fallback. My plan is to see if I can make this behave in a more css like manner to scale the gaps between text accordingly because this feels like a bad interaction between autowrapping height & fixed heights.
Assignee | ||
Comment 3•9 years ago
|
||
band-aid works on htc evo 3d, nexus 4, samsung galaxy s5
Assignee | ||
Comment 4•9 years ago
|
||
Sony Ericsson xperia : doesnt fit so scrolls properly as expected
Assignee | ||
Comment 5•9 years ago
|
||
huawei ascend also fitting, though I note that the blue area is smaller proportionally
Assignee | ||
Comment 6•9 years ago
|
||
it seems that in the android layout world, explicit padding around child messes up the weighting of parent elements?
Assignee | ||
Comment 7•9 years ago
|
||
given the upcoming merge, I think we might have to run with the bandaid
Assignee | ||
Comment 8•9 years ago
|
||
Assignee | ||
Comment 9•9 years ago
|
||
yes, you may give me shit for taking a photo. intellij's screenshot thingy is not cooperating and this discussion needs to _move_.
Assignee | ||
Comment 10•9 years ago
|
||
scrolls around in landscape view
Comment 11•9 years ago
|
||
I'm okay with this approach if this is a separate mdpi layout, so we minimize the impact of this on larger devices, esp in landscape.
Assignee | ||
Comment 12•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=786478ed8eb3
Assignee | ||
Comment 13•9 years ago
|
||
bandaid #2, w/mdpi layout tested on the htc evo 3d
Attachment #8582531 -
Flags: review?(liuche)
Comment 16•9 years ago
|
||
Comment on attachment 8582531 [details] [diff] [review] welcomefirstrunlayout Review of attachment 8582531 [details] [diff] [review]: ----------------------------------------------------------------- I thought at first that this should be in layout-small, but this problem isn't restricted to small devices (and in fact doesn't manifest on some small devices like the Galaxy Ch@t) so layout-mdpi it is. Curious and curiouser. Please land and request uplift when fx-team opens again. Also, good practice to just include the commit message when you flag for review, esp for an aurora bug.
Attachment #8582531 -
Flags: review?(liuche) → review+
Comment 17•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/466c2dfda162
Target Milestone: --- → Firefox 39
Comment 18•9 years ago
|
||
Comment on attachment 8582531 [details] [diff] [review] welcomefirstrunlayout Approval Request Comment [Feature/regressing bug #]: First Run (bug 1063844) on mdpi devices [User impact if declined]: First-time users on mdpi devices will have an unscrollable, truncated first run experience in portrait [Describe test coverage new/current, TreeHerder]: try run, local testing [Risks and why]: Very low, new resource file that is an exact copy of an existing resource file with one item height change, to be used only by mdpi devices [String/UUID change made/needed]: none
Attachment #8582531 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
status-firefox38:
--- → affected
Updated•9 years ago
|
Attachment #8582531 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 21•9 years ago
|
||
Verified as fixed in builds: - 39.0a1 2015-03-26; - 38.0a2 2015-03-26; Device: Samsung Galaxy R (Android 2.3.).
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
•