Issue 1502 - Cache translation catalogs on Safari

safari/ext/common.js

Index: safari/ext/common.js
===================================================================
--- a/safari/ext/common.js
+++ b/safari/ext/common.js
@@ -77,9 +77,6 @@
 
   /* I18n */
 
-  var localeCandidates = null;
-  var uiLocale;
-
   var getLocaleCandidates = function()
   {
     var candidates = [];
@@ -100,82 +97,82 @@
     return candidates;
   };
 
-  var getCatalog = function(locale)
+  var locales = getLocaleCandidates();
+  var catalog = {__proto__: null, "@@ui_locale": [locales[0], []]};
+
+  var replacePlaceholder = function(text, placeholder, content)
+  {
+    return text.split("$" + placeholder + "$").join(content || "");
+  };
+
+  var parseMessage = function(source)
+  {
+    var text = source.message;
+    var placeholders = [];
+
+    for (var placeholder in source.placeholders)
+    {
+      var content = source.placeholders[placeholder].content;
+
+      if (/^\$\d+$/.test(content))
+        placeholders[parseInt(content.substr(1)) - 1] = placeholder;
+      else
+        text = replacePlaceholder(text, placeholder, content);
+    }
+
+    return [text, placeholders];
+  };
+
+  var readCatalog = function(locale)
   {
     var xhr = new XMLHttpRequest();
 
     xhr.open("GET", safari.extension.baseURI + "_locales/" + locale + "/messages.json", false);
 
-    try {
+    try
+    {
       xhr.send();
     }
     catch (e)
     {
-      return null;
+      return;
     }
 
     if (xhr.status != 200 && xhr.status != 0)
-      return null;
+      return;
 
-    return JSON.parse(xhr.responseText);
+    var source = JSON.parse(xhr.responseText);

[kzar, 2014/10/28 19:12:16]

I found it confusing that this variable is called source and then part of it is
passed to the parsing function as a parameter called source. Maybe rename the
source var here or the source parameter above?

[Sebastian Noack, 2014/10/28 19:35:53]

I could argue that this is a different context and "source" is a rather generic
term, like "i" (short for "iteration"), which is used in for-loops all over the
place.

However, if you have a better idea how to call this and/or the other variable, I
can rename them.

[kzar, 2014/10/28 22:05:29]

Perhaps the source variable here could be renamed "messages" and the source
parameter above could be renamed "message"?

[Sebastian Noack, 2014/10/29 08:08:54]

Not really consistent. Since the variable storing the parsed message is already
called just "message", one might assume that "messages" would be a collection of
them.

However, I renamed this variable to "rawMessage" (since its parsed counterpart
is called "message") and the other variable to "rawCatalog" (since its parsed
counterpart is called "catalog").

[kzar, 2014/10/30 12:40:15]

Cool that makes more sense to me now.

+    for (var msgId in source)
+    {
+      if (!(msgId in catalog))
+        catalog[msgId] = parseMessage(source[msgId]);
+    }
   };
 
   ext.i18n = {
     getMessage: function(msgId, substitutions)
     {
-      if (!localeCandidates)
+      while (true)
       {
-        localeCandidates = getLocaleCandidates();
-        uiLocale = localeCandidates[0];
-      }
+        var message = catalog[msgId];
+        if (message)
+        {
+          var [text, placeholders] = message;

[kzar, 2014/10/28 19:12:16]

I've never seen this syntax in JS before, sure it's valid? (I tried `var [a,b] =
[1,2];` in the console just now and it didn't seem to work.)

[Sebastian Noack, 2014/10/28 19:35:53]

It's called destructuring assignment, and is a new feature in ES6:
http://wiki.ecmascript.org/doku.php?id=harmony:destructuring

It's not yet supported by Safari and Chrome. However, this code is processed by
jshydra, which lets us use some new JavaScript syntax like this, and converts
that line into following code:

  var text = message[0];
  var placeholders = message[1];

[kzar, 2014/10/28 22:05:29]

I guessed you were somehow right :p (Understand about destructuring from other
languages but hadn't realised it was included in ES6!)

 
-      if (msgId == "@@ui_locale")
-        return uiLocale;
+          if (!(substitutions instanceof Array))
+            substitutions = [substitutions];
 
-      for (var i = 0; i < localeCandidates.length; i++)
-      {
-        var catalog = getCatalog(localeCandidates[i]);
-        if (!catalog)
-        {
-          // if there is no catalog for this locale
-          // candidate, don't try to load it again
-          localeCandidates.splice(i--, 1);
-          continue;
+          for (var i = 0; i < placeholders.length; i++)
+            text = replacePlaceholder(text, placeholders[i], substitutions[i]);
+
+          return text;
         }
 
-        var msg = catalog[msgId];
-        if (!msg)
-          continue;
+        if (locales.length == 0)
+          return "";
 
-        var msgstr = msg.message;
-        if (!msgstr)
-          continue;
-
-        for (var placeholder in msg.placeholders)
-        {
-          var placeholderDetails = msg.placeholders[placeholder];
-          if (!placeholderDetails || !placeholderDetails.content)
-            continue;
-          if (placeholderDetails.content.indexOf("$") != 0)
-            continue;
-
-          var placeholderIdx = parseInt(placeholderDetails.content.substr(1));
-          if (isNaN(placeholderIdx) || placeholderIdx < 1)
-            continue;
-
-          var placeholderValue;
-          if (typeof substitutions != "string")
-            placeholderValue = substitutions[placeholderIdx - 1];
-          else if (placeholderIdx == 1)
-            placeholderValue = substitutions;
-
-          msgstr = msgstr.replace("$" + placeholder + "$", placeholderValue || "");
-        }
-
-        return msgstr;
+        readCatalog(locales.shift());
       }
-
-      return "";
     }
   };