Issue 1502 - Cache translation catalogs on Safari

safari/ext/common.js

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2014 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 59 matching lines @@
 
       this._messageDispatcher.dispatchMessage("request", request);
     }
   };
 
   ext.onMessage = new ext._EventTarget();
 
 
   /* I18n */
 
-  var localeCandidates = null;
-  var uiLocale;
-
   var getLocaleCandidates = function()
   {
     var candidates = [];
     var defaultLocale = "en_US";
 
     // e.g. "ja-jp-mac" -> "ja", "jp", note that the part after the second
     // dash is dropped, since we only support language and region
     var [language, region] = navigator.language.split("-");
 
     if (region)
       candidates.push(language + "_" + region.toUpperCase());
 
     candidates.push(language);
 
     if (candidates.indexOf(defaultLocale) == -1)
       candidates.push(defaultLocale);
 
     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 || "");

[Wladimir Palant, 2014/10/30 23:00:44]

Why not keep the more obvious text.replace(...)? And - no, I don't care which
one is a split millisecond faster.

[Sebastian Noack, 2014/10/30 23:53:01]

Because .replace() with a string as first argument only replaces the first
occurrence of that string. I could have used .replace() with a regex using the g
flag as first argument. However, since the string is variable it would require
escaping. So using .split().join() is simplest way to replace all occurrences of
a variable string.

The old code was just broken. So yes, this change is unrelated. However, I had
to restructure that code anyway, and didn't mind fixing it while doing so.

+  };
+
+  var parseMessage = function(rawMessage)
+  {
+    var text = rawMessage.message;
+    var placeholders = [];
+
+    for (var placeholder in rawMessage.placeholders)

[Wladimir Palant, 2014/10/30 23:00:44]

I didn't expect it but apparently one can iterate over null :/

[Sebastian Noack, 2014/10/30 23:53:01]

You can. Iterating over null (or undefined) has the same result as iterating
over an empty object. While talking about unrelated changes, note that the old
code also didn't mind checking for null here. ;)

+    {
+      var content = rawMessage.placeholders[placeholder].content;
+
+      if (/^\$\d+$/.test(content))
+        placeholders[parseInt(content.substr(1)) - 1] = placeholder;

[Wladimir Palant, 2014/10/30 23:00:44]

Doing both regular expressions and string manipulation is unnecessary here and
obfuscates the meaning. How about:

 var match = /^\$(\d+)$/.exec(content);
 if (match)
    placeholders[parseInt(match[1], 10)] = placeholder;

Note that the second parameter to parseInt() is important. I'm not sure whether
all our supported Safari versions will have parseInt("017") == 17.

[Sebastian Noack, 2014/10/30 23:53:01]

Neither the old code, nor most other calls to parseInt() in this repository,
specify a radix. I'm going to file a separate issue for that.

+      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 rawCatalog = JSON.parse(xhr.responseText);
+    for (var msgId in rawCatalog)
+    {
+      if (!(msgId in catalog))
+        catalog[msgId] = parseMessage(rawCatalog[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;
 
-      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 "";
     }
   };
 
 
   /* Utils */
 
   ext.getURL = function(path)
   {
     return safari.extension.baseURI + path;
   };
 })();