1151768 - [RTL][Settings]The "+" symbol is shown at wrong side of number in Call Forwarding view.

Status: VERIFIED FIXED

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

Description

[Sue]

Attachment: call_forwarding.png

[1.Description]:
[RTL][v2.2 & v3.0][Settings]The "+" symbol is shown at right side of number in Call Forwarding view.
See attachment:call_forwarding.png

[2.Testing Steps]: 
1. Insert an available SIM card into device and set your phone language to Arabic. 
2. Launch Settings app. 
3. Tap "Call Settings". 
4. Tap "SIM 1" or "SIM 2".
5. Tap "Call Forwarding".
6. Observe the number with "+" symbol under "Forward when busy" and "Forward when unreachable".

[3.Expected Result]: 
6. The "+" symbol should be shown at left side of number.

[4.Actual Result]: 
6. The "+" symbol is shown at right side of number.

[5.Reproduction build]: 
Device: Flame 2.2 (affected)
Build ID               20150406002503
Gaia Revision          a6351e1197d54f8624523c2db9ba1418f2aa046f
Gaia Date              2015-04-03 22:06:41
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/c3335a5d3063
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150406.040047
Firmware Date          Mon Apr  6 04:00:58 EDT 2015
Bootloader             L1TC000118D0

Device: Flame 3.0 (affected)
Build ID               20150406160205
Gaia Revision          834385f4c834238a4306bf87cc4be41615d91ff0
Gaia Date              2015-04-06 19:41:47
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/a530b5c3b713
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150406.194015
Firmware Date          Mon Apr  6 19:40:27 EDT 2015
Bootloader             L1TC000118D0

[6.Reproduction Frequency]: 
Always Recurrence,5/5

[7.TCID]: 
15996

Comment 1

[Delphine Lebédel [:delphine - use need info]]

This should have been blocking settings-rtl to be caught by triage.
Triage: P1 - nominating as this is not how phone numbers should be shown.

Comment 2

[howie [:howie]]

Triage: blocking.

Comment 3

[GitHub Autolander Bot]

Attachment: [gaia] EragonJ:bug-1151768 > mozilla-b2g:master

Comment 4

[GitHub Autolander Bot]

Attachment: [gaia] EragonJ:v2.2-bug-1151768 > mozilla-b2g:v2.2

Comment 5

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Hey Stas, I did read the topic you mentioned on dev gaia about l10n.js. But in order to make sure things are right, can you help me check whether these patches are ok or not ?

For master patch, I use <bdi> without .innerHTML in that string.
For 2.2 patch, I use <bdi> with .innerHTML in that string.

Hope this change make sense to you, thanks :)

Comment 6

[Staś Małolepszy :stas]

EJ, you're right about how this works and your pull requests should pass the tests and all, but in reality they might break our infra builds on pvt :(  Apparently we're still using an old version of Gecko somewhere and not using innerHTML explicitly breaks the builds.  I'm working on upgrading that builder.

For the time being, can you please use the old .innerHTML approach on master, too?  I'll remove it once all infra problems are dealt with—fortunately innerHTML is easy to grep for in properties files :)

Thanks!

Comment 7

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

ohh ! okay, i'll use innerHTML for both patches and feel free to update them later if the infra problems are solved !

Comment 8

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8589563 [details] [review]
[gaia] EragonJ:bug-1151768 > mozilla-b2g:master

Arthur, based on Stas's comment, we should all use .innerHTML right now no matter we are at 2.2 or master, so can you help me review this one-line change ? THanks !

Comment 9

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8589565 [details] [review]
[gaia] EragonJ:v2.2-bug-1151768 > mozilla-b2g:v2.2

Arthur, based on Stas's comment, we should all use .innerHTML right now no matter we are at 2.2 or master, so can you help me review this one-line change ? THanks !

Comment 10

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8589563 [details] [review]
[gaia] EragonJ:bug-1151768 > mozilla-b2g:master

Looking good to me, thanks.

Comment 11

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8589565 [details] [review]
[gaia] EragonJ:v2.2-bug-1151768 > mozilla-b2g:v2.2

The patch is exactly the same as the one against master, I think we can simply request approval using it.

Comment 12

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Thanks Arthur, this patch was merged into Gaia/master : https://github.com/mozilla-b2g/gaia/commit/91e3751d7251f405949df699f681d986c5a803dd

Comment 13

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8589563 [details] [review]
[gaia] EragonJ:bug-1151768 > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): no
[User impact] if declined: the direction of '+' symbol is wrong in RTL languages.
[Testing completed]: yes with manual test
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: yes, one line.

Just noticed that the string is frozen in 2.2, not sure whether this one-line change would be acceptable or not.

Comment 14

[Sue]

Attachment: correct_call_forwarding.png

This issue has been verified successfully on latest build of Flame 3.0 with the same steps in comment 0.
See attachment:correct_call_forwarding.png
Rate:0/5

Device: Flame 3.0 (pass)
Build ID               20150409160204
Gaia Revision          eaf69e651e19b98c096f8e63b9829fb31df50927
Gaia Date              2015-04-09 12:43:21
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/dd32e3ff3717
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150409.193255
Firmware Date          Thu Apr  9 19:33:06 EDT 2015
Bootloader             L1TC000118D0

Comment 15

[Ryan VanderMeulen [:RyanVM]]

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

Comment 16

[Sue]

Attachment: v2.2_verify.png

This issue has been verified successfully on latest build of Flame 2.2 with the same steps in comment 0.
See attachment:v2.2_verify.png
Rate:0/3

Device: Flame 2.2 (pass)
Build ID               20150412002502
Gaia Revision          cec00d643f517ffd96cde559cd3bbd43ab85816c
Gaia Date              2015-04-10 21:41:12
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/5005522fd68e
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150412.040730
Firmware Date          Sun Apr 12 04:07:41 EDT 2015
Bootloader             L1TC000118D0