[RTL] Dividers and padding are reversed in RTL in the tab strip on tablets

RESOLVED FIXED in Firefox 54



Firefox for Android
Theme and Visual Design
6 months ago
3 months ago


(Reporter: Tom Klein, Assigned: Tom Klein, NeedInfo)


(Blocks: 1 bug)

Firefox 55
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox54 fixed, firefox55 fixed)


MozReview Requests


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


(3 attachments)



6 months ago
Created attachment 8835114 [details]
Top: missing padding to the right; bottom: missing divider to the left

This bug has a similar underlying cause to bug 1337933.

The two screen shots here are both from a tablet with the tab strip scrolled all the way to the right.

In this case, the ItemDecoration for the tab strip RecyclerView draws a divider to the left of each tab[1], and also adjusts the padding of the leftmost and rightmost tabs in order to view the full tab[2].  In RTL mode the notions of left and right are reversed, so we do the wrong thing.

Issue [1] is visible in the bottom screenshot: the rightmost visible tab here doesn't get a divider to its left because it thinks it's the first (leftmost(!)) tab and therefore doesn't need a divider to its left.

Issue [2] is visible in the top screenshot where we've added extra padding to the left of the rightmost tab because we think it's actually the leftmost tab, which makes the right side of that tab cut off because it didn't get that extra padding (I think that's what's happening, I haven't actually checked).

[1] https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/tabs/TabStripDividerItem.java#81-86
[2] https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/tabs/TabStripDividerItem.java#66-69
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 3

4 months ago
Comment on attachment 8856526 [details]
Bug 1337947 - Fix tab strip divider drawing for RTL.


Attachment #8856526 - Flags: review?(max) → review+

Comment 4

4 months ago
Comment on attachment 8856527 [details]
Bug 1337947 - Fix tab strip item offsets for RTL.

Attachment #8856527 - Flags: review?(max) → review+


4 months ago
Assignee: nobody → twointofive

Comment 5

4 months ago
Pushed by twointofive@gmail.com:
Fix tab strip divider drawing for RTL. r=maliu
Fix tab strip item offsets for RTL. r=maliu

Comment 6

4 months ago
Last Resolved: 4 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55

Comment 7

3 months ago
Comment on attachment 8856526 [details]
Bug 1337947 - Fix tab strip divider drawing for RTL.

Approval Request Comment
[Feature/Bug causing the regression]: RTL for fennec
[User impact if declined]: Dividers and padding in the tab strip on tablets are incorrect in RTL at some positions.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: See comments and screenshot in comment 0.
[List of other uplifts needed for the feature/fix]: Please uplift both patches.
[Is the change risky?]: Low risk.
[Why is the change risky/not risky?]: The fix changes paddings and dividers in RTL mode by simply taking into account the fact that tab positions in the tab strip are counted starting from the right in RTL mode.
[String changes made/needed]: None.
Attachment #8856526 - Flags: approval-mozilla-aurora?

Comment 8

3 months ago
Hi Mihai,
Can you help check if this is fixed in the latest nightly?
Flags: qe-verify+
Flags: needinfo?(mihai.ninu)

Comment 9

3 months ago
Comment on attachment 8856526 [details]
Bug 1337947 - Fix tab strip divider drawing for RTL.

Fix an RTL issue. Aurora54+.
Attachment #8856526 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 10

3 months ago
status-firefox54: affected → fixed


3 months ago
Flags: needinfo?(mihai.ninu) → needinfo?(ioana.chiorean)
You need to log in before you can comment on or make changes to this bug.