1133274 - [Privacy Panel] Fix l10n (Blur distance not displayed, L10n JS errors, English quotes)

Status: RESOLVED FIXED

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

Description

[Théo Chevalier]

First, when setting an adjusted position in Privacy controls > Location Accuracy, the blur distance that used to be displayed above the blur slider is now missing.
It should also be displayed when adjusting position for Exceptions.

Also a couple of strings are in the templates, but are missing from .properties files (ala-panel, location-adjustment-information) and are throwing JS errors. Since they don't exist, I simply removed them.

Finally, fixing quotes for gt-rp-passphrase-desc.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] TheoChevalier:privacy-panel-blur-l10n > mozilla-b2g:master

Comment 2

[Théo Chevalier]

Comment on attachment 8564651 [details] [review]
[gaia] TheoChevalier:privacy-panel-blur-l10n > mozilla-b2g:master

Actually, also fixed single quotes for a couple more strings (no need to update entity name in this case, localizers already use their own typographic rules).


Hey flod, I removed the generic {{ value }} {{ unit }} string, because that was avoiding us a call to l10n.get method and replacing value + unit for blur distance was the only use for this string.

However, I couldn't avoid calling l10n.get to get the translation of the unit. Do you see any way to avoid that?

Comment 3

[Francesco Lodolo [:flod]]

Comment on attachment 8564651 [details] [review]
[gaia] TheoChevalier:privacy-panel-blur-l10n > mozilla-b2g:master

String-wise it looks good to me, but I suggest to ask either :gandalf or :stas to take a quick look for the l10n.js bit

Comment 4

[Théo Chevalier]

Sure, thanks!

Hi Stas, do you know any trick to avoid calling .get() or is it acceptable here?
Is there an asynchronous way to fetch localized content into a variable?
Thanks!

Comment 5

[Staś Małolepszy :stas]

Since there are only two units involved, I'd probably go for putting the unit in the string, like so:

  blur-distance-km = {{ value }} km blur
  blur-distance-m = {{ value }} m blur

mozL10n.get is sometimes hard to avoid but in this case, it looks like it's worth it:  we end up with one string fewer and we make sure the string changes correctly on retranslation.

Comment 6

[Staś Małolepszy :stas]

(In reply to Staś Małolepszy :stas from comment #5)

> mozL10n.get is sometimes hard to avoid but in this case, it looks like it's
> worth it:  we end up with one string fewer and we make sure the string
> changes correctly on retranslation.

To elaborate a bit more on the retranslation thing:  with the following code:

  navigator.mozL10n.setAttributes(
    this.label,
    'blur-distance',
    { 
      value: lab.value,
      unit: navigator.mozL10n.get(l10nId)
    });

The translation for 'unit' will be fetched only once.  Suppose it's 'km' in the current language.  data-l10n-args will be set to { value: 1, unit: 'km' }.  The user then changes the language to Russian. The translation of 'unit' should now be 'км', but it has been already set in data-l10n-args and 'km' is used instead.  And that's why using .get is bad :)

Comment 7

[Théo Chevalier]

Thanks for the explanation stas, now I understand why it's so bad ;)
Updated the PR with your feedback, seeking review from Settings peer.

Comment 8

[Théo Chevalier]

Comment on attachment 8564651 [details] [review]
[gaia] TheoChevalier:privacy-panel-blur-l10n > mozilla-b2g:master

Hi Evelyn, could you take a look at this one please? :)

Comment 9

[Fabien Cazenave [:kaze]]

Comment on attachment 8564651 [details] [review]
[gaia] TheoChevalier:privacy-panel-blur-l10n > mozilla-b2g:master

I was about to fix that, as the l10n warnings were getting on my nerves. Thanks for doing the job Théo. :-)

Comment 10

[Théo Chevalier]

Tagging late-l10n, since we're in soft string-freeze on 2.2 starting today.

Comment 11

[Evelyn Hung]

Comment on attachment 8564651 [details] [review]
[gaia] TheoChevalier:privacy-panel-blur-l10n > mozilla-b2g:master

just a rubber stamp because kaze has f+. Thanks!

Comment 12

[Théo Chevalier]

Thanks Evelyn, landing

Comment 13

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/22230c9a340a0a2c3af73320cf47e1cb544517ad

Comment 14

[Théo Chevalier]

Comment on attachment 8564651 [details] [review]
[gaia] TheoChevalier:privacy-panel-blur-l10n > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: No way to guess the blur distance value for adjusted positions in Privacy Panel
[Testing completed]: Latest Flame master build
[Risk to taking this patch] (and alternatives if risky): very low
[String changes made]: yes, -2/+2 and fixed en-US only typography

Comment 15

[Ryan VanderMeulen [:RyanVM]]

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