Closed Bug 1106835 Opened 10 years ago Closed 10 years ago

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

Categories

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

x86
macOS
defect
Not set
normal

Tracking

(b2g-v2.2 fixed, b2g-master fixed)

RESOLVED FIXED
2.2 S4 (23jan)
Tracking Status
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: eragonj, Assigned: eragonj)

References

Details

Attachments

(1 file)

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.
: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 ++
Flags: needinfo?(stas)
And also, if you want to try latest results, you can apply my patch and see what's changed in Internet_ sharing/wifi_setings.
Zibi, if you can help too, please feel free to leave some comments here and I would really appreciate it !
Flags: needinfo?(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.
Flags: needinfo?(gandalf)
(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.
CCing Ahmed and Manel to put this bug on their radar. Simon, how does the jsbin example from comment 5 look to you?
Flags: needinfo?(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.
Depends on: 922963
Flags: needinfo?(smontagu)
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.
Flags: needinfo?(stas)
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 :)
Flags: needinfo?(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;
Flags: needinfo?(stas)
Ej, will that work for you? We'd like this to be included in 2.2 before we send start the localization phase.
Flags: needinfo?(ejchen)
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.
Flags: needinfo?(ejchen)
Comment on attachment 8536421 [details] 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 :)
Attachment #8536421 - Flags: review?(arthur.chen)
Comment on attachment 8536421 [details] patch on master Looks good to me, r=me! Thanks.
Attachment #8536421 - Flags: review?(arthur.chen) → review+
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 ?
Flags: needinfo?(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 :)
Flags: needinfo?(gandalf) → needinfo?(francesco.lodolo)
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 # (&#x202a; and &#x202c;) to their translations to force the correct direction # of the text. # # RTL locales should have a translation similar to this: # hotspot-wpa-psk.innerHTML= &#x202a;WPA (TKIP)&#x202c; # hotspot-wpa2-psk.innerHTML= &#x202a;WPA2 (AES)&#x202c; 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.
Flags: needinfo?(francesco.lodolo)
I suggested a (hopefully) succinct localization note in comment 11, if you'd like to use it.
(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?).
I won't hurt and keeps things consistent.
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)
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment on attachment 8536421 [details] 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.
Attachment #8536421 - Flags: approval-gaia-v2.2?
Attachment #8536421 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: