1010615 - [B2G][RTL][FTE] Arabic: Switches right aligned in FTE when a RTL language is selected

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::First Time Experience, defect, P1)

Description

[Warren Holdsworth]

Attachment: FTE_Cellular_logcat.txt

Description:
When a RTL language such as Arabic is selected during the FTE the 'Enable Data' switch for Cellular Data is right aligned instead of left aligned

Repro Steps:
1) Update a Buri to BuildID: 20140514000204.
2) Select 'Arabic' in the FTE Language selection screen.
3) Select 'Next'.
4) Observe that the 'Enable Data' switch is right aligned.


Actual:
Switches are right aligned in FTE screens when using RTL languages.

Expected:
Switches are Left aligned in all screens when using RTL languages.

1.4 Environmental Variables:
Device: Buri 1.4 MOZ
BuildID: 20140514000204
Gaia: b40103dec34a147c9018a1af76eb21c3184f2f93
Gecko: 7788969f70b0
Version: 30.0
Firmware Version: v1.2-device.cfg


Repro frequency: 100%
FTE_Cellular_logcat.txt
FTE_Cellular.png

Keywords:
Toggle, button

Comment 1

[Warren Holdsworth]

Attachment: FTE_Cellular.png

1.3 Environmental Variables:
Device: Buri 1.3 MOZ
BuildID: 20140514024003
Gaia: 96e3fa769a436a2182e6d54088fb41386eb2b5b5
Gecko: 685cf1d0dedb
Version: 28.0
Firmware Version: v1.2-device.cfg

Comment 2

[Warren Holdsworth]

Switches are right aligned in FTE 1.3 MOZ RIL

Comment 3

[Stephany Wilkes]

Attachment: 2014-09-23-18-18-01.png

Ahmed, I believe this bug is fixed, per my screenshot. Can you please verify that this screen looks correct and that this is what you would expect to see here, from the toggle position and state? Thank you!

Comment 4

[Stephany Wilkes]

Ahmed, any update on this? Can we resolve as fixed based on my screenshot? Thanks!

Comment 5

[Peter Bylenga [:PBylenga]]

Mass Edit: adding the [rtl-meta]

Comment 6

[Elie]

Accordding to comment3
This issue Verified successfully on flame 2.2:
Gaia-Rev        f5b3d1b6cfa3e702033f613915ae637cb735cbfb
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/5d7497ce4cc7
Build-ID        20150120002507
Version         37.0a2
Device-Name     flame
FW-Release      4.4.2

Comment 7

[Elie]

Attachment: 2015-01-20-19-02-41.png

Comment 8

[Delphine Lebédel [:delphine - Use needinfo]]

As per UX spec page 10 (https://mozilla.app.box.com/bidi-22/1/2653676065/25045646579/1), the screenshot in comment 7 is incorrect RTL behavior:
* toggles should be left-aligned in RTL. On screenshot in comment 7 it is still right-aligned
* text should be right aligned in RTL: so text presently at the left of the toggle should be right-aligned
* buttons have been swiped on the above screenshot compared to Stephany's screenshot in comment 3. Not sure what the decision was on this one in the end. These buttons show direction so I would assume they would be mirrored as per UX specs. But since this is in FTE I'm not sure swaping directions would make sense. 

Thus leaving the ni on Ahmed and reopening, because it doesn't seem this one has been resolved. Thanks

Comment 12

[Sam Foster [:sfoster] (he/him)]

We're currently using gaia-switch for these toggle buttons, which is an early web-component-based implementation (not using gaia-component) https://github.com/mozilla-b2g/gaia/tree/master/shared/elements/gaia_switch

The equivalent screens and controls in Settings seem to be RTL-ready, so I'm leaning towards bringing the markup and CSS into line with Settings' implementation (which is older building-block code)

Comment 13

[Lancy(Leave from Mozilla)]

Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/15452/

Comment 14

[Sam Foster [:sfoster] (he/him)]

I'm working on this. My current approach is to ditch gaia-switch and more closely align the markup and css for these toggle switches with that used in Settings (which works great in RTL)

Comment 16

[GitHub Autolander Bot]

Attachment: [PullReq] sfoster:ftu-rtl-toggle-bug-980671 to mozilla-b2g:master

Comment 17

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8558717 [details] [review]
[PullReq] sfoster:ftu-rtl-toggle-bug-980671 to mozilla-b2g:master

Ditch the gaia-switch component in favor of label.pack-switch for consistency with Settings app

Comment 18

[Fernando Campo (:fcampo)]

Comment on attachment 8558717 [details] [review]
[PullReq] sfoster:ftu-rtl-toggle-bug-980671 to mozilla-b2g:master

works great with both arabic and mirrored, thanks Sam!

Comment 19

[Sam Foster [:sfoster] (he/him)]

Merged to master: https://github.com/mozilla-b2g/gaia/commit/bc412ba2f87dcd5e3bac96f448338d8ff0ba1ba7

Comment 20

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8558717 [details] [review]
[PullReq] sfoster:ftu-rtl-toggle-bug-980671 to mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): RTL
[User impact] if declined: All toggle switches in FTU app are left aligned and/or misaligned in an RTL language
[Testing completed]: Testing on device in Mirrored and Arabic (as well as latin ) languages. Gaia-Try all green
[Risk to taking this patch] (and alternatives if risky): Minimal risk
[String changes made]: None

Comment 21

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/a52999ce7f783177deb17e267bf003a53e6fde06

Comment 22

[Brogan Zumwalt [Inactive]]

