Closed Bug 962284 Opened 6 years ago Closed 6 years ago

Metro Tab strip overflow layout is wrong in RTL locales

Categories

(Firefox for Metro Graveyard :: Theme, defect, P2)

All
Windows 8.1
defect

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 29

People

(Reporter: mbrubeck, Assigned: emtwo)

References

Details

(Keywords: rtl, Whiteboard: [defect] p=1)

Attachments

(1 file, 1 obsolete file)

When running in a right-to-left locale (bug 851165), if there are enough tabs open to make the tab strip scrollable, the tabs do not line up correctly with the vertical separators (bug 948778).
Assignee: nobody → msamuel
Oops, I stole a mentor-able bug O_o
Attachment #8368220 - Flags: review?(mbrubeck)
Comment on attachment 8368220 [details] [diff] [review]
v1: Adjust tabstrip css in rtl locales

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

This fixes the main issue I reported, plus some that I hadn't noticed.  But there seem to be some remaining issues, and I don't understand all of the styles.  (They might just need some comments to explain them.)

::: browser/metro/theme/browser.css
@@ +55,5 @@
>  }
>  
> +.tabs-scrollbox > .scrollbutton-up:-moz-locale-dir(rtl),
> +.tabs-scrollbox > .scrollbutton-down:-moz-locale-dir(rtl) {
> +   transform: scaleX(-1);

The arrows now look correct, but they still behave wrong (sometimes?).  I'm not sure exactly why... do we need to set a -moz-box-direction: reverse somewhere?

@@ +59,5 @@
> +   transform: scaleX(-1);
> +}
> +
> +.tabs-scrollbox > .scrollbutton-up:not([disabled]):not([collapsed]):-moz-locale-dir(rtl)::after {
> +  left: @tabs_scrollarrow_width@ !important;

With this patch applied, the separators now appear correctly in RTL when using the mouse (inputmode="precise") but I don't see them when using RTL+touch.  (In LTR locales they are visible with both precise and imprecise input.)  Again, it's not clear to me why this isn't working correctly.

@@ +64,5 @@
> +}
> +
> +.tabs-scrollbox > .scrollbutton-down:not([disabled]):not([collapsed]):-moz-locale-dir(rtl)::before {
> +  right: auto;
> +  left: 0px !important;

Do you know whether/why the !important is necessary?
Attachment #8368220 - Flags: review?(mbrubeck) → review-
> The arrows now look correct, but they still behave wrong (sometimes?).

Was there any other incorrect behaviour that you saw besides the lack of separators when using imprecise input?

When I wrote that patch, I looked for where the lines for :before and :after were being drawn then adjusted the "left" property accordingly because with scaleX() I could not make sense of why the :before and :after lines were where they were.

I have another patch here without scaleX() which I think should be more clear and it does not have the missing separator issue.

If you have any ideas of why scaleX() might have been causing issues please let me know. Thanks!
Attachment #8368220 - Attachment is obsolete: true
Flags: needinfo?(mbrubeck)
Comment on attachment 8368629 [details] [diff] [review]
v2: Adjust tabstrip css in rtl locales

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

(In reply to Marina Samuel [:emtwo] from comment #3)
> Was there any other incorrect behaviour that you saw besides the lack of
> separators when using imprecise input?

Yes - when clicking on the arrow buttons, the scrollbox would sometimes scroll in the correct direction and sometimes in the opposite direction.

> I have another patch here without scaleX() which I think should be more
> clear and it does not have the missing separator issue.

Yes, this looks perfect and also seems to fix the behavior of the buttons noted above.
Attachment #8368629 - Flags: review+
Flags: needinfo?(mbrubeck)
https://hg.mozilla.org/integration/fx-team/rev/438cab96ec84
Whiteboard: [defect] p=1 [mentor=mbrubeck@mozilla.com][lang=css] → [fixed-in-fx-team] [defect] p=1 [mentor=mbrubeck@mozilla.com][lang=css]
Blocks: metrov1it23
No longer blocks: metrobacklog
Status: NEW → ASSIGNED
Priority: -- → P2
QA Contact: jbecerra
Whiteboard: [fixed-in-fx-team] [defect] p=1 [mentor=mbrubeck@mozilla.com][lang=css] → [fixed-in-fx-team] [defect] p=1
https://hg.mozilla.org/mozilla-central/rev/438cab96ec84
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team] [defect] p=1 → [defect] p=1
Target Milestone: --- → Firefox 29
Mozilla/5.0 (Windows NT 6.3; rv:29.0) Gecko/20100101 Firefox/29.0
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0

Verified as fixed on Windows 8.1 using the Arabic and Hebrew locales with latest Nightly (build ID: 20140203030203).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.