1106835 - [Settings][RTL] Follow-up bug, Fixed security translations with a proper way

Status: RESOLVED FIXED

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

Description

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

In bug 1102158, we used a bad way to fix RTL problems because browsers will *guess* the current context and decide how to render the texts.

And for this bug, we will try to fix it by the way Zibi provided.

Comment 1

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

Attachment: patch on master

Comment 2

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

:Stas, on bug 1102158, Zibi posted a long comment (suggested by you) about how we can solve this problem by using `XXX.innerHTML` in locales. For this bug, I did try the trick you provided and it works basically. But for select/option case, I think this is a default browser behavior that it would remove all mockups and only strings would be treated on it. (And I think that's the reason why you would come up with this hack in shared/language_list.js here https://github.com/mozilla-b2g/gaia/blob/master/shared/js/language_list.js#L109)

So, for my case, if we check from <select>/<option>, it would still show strings like "(WPA2 (AES" instead of "WPA (AES)" in Arabic. But for <span>, this did fix the problem.

Any ideas for this case would be very helpful ! Thanks ++

Comment 3

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

And also, if you want to try latest results, you can apply my patch and see what's changed in Internet_ sharing/wifi_setings.

Comment 4

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

Zibi, if you can help too, please feel free to leave some comments here and I would really appreciate it !

Comment 5

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

Hi Ej, I'm wondering if it is possible that it is a Gecko bug with <option><bdi>foo</bdi></option>.

Unfortunately I feel that my understanding of <bdi> tag is desperately limited. I tried to test it using jsbin - http://jsbin.com/coriqumite/2/edit - and it does seem like the behavior in <option> is different than in <h1>. And it is consistent across Chrome, Firefox and Safari :(

Let's wait for Stas or someone who knows more about rtl spec to judge if that's an expected behavior and how to workaround it.

Comment 6

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

(In reply to Zibi Braniecki [:gandalf] from comment #5)
> Hi Ej, I'm wondering if it is possible that it is a Gecko bug with
> <option><bdi>foo</bdi></option>.
> 
> Unfortunately I feel that my understanding of <bdi> tag is desperately
> limited. I tried to test it using jsbin - http://jsbin.com/coriqumite/2/edit
> - and it does seem like the behavior in <option> is different than in <h1>.
> And it is consistent across Chrome, Firefox and Safari :(
> 

Yeah, that's why I asked at first about is it possible to use this trick in select/option, but based on your experiments & mine, it seems we can't fix this special case with XXX.innerHTML hack in locales.

> Let's wait for Stas or someone who knows more about rtl spec to judge if
> that's an expected behavior and how to workaround it.

Sure ! Hope Stas or someone can guide us on this special case.

Thanks Zibi.

Comment 7

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

CCing Ahmed and Manel to put this bug on their radar.

Simon, how does the jsbin example from comment 5 look to you?

Comment 8

[Simon Montagu :smontagu]

<option> isn't permitted to have sub-elements, so I expect the parser is throwing away the <bdi>s

In general the problems with parentheses should be fixed by bug 922963, which means that this ultimately depends on bug 866301. In the meanwhile, wrapping the test in LRE/PDF is probably the best workaround.

Comment 9

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

I think there is no much works I can do in Gaia for this special case, so let's keep this bug in open discussion and wait for bug 922963 to be fixed.

Comment 10

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

Let's actually wait for Stas (he's back next week I believe), to see if there's a more graceful way to handle RTL here :)

Comment 11

[Staś Małolepszy :stas]

I wasn't aware of that fact that <option> isn't permitted to have child elements.  Let's use LRE and PDF as HTML entities instead?

  # LOCALIZATION NOTE: LRE (&#x202a;) and PDF (&#x202c;) entities are used to ensure
  # proper directionality of the parenthesis, regardless of the direction of the document.

  hotspot-wpa-psk.innerHTML = &#x202a;WPA (TKIP)&#x202c;
  hotspot-wpa2-psk.innerHTML = &#x202a;WPA2 (AES)&#x202c;

Comment 12

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

Ej, will that work for you? We'd like this to be included in 2.2 before we send start the localization phase.

Comment 13

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

Okay, let's keep using LRE / PDF strings to hack this before related bugs got fixed.

I already updated the patch and let's wait for Arthur's review here, thanks guys.

Comment 14

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

Comment on attachment 8536421 [details] [review]
patch on master

Arthur, let's still use special characters to hack this and based on Zibi / Stats' suggestions, we can handle these on locale files instead of codes.

Please help me review this patch when you have time. Thanks :)

Comment 15

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

Comment on attachment 8536421 [details] [review]
patch on master

Looks good to me, r=me! Thanks.

Comment 16

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

Zibi, I just got r+ for this patch but it's so late that 2.2 has been branched out. Do you think we need to ask for 2.2 approval for this one ?

Comment 17

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

My guess is yes, because it will enable localizers to translate this new strings properly and not having to update them again in 2.3, but I'll let Flod decide since he knows better :)

Comment 18

[Francesco Lodolo [:flod]]

The current localization note is basically a "note for developers on why we need to do this $*#@".

Localizers don't actually know what settings.ar.properties or l10n.js are, they eventually know of settings.properties for their locale. Also Arabic -> RTL

Alternative comment

# LOCALIZATION NOTE (hotspot-wpa-psk.innerHTML, hotspot-wpa2-psk.innerHTML):
# Firefox OS is not able to render these strings correctly for RTL locales like
# Arabic. To work around this issue RTL locales need to add special characters
# (‪ and ‬) to their translations to force the correct direction
# of the text.
#
# RTL locales should have a translation similar to this:
# hotspot-wpa-psk.innerHTML= ‪WPA (TKIP)‬
# hotspot-wpa2-psk.innerHTML= ‪WPA2 (AES)‬

This would be useful for 2.2 if RTL is in scope (I believe it is). As for the mechanics of uplift approval, afaik we're not blocking patches with strings for 2.2 yet.

Comment 19

[Staś Małolepszy :stas]

I suggested a (hopefully) succinct localization note in comment 11, if you'd like to use it.

Comment 20

[Francesco Lodolo [:flod]]

(In reply to Staś Małolepszy :stas from comment #19)
> I suggested a (hopefully) succinct localization note in comment 11, if you'd
> like to use it.

That assumes that en-US uses those special characters, but it doesn't (should it?).

Comment 21

[Staś Małolepszy :stas]

I won't hurt and keeps things consistent.

Comment 22

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

ah, localization files are not only for localizers but also for developers, if my comment is not so understandable, I can put one more additional part for localizers.(based on stas's comment or flod's comment)

Comment 23

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

Thanks all, merged at gaia/master: https://github.com/mozilla-b2g/gaia/commit/ebc90190771a945d405f5d36efd813db6f77f965

Comment 24

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

Comment on attachment 8536421 [details] [review]
patch on master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): no
[User impact] if declined: no specific impact
[Testing completed]: no, we don't need any special test case for this patch because we moved out all related logics from code to locale files.
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: yes

Notes: For users, they won't get noticed directly what got changed in this patch. But with this patch, we can enable localizers to translate this new strings properly and not having to update them again in 2.3.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

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