1120194 - Refactor RTL styling for Settings App

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Settings, defect)

Description

[Ahmed Nefzaoui [:Nefzaoui]]

Last time an effort to make Settings compatible with RTL layout was made was at around v1.4, since then Settings app evolved too much that too many bugs are appearing for the RTL layout because of the obsolete and unused code, different values from LTR to RTL, so the best approach here is to remove all the RTL code and work our way up on a clean slate to make sure Settings up contains only valid code.

Comment 1

[Ahmed Nefzaoui [:Nefzaoui]]

Attachment: PR to Github

Comment 2

[Ahmed Nefzaoui [:Nefzaoui]]

Comment on attachment 8547241 [details] [review]
PR to Github

PR's ready ready.
Please review?
Thanks!

Comment 3

[Fabien Cazenave [:kaze]]

Hey Ahmed, I’ve just left a few nitpicks on your PR. LGTM otherwise.

I’m a bit sad that those `-moz-*-[start|end]` properties aren’t candidate for any CSS spec, but I agree it’s probably the only reasonable way to stop breaking RTL support.

Comment 4

[Ahmed Nefzaoui [:Nefzaoui]]

(In reply to Fabien Cazenave [:kaze] from comment #3)
> Hey Ahmed, I’ve just left a few nitpicks on your PR. LGTM otherwise.

Updated :)
Thanks!

Comment 5

[Fabien Cazenave [:kaze]]

Good work. This removes a bunch of RTL-specific rules, and gets us rid of 2 warnings as a bonus.

Waiting for the tests to finish…

Comment 6

[Fabien Cazenave [:kaze]]

Merged: https://github.com/mozilla-b2g/gaia/commit/b0254d13e67b0bd4c74c2f2c9b9ea1251949d9fe

Comment 7

[Shine(Leave from Mozilla)]

Attachment: Screenshot

Hi Ahmed,
This problem is verified Fail on latest build of Flame2.2. On Settings app view, some toggle buttons, item contents, input box…are displayed incompletely.
See attachment: Screenshot.PNG
Reproduce rate 5/5

Flame 2.2 build:
Gaia-Rev        0518f4581a0925c0b703d730ef289ab15cbd1216
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/c6aa604a7967
Build-ID        20150126002536
Version         37.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150126.042138
FW-Date         Mon Jan 26 04:21:49 EST 2015

Comment 8

[Dylan Oliver [:doliver]]

2.2 uplift required

Comment 9

[Ahmed Nefzaoui [:Nefzaoui]]

This has been merged to master right before branching, the issues in the screenshot are related to Bug 1121748.

Comment 10

[Fabien Cazenave [:kaze]]

I confirm that Ahmed’s patch is already in the 2.2 branch — no uplift is needed.

(In reply to Shine from comment #7)
> This problem is verified Fail on latest build of Flame2.2. On Settings app
> view, some toggle buttons, item contents, input box…are displayed
> incompletely.
> See attachment: Screenshot.PNG

This is another issue, caused by a recent (and bad) Gecko regression — see bug 1121748 and bug 1119420. The fix should be uplifted to the 2.2 branch in the next few days.

We can keep this bug closed RESOLVED/FIXED.

Comment 11

[Coler]

According to Comment 10, We verifys this issue again,this issue has been verified successufully on Flame 2.2.
Reproduce rate:0/5.
Attachment:Verify_RTL_Settings.mp4

Flame 2.2 build:

Gaia-Rev        d6141fa3208f224393269e17c39d1fe53b7e6a05
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/f7414413e3a5
Build-ID        20150201002504
Version         37.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150201.043120
FW-Date         Sun Feb  1 04:31:31 EST 2015
Bootloader      L1TC000118D0

Comment 12

[Coler]

Attachment: Verify_RTL_Settings.MP4