'Start browsing' link from onboarding v1.5 is not visible on small screen devices

VERIFIED FIXED in Firefox 38

Status

()

defect
VERIFIED FIXED
4 years ago
3 years ago

People

(Reporter: CristinaM, Assigned: ally)

Tracking

(Blocks 1 bug)

Trunk
Firefox 39
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox38 verified, firefox39 verified, fennec38+)

Details

Attachments

(5 attachments)

Posted image onboarding.png
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.
Blocks: 1063844
tracking-fennec: --- → ?
Given the bugs filed for "small screen" layout issues, I think we should make a few small screen layout resources.
Assignee: nobody → liuche
tracking-fennec: ? → 38+

Updated

4 years ago
QA Contact: ioana.chiorean
Assignee: liuche → ally
Blocks: onboarding
No longer blocks: 1063844
Assignee

Comment 2

4 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

4 years ago
band-aid works on htc evo 3d, nexus 4, samsung galaxy s5
Assignee

Comment 4

4 years ago
Sony Ericsson xperia : doesnt fit so scrolls properly as expected
Assignee

Comment 5

4 years ago
huawei ascend also fitting, though I note that the blue area is smaller proportionally
Assignee

Comment 6

4 years ago
it seems that in the android layout world, explicit padding around child messes up the weighting of parent elements?
Assignee

Comment 7

4 years ago
given the upcoming merge, I think we might have to run with the bandaid
Assignee

Comment 9

4 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

4 years ago
scrolls around in landscape view
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 13

4 years ago
bandaid #2, w/mdpi layout

tested on the htc evo 3d
Attachment #8582531 - Flags: review?(liuche)
Duplicate of this bug: 1135770
Duplicate of this bug: 1135771
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 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?
https://hg.mozilla.org/mozilla-central/rev/466c2dfda162
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Attachment #8582531 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 21

4 years ago
Verified as fixed in builds:
- 39.0a1 2015-03-26;
- 38.0a2 2015-03-26;
Device: Samsung Galaxy R (Android 2.3.).
Status: RESOLVED → VERIFIED
Blocks: 1154428
You need to log in before you can comment on or make changes to this bug.