If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Unnecessary scroll to reveal full first run panel and options

RESOLVED WONTFIX

Status

()

Firefox for Android
First Run
RESOLVED WONTFIX
2 years ago
10 months ago

People

(Reporter: Ioana Chiorean, Assigned: Shubham, Mentored)

Tracking

(Blocks: 1 bug)

unspecified
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox46 unaffected, firefox47 affected, firefox48 affected)

Details

(Whiteboard: [lang=java][good first bug])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

2 years ago
Android 2.3.5 - HTC Desire HD
Fennec 46 Beta 1

Steps:
1. Open Fennec with a clean profile on a low resolution device

Expected Result: 
- you should be able to see the 'Next' link at the bottom part of the panel/screen

Actual Result:
- you can not see the Next button and some users might be blocked by that ( they can use the top area to switch to other panel)

Notes:
- new first run: https://www.youtube.com/watch?v=cfykZ9wEbQI&feature=youtu.be
- old first run: https://www.youtube.com/watch?v=5dZDNDTqsR0&feature=youtu.be
Flags: needinfo?(liuche)
Thanks for the videos Ioana.

I think we can do a couple of things with this, either:
- Add a spacer with a weight in between the text body
- Have a specific layout for small devices
- WONTFIX this

I'll add a mentor flag to this bug - basically, we want this to show the "Next" text on smaller-screen devices. Let's try this by adding a spacer with a weight between the "Next" item and the body text, and trying to make it as close to the current version as possible.
Blocks: 1073128
Mentor: liuche@mozilla.com
Flags: needinfo?(liuche) → needinfo?(alam)
Whiteboard: [lang=java][good first bug]
I'm not sure what "adding a spacer" means but some screenshots on how this "spacer" affects most devices would be helpful :)
Flags: needinfo?(alam)

Comment 3

2 years ago
Hi,
I'am really interesting by this bug ( it will be my first bug)
But i don't understand how to put my emulator (mozemulator) to a low quality =/

Do you know how to do that?
Hi Alex, do you use Android Studio or IntelliJ? You should be able to create new emulators in Tools > Android > AVD Manager (or just shift-shift to bring up the search and type "AVD Manager"). From there you should be able to create an emulator that will show up in the device chooser when you want to run a build.

As a note, this probably isn't a ldpi device, an mdpi one would do - we want the case where there is space to show all the content, but we're not.

Additionally, make sure you test any changes you make on normal size devices and make sure that we're not regressing anything for the other devices.

Let me know if you have any questions!

Comment 5

2 years ago
Hi Chenxia, Thank you for your answer.
I'am using IntelliJ.
I succes to recreat the bug on a QGVA 320 * 480 mdpi, with marshmallow Android 6.0x86_64.
Can you just help me a bit where is situated the code of this bug? Or an indication ? ^.^

Comment 6

2 years ago
Hello,
Is it possible to be assigned to this bug?

Comment 7

2 years ago
(I found where is the problem, I 'am working on it  :)  )
Thanks Alex! We don't assign bugs until there's a patch, because we often get lots of interest but not much followup.

If you haven't seen it already, this is our guideline to writing/preparing patches. Let me know if you have questions, and thanks again for investigating this bug!
https://wiki.mozilla.org/Mobile/Fennec/Android/Suggested_workflow#Creating_commits_and_submitting_patches

Comment 9

2 years ago
Okey, I will do my best to fix this bug then :)

Comment 10

a year ago
Hi,
I'm actually trying to fix the UI bug, but I can't find an easy way to launch the firstscreen panel.
Do you know an easy way to do it (maybe through Intent ?).

Comment 11

a year ago
Created attachment 8749272 [details] [diff] [review]
Patch for Bug 1254555

Add files for multiples densities
Corrects the bug for mdpi and hdpi screens
Attachment #8749272 - Flags: review?(liuche)

Comment 12

a year ago
Hello,
Is it possible to have a feedback about my patch ? :)
All of the primary developers are at Google I/O this week.

Comment 14

a year ago
oh, okey thank you for the information
Comment on attachment 8749272 [details] [diff] [review]
Patch for Bug 1254555

Review of attachment 8749272 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay!

