Closed Bug 1457722 Opened 6 years ago Closed 6 years ago

[RTL] Resizer icons mirrored

Categories

(Firefox :: Settings UI, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 61
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox59 --- unaffected
firefox60 --- unaffected
firefox61 + verified

People

(Reporter: itiel_yn8, Assigned: timdream)

References

Details

(Keywords: regression, rtl)

Attachments

(2 files)

Attached image Screenshot
See attached.
They're located at the right place, but not in the right direction. They should be mirrored so they'd be directed to the left-bottom (currently right-bottom).

Note: This applies to all resizer icons across Preferences/Options, and not just in the Languages window as shown in the attached screenshot.
Is this a recent regression or has it always been facing the wrong way?
Flags: needinfo?(itiel_yn8)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #1)
> Is this a recent regression or has it always been facing the wrong way?

Mozregression output:
2018-04-30T18:24:31: DEBUG : Starting merge handling...
2018-04-30T18:24:31: DEBUG : Using url: https://hg.mozilla.org/integration/autoland/json-pushes?changeset=3a10be9bb6bd3b90594b4afe2896f9279f339e9a&full=1
2018-04-30T18:24:33: DEBUG : Found commit message:
Backed out 2 changesets (bug 1446923) for bustages in /python/mozbuild/mozbuild/test/frontend/test_emitter.py on a CLOSED TREE

Backed out changeset b9a5aab21d71 (bug 1446923)
Backed out changeset bc5ab6e2db10 (bug 1446923)

2018-04-30T18:24:33: INFO : The bisection is done.

I don't think this makes much sense but I've done it twice and this is what I got. The range is 2018-04-26 to 2018-04-27
Flags: needinfo?(itiel_yn8)
Keywords: regression
The dates don't match up perfectly, but this feels like something that might have gotten broken by bug 1451992. Itiel, how did you test the locale using mozregression?

Perhaps the language pack wasn't updated until the 2018-04-27 build. Zibi, can you look in to this?
Flags: needinfo?(itiel_yn8)
Flags: needinfo?(gandalf)
Priority: -- → P1
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #3)
> The dates don't match up perfectly, but this feels like something that might
> have gotten broken by bug 1451992. Itiel, how did you test the locale using
> mozregression?
> 
> Perhaps the language pack wasn't updated until the 2018-04-27 build. Zibi,
> can you look in to this?

I'm not using any language pack- I'm simply setting intl.uidirection to 1. No other settings tweaked.
I've bisected it again and mozregression gave the same range. Sorry :(
Flags: needinfo?(itiel_yn8)
I would be surprised if it was related to Fluent migration. We didn't touch CSS. Is it possible that some XBL removals (like resizer removal ;)) triggered it?
Flags: needinfo?(gandalf)
Tim, it looks like this broke from the removal of the resizer binding. Can you take a look at this?
Blocks: 1450017
Flags: needinfo?(timdream)
I'll try to see what's going on here...
Assignee: nobody → timdream
Status: NEW → ASSIGNED
Flags: needinfo?(timdream)
I am correcting the CSS rules with the following table:

In non-RTL locale:

      SW-direction          SE-direction
      bottomleft            bottomright*
      bottomstart           bottomend*

In RTL locale:

      SW-direction          SE-direction
      bottomleft*           bottomright
      bottomend*            bottomstart           

(*) are the resizers that is in its correct direction, "flipped" by native theming or the |background: resizer-rtl.png| rule set by :-moz-locale-dir(rtl), so we should *not* set |transform: scale(-1)| on them. Clearly, we should set |transform| conditionally on bottomleft/bottomright, but unconditionally on bottomstart.

For the selectors changing the cursor, it's much simpler because there isn't something else that would "flip" the cursor based on locale dir first.
Comment on attachment 8972448 [details]
Bug 1457722 - Don't flip resizer twice in RTL-locales

https://reviewboard.mozilla.org/r/241050/#review246932

::: toolkit/content/minimal-xul.css:94
(Diff revision 3)
>     replacing the background image. */
>  resizer:-moz-locale-dir(rtl) {
>    background: url("chrome://global/skin/icons/resizer-rtl.svg") no-repeat;
>  }
>  
> -resizer[dir="left"],
> +resizer[dir="left"]:not(:-moz-locale-dir(rtl)),

nit: :-moz-locale-dir(ltr) here and elsewhere
Attachment #8972448 - Flags: review?(dao+bmo) → review+
Pushed by timdream@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/70ee109b2478
Don't flip resizer twice in RTL-locales r=dao
https://hg.mozilla.org/mozilla-central/rev/70ee109b2478
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Looking good on latest Nightly.
Status: RESOLVED → VERIFIED
See Also: → 1459646
See Also: → 1464412
You need to log in before you can comment on or make changes to this bug.