1457722 - [RTL] Resizer icons mirrored

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P1)

Description

[Itiel]

Attachment: 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.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Is this a recent regression or has it always been facing the wrong way?

Comment 2

[Itiel]

(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

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

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?

Comment 4

[Itiel]

(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 :(

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

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?

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Tim, it looks like this broke from the removal of the resizer binding. Can you take a look at this?

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I'll try to see what's going on here...

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

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 9

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: Bug 1457722 - Don't flip resizer twice in RTL-locales

Review commit: https://reviewboard.mozilla.org/r/241050/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/241050/

Comment 10

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8972448 [details]
Bug 1457722 - Don't flip resizer twice in RTL-locales

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/241050/diff/1-2/

Comment 11

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8972448 [details]
Bug 1457722 - Don't flip resizer twice in RTL-locales

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/241050/diff/2-3/

Comment 12

[Dão Gottwald [:dao]]

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

Comment 13

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8972448 [details]
Bug 1457722 - Don't flip resizer twice in RTL-locales

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/241050/diff/3-4/

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/70ee109b2478
Don't flip resizer twice in RTL-locales r=dao

Comment 15

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/70ee109b2478

Comment 16

[Itiel]

Looking good on latest Nightly.