Closed Bug 1102158 Opened 10 years ago Closed 10 years ago

[Settings][RTL] Internet sharing security text format is incorrect

Categories

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

x86
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(tracking-b2g:+, b2g-v2.2 verified)

VERIFIED FIXED
tracking-b2g +
Tracking Status
b2g-v2.2 --- verified

People

(Reporter: bhuang, Assigned: eragonj)

References

Details

Attachments

(2 files)

In Internet Sharing (Internet Sharing -> Hotspot Settings -> Security), the security options WPA (TKIP), WPA2 (AES) show up as (WPA(TKIP and (WPA2(AES .  The text formatting shouldn't need to change in this case.
blocking-b2g: --- → backlog
tracking-b2g: --- → +
Took this one.
Assignee: nobody → ejchen
Comment on attachment 8527502 [details] [review]
patch on master (2.2)

Arthur, can you help me review this patch ? Thanks !
Attachment #8527502 - Flags: review?(arthur.chen)
Comment on attachment 8527502 [details] [review]
patch on master (2.2)

Good work! r=me.
Attachment #8527502 - Flags: review?(arthur.chen) → review+
Thanks Arthur ! Merged into Gaia/master (2.2): https://github.com/mozilla-b2g/gaia/commit/1d7b0ba3cbf98ba14d583d30b6028384af996b5d
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
I commented on this on github but I'll repeat it here since I think it's important.

The best practice whenever dealing with bi-directional text is to wrap anything that can be written in a different direction than the host document with the bdi element:

  https://developer.mozilla.org/en-US/docs/Web/HTML/Element/bdi

which semantically ensures proper isolation of the directionality. See an example at http://codepen.io/stasm/pen/dPoQOY.

I also don't think that hardcoding Unicode marks for the 'Open' string is a good idea. Do we really want to use U+202B (right-to-left embedding, RLE) for all languages here? We also shouldn't make any assumptions about which language exaclty the string will be in because in case of errors we might fall back to a different language. For instance, if the string is missing in Arabic, we might show it in French or English.

Bug 994357 will make it possible to use <bdi> freely in translations, without any of the tricks that follow. Until then, we can:

    hardcode the bdi element in the source HTML and set data-l10n-id on it; this is hardly a good idea because technically we'd need to do that on every string everywhere. So another option is to…

    …explicitly state that the translation changes innerHTML, like this:

    hotspot-open=فتح
    hotspot-wpa-psk.innerHTML=<bdi>WPA (TKIP)</bdi>
    hotspot-wpa2-psk.innerHTML=<bdi>WPA2 (AES)</bdi>

Even though en-US doesn't require bdi here, we'll need to also specify innerHTML to make sure l10n infrastructure recognizes hotspot-wpa-psk.innerHTML as a valid string:

hotspot-open=open
hotspot-wpa-psk.innerHTML=WPA (TKIP)
hotspot-wpa2-psk.innerHTML=WPA2 (AES)

You can now safely use mozL10n.setAttributes to localize this in a declarative manner.

  https://developer.mozilla.org/en-US/docs/Web/API/L10n.setAttributes
  https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/localization_code_best_practices#Using_a_declarative_API

BTW. even though my example is adding attributes to hotspot-wpa-psk and hotspot-wpa2-psk there's no need to change the identifiers of those strings. L10n.js will do the right thing (bug 1091113).
Ej, do you want to revert this or follow up? This is a regression.
Flags: needinfo?(ejchen)
Thanks for providing these tips about RTL, I'll fill a follow-up bug for this, thanks Zibi.
Flags: needinfo?(ejchen)
Zibi, in this bug, our translated strings would be put inside <select>, does the trick you provided would work in such scenario (?)

And I didn't quite understand your comment below :

> BTW. even though my example is adding attributes to hotspot-wpa-psk and hotspot-wpa2-psk there's no need 
> to change the identifiers of those strings. L10n.js will do the right thing (bug 1091113).

And, If I don't modify identifiers, how do I tell l10n.js that these strings should be handled differently than the others in such scenario ? Should I do something special when using declarative API ?

I am not quite similar with l10n here, any information / tips would be thankful. Thanks :)
Flags: needinfo?(gandalf)
About the last point (string IDs). In the past we couldn't simply move from "hotspot-wpa2-psk" to "hotspot-wpa2-psk.innerHTML", because funny things happened: the old string would be picked up anyway and displayed because of the merge system.

So we were forced to have a "hotspot-wpa2-pskSOMETHING.innerHTML". Now we can simply have a new string called "hotspot-wpa2-psk.innerHTML".

The confusion is that for gandalf the string ID is "hotspot-wpa2-psk" in both cases, for me (and probably you) the string ID is the entire name ("hotspot-wpa2-psk.innerHTML"), hence the confusion.
Hi EJ, it was :stas that provided the solution, not me, actually :)

(In reply to EJ Chen [:eragonj][:小龍哥][ni? if you need me] from comment #9)
> Zibi, in this bug, our translated strings would be put inside <select>, does
> the trick you provided would work in such scenario (?)

Yeah, since select's node value is used, you should be able to use innerHTML the same way as on any other node with localizable content.

> And, If I don't modify identifiers, how do I tell l10n.js that these strings
> should be handled differently than the others in such scenario ? Should I do
> something special when using declarative API ?

As :flod pointed out, just use .innerHTML attribute:

hotspot-wpa-psk = WPA (TKIP)

becomes:

hotspot-wpa-psk.innerHTML=<bdi>WPA (TKIP)</bdi>


> I am not quite similar with l10n here, any information / tips would be
> thankful. Thanks :)

I recommend taking a look at the table of contents here: https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/localization_code_best_practices - we're working on this document to help you write localizable code :)

Let me know if you have more questions!
Flags: needinfo?(gandalf)
(In reply to Francesco Lodolo [:flod] from comment #10)
> About the last point (string IDs). In the past we couldn't simply move from
> "hotspot-wpa2-psk" to "hotspot-wpa2-psk.innerHTML", because funny things
> happened: the old string would be picked up anyway and displayed because of
> the merge system.
> 
> So we were forced to have a "hotspot-wpa2-pskSOMETHING.innerHTML". Now we
> can simply have a new string called "hotspot-wpa2-psk.innerHTML".
> 
> The confusion is that for gandalf the string ID is "hotspot-wpa2-psk" in
> both cases, for me (and probably you) the string ID is the entire name
> ("hotspot-wpa2-psk.innerHTML"), hence the confusion.

Yeah Francesco, you are right. That's why I got confused by that comment. Thanks for clarifying it for me :P
(In reply to Zibi Braniecki [:gandalf] from comment #11)
> Hi EJ, it was :stas that provided the solution, not me, actually :)
> 

Thanks :stas !

> (In reply to EJ Chen [:eragonj][:小龍哥][ni? if you need me] from comment #9)
> > Zibi, in this bug, our translated strings would be put inside <select>, does
> > the trick you provided would work in such scenario (?)
> 
> Yeah, since select's node value is used, you should be able to use innerHTML
> the same way as on any other node with localizable content.
> 
> > And, If I don't modify identifiers, how do I tell l10n.js that these strings
> > should be handled differently than the others in such scenario ? Should I do
> > something special when using declarative API ?
> 
> As :flod pointed out, just use .innerHTML attribute:
> 
> hotspot-wpa-psk = WPA (TKIP)
> 
> becomes:
> 
> hotspot-wpa-psk.innerHTML=<bdi>WPA (TKIP)</bdi>
> 

Yeah, I got it now, let me give it a try :)
> 
> > I am not quite similar with l10n here, any information / tips would be
> > thankful. Thanks :)
> 
> I recommend taking a look at the table of contents here:
> https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/
> localization_code_best_practices - we're working on this document to help
> you write localizable code :)
> 

Nice, I'll give it a check ! thanks

> Let me know if you have more questions!
Keywords: verifyme
Status: RESOLVED → VERIFIED
Keywords: verifyme
Attached image 2015-01-15-05-18-14.png
This issue does not exist on Flame 2.2
Gaia-Rev        7c5b27cad370db377b18a742d3f3fdb0070e899f
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/ce27f2692382
Build-ID        20150115002505
Version         37.0a2
Reproduce rate   0/5
Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/15675/
Flags: in-moztrap+
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: