Bug 1221415: [webext] Improve error checking and Chrome-compatibility of i18n API. r=billm
authorKris Maglione <maglione.k@gmail.com>
Sun, 15 Nov 2015 16:54:41 -0800
changeset 273009 4916558f00d5ccd519f1de94039fb417f0cf92be
parent 273008 ba16287438d4f4f6e3a5bdb43e00bda8cde4cfea
child 273010 fb5b48c3abe6dc120cde93464372d2bf9095a7a2
push id29692
push usercbook@mozilla.com
push dateWed, 18 Nov 2015 13:48:23 +0000
treeherdermozilla-central@f8cc032951d3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs1221415
milestone45.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1221415: [webext] Improve error checking and Chrome-compatibility of i18n API. r=billm
toolkit/components/extensions/Extension.jsm
toolkit/components/extensions/ExtensionUtils.jsm
toolkit/components/extensions/test/mochitest/mochitest.ini
toolkit/components/extensions/test/mochitest/test_ext_i18n.html
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -61,16 +61,17 @@ ExtensionManagement.registerScript("chro
 
 Cu.import("resource://gre/modules/ExtensionUtils.jsm");
 var {
   MessageBroker,
   Messenger,
   injectAPI,
   extend,
   flushJarCache,
+  instanceOf,
 } = ExtensionUtils;
 
 const LOGGER_ID_BASE = "addons.webextension.";
 
 var scriptScope = this;
 
 // This object loads the ext-*.js scripts that define the extension API.
 var Management = {
@@ -341,20 +342,20 @@ var GlobalManager = {
 // No functionality of this class is guaranteed to work before
 // |readManifest| has been called, and completed.
 this.ExtensionData = function(rootURI)
 {
   this.rootURI = rootURI;
 
   this.manifest = null;
   this.id = null;
-  // Map(locale-name -> message-map)
+  // Map(locale-name -> Map(message-key -> localized-strings))
+  //
   // Contains a key for each loaded locale, each of which is a
-  // JSON-compatible object with a property for each message
-  // in that locale.
+  // Map of message keys to their localized strings.
   this.localeMessages = new Map();
   this.selectedLocale = null;
   this._promiseLocales = null;
 
   this.errors = [];
 }
 
 ExtensionData.prototype = {
@@ -370,51 +371,46 @@ ExtensionData.prototype = {
 
   // Report an error about the extension's general packaging.
   packagingError(message) {
     this.errors.push(message);
     this.logger.error(`Loading extension '${this.id}': ${message}`);
   },
 
   // https://developer.chrome.com/extensions/i18n
-  localizeMessage(message, substitutions, locale = this.selectedLocale) {
-    let messages = {};
-    if (this.localeMessages.has(locale)) {
-      messages = this.localeMessages.get(locale);
-    }
+  localizeMessage(message, substitutions = [], locale = this.selectedLocale, defaultValue = "??") {
+    let locales = new Set([locale, this.defaultLocale]
+                          .filter(locale => this.localeMessages.has(locale)));
 
-    if (message in messages) {
-      let str = messages[message].message;
+    // Message names are case-insensitive, so normalize them to lower-case.
+    message = message.toLowerCase();
+    for (let locale of locales) {
+      let messages = this.localeMessages.get(locale);
+      if (messages.has(message)) {
+        let str = messages.get(message)
 
-      if (!substitutions) {
-        substitutions = [];
-      } else if (!Array.isArray(substitutions)) {
-        substitutions = [substitutions];
-      }
+        if (!Array.isArray(substitutions)) {
+          substitutions = [substitutions];
+        }
 
-      // https://developer.chrome.com/extensions/i18n-messages
-      // |str| may contain substrings of the form $1 or $PLACEHOLDER$.
-      // In the former case, we replace $n with substitutions[n - 1].
-      // In the latter case, we consult the placeholders array.
-      // The placeholder may itself use $n to refer to substitutions.
-      let replacer = (matched, name) => {
-        if (name.length == 1 && name[0] >= '1' && name[0] <= '9') {
-          return substitutions[parseInt(name) - 1];
-        } else {
-          let content = messages[message].placeholders[name].content;
-          if (content[0] == '$') {
-            return replacer(matched, content[1]);
+        let replacer = (matched, index, dollarSigns) => {
+          if (index) {
+            // This is not quite Chrome-compatible. Chrome consumes any number
+            // of digits following the $, but only accepts 9 substitutions. We
+            // accept any number of substitutions.
+            index = parseInt(index) - 1;
+            return index in substitutions ? substitutions[index] : "";
           } else {
-            return content;
+            // For any series of contiguous `$`s, the first is dropped, and
+            // the rest remain in the output string.
+            return dollarSigns;
           }
-        }
-      };
-      return str.replace(/\$([A-Za-z_@]+)\$/, replacer)
-                .replace(/\$([0-9]+)/, replacer)
-                .replace(/\$\$/, "$");
+        };
+        return str.replace(/\$(?:([1-9]\d*)|(\$+))/g, replacer);
+      }
     }
 
     // Check for certain pre-defined messages.
     if (message == "@@extension_id") {
       if ("uuid" in this) {
         // Per Chrome, this isn't available before an ID is guaranteed
         // to have been assigned, namely, in manifest files.
         // This should only be present in instances of the |Extension|
@@ -423,32 +419,32 @@ ExtensionData.prototype = {
       }
     } else if (message == "@@ui_locale") {
       return Locale.getLocale();
     } else if (message == "@@bidi_dir") {
       return "ltr"; // FIXME
     }
 
     Cu.reportError(`Unknown localization message ${message}`);
-    return "??";
+    return defaultValue;
   },
 
   // Localize a string, replacing all |__MSG_(.*)__| tokens with the
   // matching string from the current locale, as determined by
   // |this.selectedLocale|.
   //
   // This may not be called before calling either |initLocale| or
   // |initAllLocales|.
   localize(str, locale = this.selectedLocale) {
     if (!str) {
       return str;
     }
 
     return str.replace(/__MSG_([A-Za-z0-9@_]+?)__/g, (matched, message) => {
-      return this.localizeMessage(message, [], locale);
+      return this.localizeMessage(message, [], locale, matched);
     });
   },
 
   // If a "default_locale" is specified in that manifest, returns it
   // as a Gecko-compatible locale string. Otherwise, returns null.
   get defaultLocale() {
     if ("default_locale" in this.manifest) {
       return this.normalizeLocaleCode(this.manifest.default_locale);
@@ -563,26 +559,82 @@ ExtensionData.prototype = {
       if (typeof this.id != "string") {
         this.manifestError("Missing required `applications.gecko.id` property");
       }
 
       return manifest;
     });
   },
 
+  // Validates the contents of a locale JSON file, normalizes the
+  // messages into a Map of message key -> localized string pairs.
+  processLocale(locale, messages) {
+    let result = new Map();
+
+    // Chrome does not document the semantics of its localization
+    // system very well. It handles replacements by pre-processing
+    // messages, replacing |$[a-zA-Z0-9@_]+$| tokens with the value of their
+    // replacements. Later, it processes the resulting string for
+    // |$[0-9]| replacements.
+    //
+    // Again, it does not document this, but it accepts any number
+    // of sequential |$|s, and replaces them with that number minus
+    // 1. It also accepts |$| followed by any number of sequential
+    // digits, but refuses to process a localized string which
+    // provides more than 9 substitutions.
+    if (!instanceOf(messages, "Object")) {
+      this.packagingError(`Invalid locale data for ${locale}`);
+      return result;
+    }
+
+    for (let key of Object.keys(messages)) {
+      let msg = messages[key];
+
+      if (!instanceOf(msg, "Object") || typeof(msg.message) != "string") {
+        this.packagingError(`Invalid locale message data for ${locale}, message ${JSON.stringify(key)}`);
+        continue;
+      }
+
+      // Substitutions are case-insensitive, so normalize all of their names
+      // to lower-case.
+      let placeholders = new Map();
+      if (instanceOf(msg.placeholders, "Object")) {
+        for (let key of Object.keys(msg.placeholders)) {
+          placeholders.set(key.toLowerCase(), msg.placeholders[key]);
+        }
+      }
+
+      let replacer = (match, name) => {
+        let replacement = placeholders.get(name.toLowerCase());
+        if (instanceOf(replacement, "Object") && "content" in replacement) {
+          return replacement.content;
+        }
+        return "";
+      };
+
+      let value = msg.message.replace(/\$([A-Za-z0-9@_]+)\$/g, replacer);
+
+      // Message names are also case-insensitive, so normalize them to lower-case.
+      result.set(key.toLowerCase(), value);
+    }
+
+    return result;
+  },
+
   // Reads the locale file for the given Gecko-compatible locale code, and
   // stores its parsed contents in |this.localeMessages.get(locale)|.
   readLocaleFile: Task.async(function* (locale) {
     let locales = yield this.promiseLocales();
     let dir = locales.get(locale);
     let file = `_locales/${dir}/messages.json`;
 
-    let messages = {};
+    let messages = new Map();
     try {
       messages = yield this.readJSON(file);
+      messages = this.processLocale(locale, messages);
     } catch (e) {
       this.packagingError(`Loading locale file ${file}: ${e}`);
     }
 
     this.localeMessages.set(locale, messages);
     return messages;
   }),
 
@@ -636,26 +688,35 @@ ExtensionData.prototype = {
 
     return this.localeMessages;
   }),
 
   // Reads the locale file for the given Gecko-compatible locale code, or the
   // default locale if no locale code is given, and sets it as the currently
   // selected locale on success.
   //
+  // Pre-loads the default locale for fallback message processing, regardless
+  // of the locale specified.
+  //
   // If no locales are unavailable, resolves to |null|.
   initLocale: Task.async(function* (locale = this.defaultLocale) {
     if (locale == null) {
       return null;
     }
 
-    let localeData = yield this.readLocaleFile(locale);
+    let promises = [this.readLocaleFile(locale)];
 
+    let { defaultLocale } = this;
+    if (locale != defaultLocale && !this.localeMessages.has(defaultLocale)) {
+      promises.push(this.readLocaleFile(defaultLocale));
+    }
+
+    let results = yield Promise.all(promises);
     this.selectedLocale = locale;
-    return localeData;
+    return results[0];
   }),
 };
 
 // All moz-extension URIs use a machine-specific UUID rather than the
 // extension's own ID in the host component. This makes it more
 // difficult for web pages to detect whether a user has a given add-on
 // installed (by trying to load a moz-extension URI referring to a
 // web_accessible_resource from the extension). getExtensionUUID
@@ -775,17 +836,17 @@ this.Extension.generate = function(id, d
   if (data.background) {
     let uuidGenerator = Cc["@mozilla.org/uuid-generator;1"].getService(Ci.nsIUUIDGenerator);
     let bgScript = uuidGenerator.generateUUID().number + ".js";
 
     provide(manifest, ["background", "scripts"], [bgScript], true);
     files[bgScript] = data.background;
   }
 
-  provide(files, ["manifest.json"], JSON.stringify(manifest));
+  provide(files, ["manifest.json"], manifest);
 
   let ZipWriter = Components.Constructor("@mozilla.org/zipwriter;1", "nsIZipWriter");
   let zipW = new ZipWriter();
 
   let file = FileUtils.getFile("TmpD", ["generated-extension.xpi"]);
   file.createUnique(Ci.nsIFile.NORMAL_FILE_TYPE, FileUtils.PERMS_FILE);
 
   const MODE_WRONLY = 0x02;
@@ -805,16 +866,18 @@ this.Extension.generate = function(id, d
       }
     }
   }
 
   for (let filename in files) {
     let script = files[filename];
     if (typeof(script) == "function") {
       script = "(" + script.toString() + ")()";
+    } else if (typeof(script) == "object") {
+      script = JSON.stringify(script);
     }
 
     let stream = Cc["@mozilla.org/io/string-input-stream;1"].createInstance(Ci.nsIStringInputStream);
     stream.data = script;
 
     generateFile(filename);
     zipW.addEntryStream(filename, time, 0, stream, false);
   }
@@ -938,17 +1001,17 @@ Extension.prototype = extend(Object.crea
 
   // Reads the locale file for the given Gecko-compatible locale code, or if
   // no locale is given, the available locale closest to the UI locale.
   // Sets the currently selected locale on success.
   initLocale: Task.async(function* (locale = undefined) {
     if (locale === undefined) {
       let locales = yield this.promiseLocales();
 
-      let localeList = Object.keys(locales).map(locale => {
+      let localeList = Array.from(locales.keys(), locale => {
         return { name: locale, locales: [locale] };
       });
 
       let match = Locale.findClosestLocale(localeList);
       locale = match ? match.name : this.defaultLocale;
     }
 
     return ExtensionData.prototype.initLocale.call(this, locale);
@@ -969,17 +1032,17 @@ Extension.prototype = extend(Object.crea
       }
 
       GlobalManager.init(this);
 
       Management.emit("startup", this);
 
       return this.runManifest(this.manifest);
     }).catch(e => {
-      dump(`Extension error: ${e} ${e.filename}:${e.lineNumber}\n`);
+      dump(`Extension error: ${e} ${e.filename || e.fileName}:${e.lineNumber}\n`);
       Cu.reportError(e);
       throw e;
     });
   },
 
   cleanupGeneratedFile() {
     if (!this.cleanupFile) {
       return;
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -59,16 +59,22 @@ function runSafe(context, f, ...args)
     args = Cu.cloneInto(args, context.cloneScope);
   } catch (e) {
     Cu.reportError(e);
     dump(`runSafe failure: cloning into ${context.cloneScope}: ${e}\n\n${Error().stack}`);
   }
   return runSafeWithoutClone(f, ...args);
 }
 
+// Return true if the given value is an instance of the given
+// native type.
+function instanceOf(value, type) {
+  return {}.toString.call(value) == `[object ${type}]`;
+}
+
 // Extend the object |obj| with the property descriptors of each object in
 // |args|.
 function extend(obj, ...args) {
   for (let arg of args) {
     let props = [...Object.getOwnPropertyNames(arg),
                  ...Object.getOwnPropertySymbols(arg)];
     for (let prop of props) {
       let descriptor = Object.getOwnPropertyDescriptor(arg, prop);
@@ -629,9 +635,10 @@ this.ExtensionUtils = {
   EventManager,
   SingletonEventManager,
   ignoreEvent,
   injectAPI,
   MessageBroker,
   Messenger,
   extend,
   flushJarCache,
+  instanceOf,
 };
--- a/toolkit/components/extensions/test/mochitest/mochitest.ini
+++ b/toolkit/components/extensions/test/mochitest/mochitest.ini
@@ -33,8 +33,9 @@ support-files =
 [test_ext_background_runtime_connect_params.html]
 [test_ext_cookies.html]
 [test_ext_bookmarks.html]
 [test_ext_alarms.html]
 [test_ext_background_window_properties.html]
 [test_ext_background_sub_windows.html]
 [test_ext_jsversion.html]
 skip-if = e10s # Uses a console monitor which doesn't work from a content process. The code being tested doesn't run in a tab content process in any case.
+[test_ext_i18n.html]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/mochitest/test_ext_i18n.html
@@ -0,0 +1,121 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Test for WebExtension localization APIs</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SpawnTask.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/ExtensionTestUtils.js"></script>
+  <script type="text/javascript" src="head.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+
+<script type="application/javascript;version=1.8">
+
+add_task(function* test_i18n() {
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "default_locale": "jp",
+    },
+
+    files: {
+      "_locales/en_US/messages.json": {
+        "foo": {
+          "message": "Foo.",
+          "description": "foo",
+        },
+
+        "basic_substitutions": {
+          "message": "'$0' '$14' '$1' '$5' '$$$$$' '$$'.",
+          "description": "foo",
+        },
+
+        "Named_placeholder_substitutions": {
+          "message": "$Foo$\n$2\n$bad name$\n$bad_value$\n$bad_content_value$\n$foo",
+          "description": "foo",
+          "placeholders": {
+            "foO": {
+              "content": "_foo_ $1 _bar_",
+              "description": "foo",
+            },
+
+            "bad name": {
+              "content": "Nope.",
+              "description": "bad name",
+            },
+
+            "bad_value": "Nope.",
+
+            "bad_content_value": {
+              "content": ["Accepted, but shouldn't break."],
+              "description": "bad value",
+            },
+          },
+        },
+
+        "broken_placeholders": {
+          "message": "$broken$",
+          "description": "broken placeholders",
+          "placeholders": "foo.",
+        },
+      },
+
+      "_locales/jp/messages.json": {
+        "foo": {
+          "message": "(foo)",
+          "description": "foo",
+        },
+
+        "bar": {
+          "message": "(bar)",
+          "description": "bar",
+        },
+      },
+    },
+
+    background: "new " + function() {
+      let _ = browser.i18n.getMessage.bind(browser.i18n);
+
+      browser.test.assertEq("Foo.", _("Foo"), "Simple message in selected locale.");
+
+      browser.test.assertEq("(bar)", _("bar"), "Simple message fallback in default locale.");
+
+      let substitutions = [];
+      substitutions[4] = "5";
+      substitutions[13] = "14";
+
+      browser.test.assertEq("'$0' '14' '' '5' '$$$$' '$'.",
+                            _("basic_substitutions", substitutions),
+                            "Basic numeric substitutions");
+
+      browser.test.assertEq("'$0' '' 'just a string' '' '$$$$' '$'.",
+                            _("basic_substitutions", "just a string"),
+                            "Basic numeric substitutions, with non-array value");
+
+      let values = _("named_placeholder_substitutions", ["(subst $1 $2)", "(2 $1 $2)"]).split("\n");
+
+      browser.test.assertEq("_foo_ (subst $1 $2) _bar_", values[0], "Named and numeric substitution");
+
+      browser.test.assertEq("(2 $1 $2)", values[1], "Numeric substitution amid named placeholders");
+
+      browser.test.assertEq("$bad name$", values[2], "Named placeholder with invalid key");
+
+      browser.test.assertEq("", values[3], "Named placeholder with an invalid value");
+
+      browser.test.assertEq("Accepted, but shouldn't break.", values[4], "Named placeholder with a strange content value");
+
+      browser.test.assertEq("$foo", values[5], "Non-placeholder token that should be ignored");
+
+      browser.test.notifyPass("l10n");
+    },
+  });
+
+  yield extension.startup();
+  yield extension.awaitFinish("l10n");
+  yield extension.unload();
+});
+
+</script>
+
+</body>
+</html>