1102193 - Re-implement MozLoopService localization code

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect, P2)

Description

[Mike de Boer [:mikedeboer]]

- The localizedStrings property of MozLoopServiceInternal is implemented around the outdated assumption that the locale keys may contains dots ('.') to discern various types of strings, thus populates the cache with objects unnecessarily.
 - The getStrings() method of MozLoopService therefore returns objects, instead of strings which its name tends to suggest, and even serializes them to JSON.

This is all needlessly complicated; there are no locale key names with dots in loop.properties, so we can just cache a map of strings. If we were to introduce a key with dots, the current implementation would yield confusing results.

Comment 1

[Mark Banner (:standard8)]

The reason we do it this way is to be compatible with content/libs/l10n.js which expects this format - we use l10n.js as a wrapper between chrome & content; to work and provide a similar interface to the gaia one that standalone uses.

We haven't used the dot format in loop.properties as we're not using the l10n.js code to translate elements - as we're filling the strings in dynamically.

If we can keep some of the existing interface to the content code, and simplify the backend, I'm all for that.

Comment 2

[Mike de Boer [:mikedeboer]]

Attachment: Patch v1: simplify l10n code in MozLoopService

Comment 3

[Marco Mucci [:MarcoM]]

Added to IT 36.3

Comment 4

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Comment on attachment 8526028 [details] [diff] [review]
Patch v1: simplify l10n code in MozLoopService

Review of attachment 8526028 [details] [diff] [review]:
-----------------------------------------------------------------

Looks much better! Ship it :)

::: browser/components/loop/MozLoopService.jsm
@@ +681,5 @@
>    /**
>     * A getter to obtain and store the strings for loop. This is structured
>     * for use by l10n.js.
>     *
> +   * @returns {Map} a map of element ids with localized string values

Nit: Missing full stop at line end.

@@ -709,3 @@
>      }
>  
> -    return gLocalizedStrings = map;

Wow. Thanks for removing this.

Comment 5

[Mike de Boer [:mikedeboer]]

Thanks Niko!

https://hg.mozilla.org/integration/fx-team/rev/2bb6548450f7

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2bb6548450f7

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8526028 [details] [diff] [review]
Patch v1: simplify l10n code in MozLoopService

Approval Request Comment
[Risks and why]: Loop rooms code needed for 35 pref-on.  Avoids massive annoyance uplifting any other patches
Low risk, simplifies things
[String/UUID change made/needed]: none

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5c877d79d8a3