Issue verified fixed on Flame 2.2 and 3.0

With language set to Arabic, the Cell Data switch is left aligned and text on page is right aligned.

Device: Flame 2.2
Build ID: 20150205002503
Gaia: a52999ce7f783177deb17e267bf003a53e6fde06
Gecko: adfba0a07e9b
Version: 37.0a2 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Device: Flame 3.0 Master
Build ID: 20150205010209
Gaia: 2b83a6d5d1185a438b5bbd287497ac2743b501db
Gecko: 34a66aaaca81
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Comment 23

[GitHub Autolander Bot]

Attachment: [PullReq] sfoster:backout-1010615 to mozilla-b2g:master

Comment 24

[Kevin Grandon :kgrandon]

Any idea what we need to do to make gaia-switch work RTL? Eventually we want to get rid of switches.css in favor of the web component.

Comment 25

[Sam Foster [:sfoster] (he/him)]

Patch got backed out for bug 1129922
https://github.com/mozilla-b2g/gaia/commit/7cc494249ecfc3e629f281b846d03844057f8975

Think I found the issue, will attach a new PR.

Comment 26

[GitHub Autolander Bot]

Attachment: [PullReq] sfoster:ftu-rtl-switches-bug-1010615 to mozilla-b2g:master

Comment 27

[Sam Foster [:sfoster] (he/him)]

Issue was a missing 'checked' attribute for the geolocation switch, my bad. :viorela, heads-up that I'll carry r+ and a+ and intend to re-land this once Gaia-Try looks good.

Comment 28

[Sam Foster [:sfoster] (he/him)]

I have the test_ftu_skip_tour.py test running on device here now, and I can see what's going on. PR is updated, will land if/when its Gaia-Try is green

Comment 29

[Sam Foster [:sfoster] (he/him)]

Carried r+ and +a from attachment #8558717 [details] [review]
Merged: https://github.com/mozilla-b2g/gaia/commit/bbc853ff4d1bffad41d19bb47e066e3ba21c6709
Cset: 6ae7ce4e9c88018efaefde40d6a5c12ca802be9e

Comment 30

[Kevin Grandon :kgrandon]

Sam - as I asked before, any idea what we need to do to make gaia-switch work RTL? Eventually we want to get rid of switches.css in favor of the web component. This is kind of going backwards in a way, so if the component is missing something, we should file some bugs to get this added. Thanks!

Comment 31

[Sam Foster [:sfoster] (he/him)]

(In reply to Kevin Grandon :kgrandon from comment #30)
> Sam - as I asked before, any idea what we need to do to make gaia-switch
> work RTL? Eventually we want to get rid of switches.css in favor of the web
> component. This is kind of going backwards in a way, so if the component is
> missing something, we should file some bugs to get this added. Thanks!

Sorry, I went looking for the bug and forgot to come back and respond! There's some discussion in bug 980671, specifically "RTL and <style> inside shadow-dom is a tricky one at the moment. We're missing the `:host-context()` selector (bug 1082060) and `-moz-dir()` doesn't reflect the parent document's direction (bug 1100912)." But it seems the new newness for the gaia-components is slated for v3, and with registerElement also in flux this was going to be a moving target and for v2.2 getting consistency with Settings and a known-to-work strategy was a better option.  

I've filed a follow-up bug 1130665.

Comment 32

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8560108 [details] [review]
[PullReq] sfoster:ftu-rtl-switches-bug-1010615 to mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): FTU/RTL feature
[User impact] if declined: In RTL languages, toggle switches are misaligned
[Testing completed]: On device, Gaia-Try, backed out, combed over and re-tested
[Risk to taking this patch] (and alternatives if risky): Low
[String changes made]: None

Comment 34

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/5886d307bcdb6c915035aeb94166571e13987450

Comment 35

[bhavana bajaj [:bajaj]]

Requesting verification on 3.0 build before we pick this up for 2.2 given we have had a fallout from this last time.

Comment 36

[Brogan Zumwalt [Inactive]]

Issue verified fixed on Flame 3.0

When language is set to Arabic, the checkbox on the Cell Data screen is left aligned while the text itself is right aligned. Geolocation screen shows checkbox mirrored, checked on by default, and no part of checkbox is cut off by screen (and switch itself is not covering any part of text)
	
Bug 1107062 that was marked as a dupe of this bug still occurs, but does not appear to be related due to the fact that the entirety of FxA in FTE has incorrect RTL formatting and is covered by bug 1127760 (which is blocking FTE-rtl)

Leaving verifyme for 2.2 verification

Device: Flame 3.0 Master
Build ID: 20150209010211
Gaia: 0d7b35f23402c4cb29bca6b98280fec48a196dec
Gecko: 3436787a82d0
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Comment 37

[Coler]

This issue has verified successfully on Flame 2.2.
Attachment:Verify_RTL_Reset.png.

Flame 2.2 build:

Build ID               20150209162506
Gaia Revision          21cce750c095a3f815275fe5439fa9dbfe3dfc6b
Gaia Date              2015-02-09 21:57:53
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/41ccc5328740
Gecko Version          37.0a2
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150209.194557
Firmware Date          Mon Feb  9 19:46:08 EST 2015
Bootloader             L1TC000118D0

Comment 38

[Coler]

Attachment: Verify_RTL_Reset.png

Comment 39

[Sam Foster [:sfoster] (he/him)]

This looks like the v2.2 uplift: https://github.com/mozilla-b2g/gaia/commit/5886d307bcdb6c915035aeb94166571e13987450

Clearing ni flags