Closed Bug 1267956 Opened 8 years ago Closed 8 years ago

Scroll bar is not displayed in History panel

Categories

(Firefox for Android Graveyard :: Awesomescreen, defect)

ARM
Android
defect
Not set
normal

Tracking

(firefox46 unaffected, firefox47 unaffected, firefox48 verified, firefox49 verified, fennec48+)

VERIFIED FIXED
Firefox 49
Tracking Status
firefox46 --- unaffected
firefox47 --- unaffected
firefox48 --- verified
firefox49 --- verified
fennec 48+ ---

People

(Reporter: sflorean, Assigned: ahasan78, Mentored)

Details

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

Attachments

(1 file, 1 obsolete file)

Environment: 
Device: Asus Transformer Pad (Android 4.2.1);
Build: Nightly 49.0a1 (2016-04-26);

Prerequisites:
Make sure you have enough browsing history so the history panel is scroll-able.

Steps to reproduce:
1. In about:home, go to History panel;
2. Swipe down the panel.  

Expected result:
Scroll bar is displayed while scrolling.  

Actual result:
Scroll bar is not displayed.
Regression from your recent work?
tracking-fennec: --- → ?
Flags: needinfo?(liuche)
This requires adding an android:scrollbars="vertical" in the xml of home_combined_history_panel.xml.

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/layout/home_combined_history_panel.xml#21
Mentor: liuche
Flags: needinfo?(liuche)
Whiteboard: [lang=java][good first bug]
Hi I am interested in this bug. Can you please assogn this bug to me and give more guidelines to solve this bug.

Thank you
Flags: needinfo?(liuche)
Hi agore, have you been able to build Firefox for Android? Take a look at our setup docs: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_for_Android_build

After that, you need to update the file I referenced to explicitly include vertical scrollbars.

Once you upload a patch, I will be happy to assign this bug to you!
Flags: needinfo?(liuche)
Attached patch Layout for firefox history tab (obsolete) — Splinter Review
Fixed bug - scroll bar not displayed in history panel
Tested on moto G
Attachment #8747126 - Flags: review?(liuche)
Attachment #8747126 - Attachment is patch: true
Attachment #8747126 - Attachment mime type: text/xml → text/plain
Hi Ahasan, I see you uploaded a file but it's not formatted correctly. Take a look at these instructions for make a patch:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Creating_a_patch
Flags: needinfo?(ahasan78)
thanks for the link . I will read the instruction and submit the patch properly .
Flags: needinfo?(ahasan78)
Attachment #8747126 - Flags: review?(liuche)
Hi ahasan, I think I missed your ping on irc - in the future, you can ask in #mobile and there will be more people around.

I looked at the instructions and I agree, they can be kind of confusing and overwhelming. This set of instructions should be much more straightforward - let me know if you run into any problems:
https://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview-user.html
Attachment #8747446 - Flags: review?(liuche)
Assignee: nobody → ahasan78
Attachment #8747126 - Attachment is obsolete: true
Comment on attachment 8747446 [details] [diff] [review]
history panel scrollbar fix

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

Nice! Next time, also include r=liuche at the end of the commit message - when this lands in the tree, it helps us the sheriffs quickly keep track of who they might need to contact in case of breakages (and just also general bookkeeping).

If you want to continue contributing, you could take a look at bug 1261041! We have a bug query for [good next bugs] too, if you want to look through those and pick some https://bugzilla.mozilla.org/buglist.cgi?list_id=12995922&classification=Client%20Software&status_whiteboard_type=allwordssubstr&query_format=advanced&status_whiteboard=%5Bgood%20next%20bug%5D&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&product=Firefox%20for%20Android

Just out of curiosity, where did you look to find this bug in bugzilla?
Attachment #8747446 - Flags: review?(liuche) → review+
(In reply to Chenxia Liu [:liuche] from comment #10)
> Comment on attachment 8747446 [details] [diff] [review]
> history panel scrollbar fix
> 
> Review of attachment 8747446 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Nice! Next time, also include r=liuche at the end of the commit message -
> when this lands in the tree, it helps us the sheriffs quickly keep track of
> who they might need to contact in case of breakages (and just also general
> bookkeeping).
> 
> If you want to continue contributing, you could take a look at bug 1261041!
> We have a bug query for [good next bugs] too, if you want to look through
> those and pick some
> https://bugzilla.mozilla.org/buglist.
> cgi?list_id=12995922&classification=Client%20Software&status_whiteboard_type=
> allwordssubstr&query_format=advanced&status_whiteboard=%5Bgood%20next%20bug%5
> D&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPEN
> ED&product=Firefox%20for%20Android
> 
> Just out of curiosity, where did you look to find this bug in bugzilla?

thankyou for the above helpful links and replying to my questions . I found the bug from wiki/Mobile/Get_Involved . Looking forward for my next bug fix .
https://hg.mozilla.org/mozilla-central/rev/2c21e96a98ed
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Chenxia, can you request Aurora uplift for this?
tracking-fennec: ? → 48+
Flags: needinfo?(liuche)
Comment on attachment 8747446 [details] [diff] [review]
history panel scrollbar fix

Approval Request Comment
[Feature/regressing bug #]: Bug 1220928
[User impact if declined]: Scrollbars not visible in History panel
[Describe test coverage new/current, TreeHerder]: local, nightly
[Risks and why]: very small, adds one xml parameter that was missing
[String/UUID change made/needed]: none
Flags: needinfo?(liuche)
Attachment #8747446 - Flags: approval-mozilla-aurora?
Verified as fixed in build 49.0a1 2016-05-05;
Device: Asus Transformer Pad (Android 4.2.1).
Keywords: regression
Comment on attachment 8747446 [details] [diff] [review]
history panel scrollbar fix

Fix for recent regression, please uplift to aurora. 
Should there be a test added to catch this kind of regression?
Attachment #8747446 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Oh, that's a good point - we could add this to our general RecyclerView linting, assuming that we always want a scrollbar when using RecyclerViews. But also in this particular case, we were switching from one library to another library that requires this extra xml parameter - I'll definitely keep it in mind for future reviews too though. Thanks!
Verified as fixed in build 48.0a2 (2016-05-12);
Device: LG G4 (Android 5.1).
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.