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

RESOLVED FIXED in 2.2 S7 (6mar)

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: tchevalier, Assigned: tchevalier)

Tracking

({late-l10n})

unspecified
2.2 S7 (6mar)
x86_64
Linux

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.1 unaffected, b2g-v2.2 fixed, b2g-master verified)

Details

Attachments

(1 attachment)

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 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?
Attachment #8564651 - Flags: feedback?(francesco.lodolo)
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
Attachment #8564651 - Flags: feedback?(francesco.lodolo) → feedback+
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!
Flags: needinfo?(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.
Flags: needinfo?(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 :)
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 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? :)
Attachment #8564651 - Flags: review?(ehung)
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. :-)
Attachment #8564651 - Flags: feedback+
Tagging late-l10n, since we're in soft string-freeze on 2.2 starting today.
blocking-b2g: --- → 2.2?
Keywords: late-l10n
blocking-b2g: 2.2? → 2.2+

Comment 11

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

just a rubber stamp because kaze has f+. Thanks!
Attachment #8564651 - Flags: review?(ehung) → review+
Thanks Evelyn, landing
Keywords: checkin-needed
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
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
Attachment #8564651 - Flags: approval-gaia-v2.2?(bbajaj)
Attachment #8564651 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
You need to log in before you can comment on or make changes to this bug.