Hmm, so looking at this, I don't think that we want to do this by hard-coding specific dimensions for different dpi screens. I don't think this would work for all devices, so we'd just be pushing the problem off to a smaller set of specific devices. Specifically, there should be a way to use layout_weight or perhaps RelativeLayout to constrain these layouts so they don't take up too much space, so I'd like to see an approach that doesn't rely on hard-coded dimensions.

::: mobile/android/base/resources/values-hdpi/dimens.xml
@@ +4,5 @@
> +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +
> +<resources>
> +
> +    <dimen name="firstrun_content_width">300dp</dimen>

If this value isn't changing between versions, you don't need to include it in a dpi-specific file.
Attachment #8749272 - Flags: review?(liuche)
(Assignee)

Comment 16

a year ago
Hi, Can I take up this bug(this will be my first bug) from where Alex left.
Shubham: looks like it's been sitting idle for a bit. Unless Alex says he'd like to take another run at it, go for it.
(Assignee)

Comment 18

a year ago
(In reply to Alex from comment #14)
> oh, okey thank you for the information

Hey Alex, are you still working on it. If not would it be okay if I take it over?
(Assignee)

Comment 19

a year ago
Alex ping for this.
Flags: needinfo?(noxus.iceman)

Comment 20

a year ago
Hey, yeah you can do it, sorry for delay
Comment hidden (mozreview-request)
(Assignee)

Comment 22

a year ago
Hi Mike, I have pushed the patch for the bug. Looking forward to your feedback. Thanks.
Flags: needinfo?(mhoye)
Thanks, Subham, but I'm not the right person. Chenxia, what do you think?
Flags: needinfo?(mhoye) → needinfo?(liuche)
(Assignee)

Comment 24

a year ago
Hi Chenxia, did you get a chance to look at the patch.
Comment on attachment 8793470 [details]
Bug 1254555 - Unnecessary scroll to reveal full first run panel.

Thanks Shubham, I took a look at this patch! The code looks good, but there's the problem when no switch is present and the phone is in landscape, and the "Next" text overlaps the text of the slide. Also, there's the case where the "Next" button is actually above the switch. I'll add screenshots of these two cases.

You could consider setting the switch view to invisible, and adding it to the RelativeLayout positioning hierarchy.

Alternatively, if you find it isn't actually easy to take the approach to fix this, we should just WONTFIX it.

Thanks for your help!
Flags: needinfo?(noxus.iceman)
Flags: needinfo?(liuche)
Attachment #8793470 - Flags: review?(mhoye) → feedback+
Assignee: nobody → shubham2892
Created attachment 8796364 [details]
Screenshot: Overlap
Attachment #8749272 - Attachment is obsolete: true
Created attachment 8796365 [details]
Screenshot: Incorrect view ordering

Comment 28

a year ago
Hi, how can I work on this bug? I'm new to open source.
(Assignee)

Comment 29

a year ago
(In reply to Chenxia Liu [:liuche] from comment #25)
> Comment on attachment 8793470 [details]
> Bug 1254555 - Unnecessary scroll to reveal full first run panel.
> 
> Thanks Shubham, I took a look at this patch! The code looks good, but
> there's the problem when no switch is present and the phone is in landscape,
> and the "Next" text overlaps the text of the slide. Also, there's the case
> where the "Next" button is actually above the switch. I'll add screenshots
> of these two cases.
> 
> You could consider setting the switch view to invisible, and adding it to
> the RelativeLayout positioning hierarchy.
> 
> Alternatively, if you find it isn't actually easy to take the approach to
> fix this, we should just WONTFIX it.
> 
> Thanks for your help!

Hi Chenxia,
As of with the current structure, it is not easy to fix. One approach could be decreasing the padding between layout elements for the low resolution devices but as you already mentioned that would be pushing the problem off to a smaller set of specific devices.
Flags: needinfo?(liuche)
Thanks for looking into this, Shubham, because this bug has been open for a while.

I think the best course of action here is to WONTFIX this bug. I tried this out on a bunch of small devices, and for the most part with the newest firstrun experiment, this problem shows up rarely and it's not worth having more people invest time into this.

Thanks again!
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → WONTFIX

Updated

10 months ago
Flags: needinfo?(liuche)
You need to log in before you can comment on or make changes to this bug.