Bug 1455649: Localization.formatValues should accept an array of object. draft
authorDave Townsend <dtownsend@oxymoronical.com>
Wed, 25 Apr 2018 12:41:27 -0700
changeset 789240 556b02238ee3c9ffed5eac00e8f3357de9d10f43
parent 787142 26e53729a10976f52e75efa44e17b5e054969fec
child 789241 9010250fa9b8495e73effe38553ca17baf8a879b
push id108235
push userdtownsend@mozilla.com
push dateFri, 27 Apr 2018 21:27:33 +0000
bugs1455649
milestone61.0a1
Bug 1455649: Localization.formatValues should accept an array of object. Rather than accepting [[id, args], [id, args], ...] it would be clearer and easier to validate to use [{id, args}, {id, args}, ...]. This makes that change. MozReview-Commit-ID: DQmQJzzu2mj
browser/components/preferences/in-content/containers.js
browser/components/preferences/in-content/main.js
browser/components/preferences/in-content/preferences.js
browser/components/preferences/in-content/search.js
intl/l10n/DOMLocalization.jsm
intl/l10n/Localization.jsm
intl/l10n/docs/fluent_tutorial.rst
intl/l10n/test/test_localization.js
--- a/browser/components/preferences/in-content/containers.js
+++ b/browser/components/preferences/in-content/containers.js
@@ -74,20 +74,20 @@ let gContainersPane = {
   },
 
   async onRemoveCommand(button) {
     let userContextId = parseInt(button.getAttribute("value"), 10);
 
     let count = ContextualIdentityService.countContainerTabs(userContextId);
     if (count > 0) {
       let [title, message, okButton, cancelButton] = await document.l10n.formatValues([
-        ["containers-remove-alert-title"],
-        ["containers-remove-alert-msg", { count }],
-        ["containers-remove-ok-button"],
-        ["containers-remove-cancel-button"]
+        {id: "containers-remove-alert-title"},
+        {id: "containers-remove-alert-msg", args: { count }},
+        {id: "containers-remove-ok-button"},
+        {id: "containers-remove-cancel-button"}
       ]);
 
       let buttonFlags = (Ci.nsIPrompt.BUTTON_TITLE_IS_STRING * Ci.nsIPrompt.BUTTON_POS_0) +
                         (Ci.nsIPrompt.BUTTON_TITLE_IS_STRING * Ci.nsIPrompt.BUTTON_POS_1);
 
       let rv = Services.prompt.confirmEx(window, title, message, buttonFlags,
                                          okButton, cancelButton, null, null, {});
       if (rv != 0) {
--- a/browser/components/preferences/in-content/main.js
+++ b/browser/components/preferences/in-content/main.js
@@ -960,20 +960,20 @@ var gMainPane = {
     if (count == 0) {
       Services.prefs.setBoolPref("privacy.userContext.enabled", false);
       return;
     }
 
     let [
       title, message, okButton, cancelButton
     ] = await document.l10n.formatValues([
-      ["containers-disable-alert-title"],
-      ["containers-disable-alert-desc", { tabCount: count }],
-      ["containers-disable-alert-ok-button", { tabCount: count }],
-      ["containers-disable-alert-cancel-button"]
+      {id: "containers-disable-alert-title"},
+      {id: "containers-disable-alert-desc", args: { tabCount: count }},
+      {id: "containers-disable-alert-ok-button", args: { tabCount: count }},
+      {id: "containers-disable-alert-cancel-button"}
     ]);
 
     let buttonFlags = (Ci.nsIPrompt.BUTTON_TITLE_IS_STRING * Ci.nsIPrompt.BUTTON_POS_0) +
       (Ci.nsIPrompt.BUTTON_TITLE_IS_STRING * Ci.nsIPrompt.BUTTON_POS_1);
 
     let rv = Services.prompt.confirmEx(window, title, message, buttonFlags,
       okButton, cancelButton, null, null, {});
     if (rv == 0) {
--- a/browser/components/preferences/in-content/preferences.js
+++ b/browser/components/preferences/in-content/preferences.js
@@ -340,22 +340,22 @@ const CONFIRM_RESTART_PROMPT_RESTART_NOW
 const CONFIRM_RESTART_PROMPT_CANCEL = 1;
 const CONFIRM_RESTART_PROMPT_RESTART_LATER = 2;
 async function confirmRestartPrompt(aRestartToEnable, aDefaultButtonIndex,
                                     aWantRevertAsCancelButton,
                                     aWantRestartLaterButton) {
   let [
     msg, title, restartButtonText, noRestartButtonText, restartLaterButtonText
   ] = await document.l10n.formatValues([
-    [aRestartToEnable ?
-      "feature-enable-requires-restart" : "feature-disable-requires-restart"],
-    ["should-restart-title"],
-    ["should-restart-ok"],
-    ["cancel-no-restart-button"],
-    ["restart-later"],
+    {id: aRestartToEnable ?
+      "feature-enable-requires-restart" : "feature-disable-requires-restart"},
+    {id: "should-restart-title"},
+    {id: "should-restart-ok"},
+    {id: "cancel-no-restart-button"},
+    {id: "restart-later"},
   ]);
 
   // Set up the first (index 0) button:
   let buttonFlags = (Services.prompt.BUTTON_POS_0 *
                      Services.prompt.BUTTON_TITLE_IS_STRING);
 
 
   // Set up the second (index 1) button:
--- a/browser/components/preferences/in-content/search.js
+++ b/browser/components/preferences/in-content/search.js
@@ -356,21 +356,21 @@ var gSearchPane = {
           eduplicate = true;
           dupName = engine.name;
           break;
         }
       }
 
       // Notify the user if they have chosen an existing engine/bookmark keyword
       if (eduplicate || bduplicate) {
-        let msgids = [["search-keyword-warning-title"]];
+        let msgids = [{id: "search-keyword-warning-title"}];
         if (eduplicate) {
-          msgids.push(["search-keyword-warning-engine", { name: dupName }]);
+          msgids.push({id: "search-keyword-warning-engine", args: { name: dupName }});
         } else {
-          msgids.push(["search-keyword-warning-bookmark"]);
+          msgids.push({id: "search-keyword-warning-bookmark"});
         }
 
         let [dtitle, msg] = await document.l10n.formatValues(msgids);
 
         Services.prompt.alert(window, dtitle, msg);
         return false;
       }
     }
--- a/intl/l10n/DOMLocalization.jsm
+++ b/intl/l10n/DOMLocalization.jsm
@@ -613,17 +613,17 @@ class DOMLocalization extends Localizati
       // operations during startup.
       // For details see bug 1441037, bug 1442262, and bug 1363862.
 
       // A sparse array which will store translations separated out from
       // all translations that is needed for DOM Overlay.
       const overlayTranslations = [];
 
       const getTranslationsForItems = async l10nItems => {
-        const keys = l10nItems.map(l10nItem => [l10nItem.l10nId, l10nItem.l10nArgs]);
+        const keys = l10nItems.map(l10nItem => ({ id: l10nItem.l10nId, args: l10nItem.l10nArgs }));
         const translations = await this.formatMessages(keys);
 
         // Here we want to separate out elements that require DOM Overlays.
         // Those elements will have to be translated using our JS
         // implementation, while everything else is going to use the fast-path.
         for (const [i, translation] of translations.entries()) {
           if (translation === undefined) {
             continue;
@@ -727,17 +727,17 @@ class DOMLocalization extends Localizati
    * Get the `data-l10n-*` attributes from DOM elements as a two-element
    * array.
    *
    * @param {Element} element
    * @returns {Array<string, Object>}
    * @private
    */
   getKeysForElement(element) {
-    return [
-      element.getAttribute(L10NID_ATTR_NAME),
-      JSON.parse(element.getAttribute(L10NARGS_ATTR_NAME) || null)
-    ];
+    return {
+      id: element.getAttribute(L10NID_ATTR_NAME),
+      args: JSON.parse(element.getAttribute(L10NARGS_ATTR_NAME) || null)
+    };
   }
 }
 
 this.DOMLocalization = DOMLocalization;
 var EXPORTED_SYMBOLS = ["DOMLocalization"];
--- a/intl/l10n/Localization.jsm
+++ b/intl/l10n/Localization.jsm
@@ -127,17 +127,17 @@ class Localization {
 
   /**
    * Format translations and handle fallback if needed.
    *
    * Format translations for `keys` from `MessageContext` instances on this
    * DOMLocalization. In case of errors, fetch the next context in the
    * fallback chain.
    *
-   * @param   {Array<Array>}          keys    - Translation keys to format.
+   * @param   {Array<Object>}         keys    - Translation keys to format.
    * @param   {Function}              method  - Formatting function.
    * @returns {Promise<Array<string|Object>>}
    * @private
    */
   async formatWithFallback(keys, method) {
     const translations = [];
 
     for await (let ctx of this.ctxs) {
@@ -165,18 +165,18 @@ class Localization {
   /**
    * Format translations into {value, attrs} objects.
    *
    * The fallback logic is the same as in `formatValues` but the argument type
    * is stricter (an array of arrays) and it returns {value, attrs} objects
    * which are suitable for the translation of DOM elements.
    *
    *     docL10n.formatMessages([
-   *       ['hello', { who: 'Mary' }],
-   *       ['welcome', undefined]
+   *       {id: 'hello', args: { who: 'Mary' }},
+   *       {id: 'welcome'}]
    *     ]).then(console.log);
    *
    *     // [
    *     //   { value: 'Hello, Mary!', attrs: null },
    *     //   { value: 'Welcome!', attrs: { title: 'Hello' } }
    *     // ]
    *
    * Returns a Promise resolving to an array of the translation strings.
@@ -191,19 +191,19 @@ class Localization {
 
   /**
    * Retrieve translations corresponding to the passed keys.
    *
    * A generalized version of `DOMLocalization.formatValue`. Keys can
    * either be simple string identifiers or `[id, args]` arrays.
    *
    *     docL10n.formatValues([
-   *       ['hello', { who: 'Mary' }],
-   *       ['hello', { who: 'John' }],
-   *       ['welcome']
+   *       {id: 'hello', args: { who: 'Mary' }},
+   *       {id: 'hello', args: { who: 'John' }},
+   *       {id: 'welcome'}
    *     ]).then(console.log);
    *
    *     // ['Hello, Mary!', 'Hello, John!', 'Welcome!']
    *
    * Returns a Promise resolving to an array of the translation strings.
    *
    * @param   {Array<Array>} keys
    * @returns {Promise<Array<string>>}
@@ -230,17 +230,17 @@ class Localization {
    * retranslated when the user changes their language preferences, e.g. in
    * notifications.
    *
    * @param   {string}  id     - Identifier of the translation to format
    * @param   {Object}  [args] - Optional external arguments
    * @returns {Promise<string>}
    */
   async formatValue(id, args) {
-    const [val] = await this.formatValues([[id, args]]);
+    const [val] = await this.formatValues([{id, args}]);
     return val;
   }
 
   /**
    * Register weak observers on events that will trigger cache invalidation
    */
   registerObservers() {
     Services.obs.addObserver(this, "intl:app-locales-changed", true);
@@ -374,27 +374,27 @@ function messageFromContext(ctx, errors,
  *
  * @returns {Set<string>}
  * @private
  */
 function keysFromContext(method, ctx, keys, translations) {
   const messageErrors = [];
   const missingIds = new Set();
 
-  keys.forEach((key, i) => {
+  keys.forEach(({ id, args = undefined }, i) => {
     if (translations[i] !== undefined) {
       return;
     }
 
-    if (ctx.hasMessage(key[0])) {
+    if (ctx.hasMessage(id)) {
       messageErrors.length = 0;
-      translations[i] = method(ctx, messageErrors, key[0], key[1]);
+      translations[i] = method(ctx, messageErrors, id, args);
       // XXX: Report resolver errors
     } else {
-      missingIds.add(key[0]);
+      missingIds.add(id);
     }
   });
 
   return missingIds;
 }
 
 this.Localization = Localization;
 var EXPORTED_SYMBOLS = ["Localization"];
--- a/intl/l10n/docs/fluent_tutorial.rst
+++ b/intl/l10n/docs/fluent_tutorial.rst
@@ -323,17 +323,17 @@ Non-Markup Localization
 -----------------------
 
 In rare cases, when the runtime code needs to retrieve the translation and not
 apply it onto the DOM, Fluent provides an API to retrieve it:
 
 .. code-block:: javascript
 
   let [ msg ] = await document.l10n.formatValues([
-    ["remove-containers-description"]
+    {id: "remove-containers-description"}
   ]);
 
   alert(msg);
 
 This model is heavily discouraged and should be used only in cases where the
 DOM annotation is not possible.
 
 .. note::
@@ -545,17 +545,17 @@ contexts manually using `Localization` c
   
   const myL10n = new Localization([
     "branding/brand.ftl",
     "browser/preferences/preferences.ftl"
   ]);
   
   
   let [isDefaultMsg, isNotDefaultMsg] =
-    myL10n.formatValues(["is-default", "is-not-default"]);
+    myL10n.formatValues({id: "is-default"}, {id: "is-not-default"});
 
 
 .. admonition:: Example
 
   An example of a use case is the Preferences UI in Firefox which uses the
   main context to localize the UI but also to build a search index.
 
   It is common to build such search index both in a current language and additionally
--- a/intl/l10n/test/test_localization.js
+++ b/intl/l10n/test/test_localization.js
@@ -32,17 +32,17 @@ add_task(async function test_methods_cal
   async function* generateMessages(resIds) {
     yield * await L10nRegistry.generateContexts(["de", "en-US"], resIds);
   }
 
   const l10n = new Localization([
     "/browser/menu.ftl"
   ], generateMessages);
 
-  let values = await l10n.formatValues([["key"], ["key2"]]);
+  let values = await l10n.formatValues([{id: "key"}, {id: "key2"}]);
 
   equal(values[0], "[de] Value2");
   equal(values[1], "[en] Value3");
 
   L10nRegistry.sources.clear();
   L10nRegistry.load = originalLoad;
   Services.locale.setRequestedLocales(originalRequested);
 });
@@ -78,16 +78,16 @@ key = { PLATFORM() ->
   async function* generateMessages(resIds) {
     yield * await L10nRegistry.generateContexts(["en-US"], resIds);
   }
 
   const l10n = new Localization([
     "/test.ftl"
   ], generateMessages);
 
-  let values = await l10n.formatValues([["key"]]);
+  let values = await l10n.formatValues([{id: "key"}]);
 
   ok(values[0].includes(
     `${ known_platforms[AppConstants.platform].toUpperCase() } Value`));
 
   L10nRegistry.sources.clear();
   L10nRegistry.load = originalLoad;
 });