First-run page (revisited)

chrome/content/ui/i18n.js

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2013 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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 var i18n;
 if (typeof chrome != "undefined")
 {
   i18n = chrome.i18n;
 }
 else
 {

[Wladimir Palant, 2013/05/27 11:02:00]

Please add a comment noting that this is the Firefox branch.

[Thomas Greiner, 2013/05/27 13:03:16]

Done.

+  // Using Firefox' approach on i18n instead
+  
   // Randomize URI to work around bug 719376
-  var stringBundle = Services.strings.createBundle("chrome://" + require("info").addonName + "/locale/firstRun.properties?" + Math.random());

[Wladimir Palant, 2013/05/27 11:02:00]

A relative URL should work here: "/locale/firstRun.properties?" + Math.random().
However, we shouldn't hardcode the page name, the same localization approach
will likely be used for other pages as well in future. So maybe:

var pageName = location.pathname.replace(/.*\//, '').replace(/\..*?$/, '');
var stringBundle = Services.strings.createBundle("/locale/" + pageName +
".properties?" + Math.random());

[Thomas Greiner, 2013/05/27 13:03:16]

Done.

+  var pageName = location.pathname.replace(/.*\//, '').replace(/\..*?$/, '');
+  var stringBundle = Services.strings.createBundle("chrome://adblockplus/locale/" + pageName +
+    ".properties?" + Math.random());
+  
   function getI18nMessage(key)
   {
     return {
       "message": stringBundle.GetStringFromName(key)
     };
   }
   
   i18n = (function()
   {
     function getText(message, args)
@@ skipping 19 matching lines @@
           continue;
 
         text = text.split("$" + key + "$").join(replacement);
       }
       return text;
     }
 
     return {
       getMessage: function(key, args)
       {
-        var message = getI18nMessage(key);
-        if (!message)
+        try{
+          var message = getI18nMessage(key);
+          return getText(message, args);
+        }
+        catch(e)
+        {
+          Cu.reportError(e);
           return "Missing translation: " + key;

[Wladimir Palant, 2013/05/27 11:02:00]

This won't work - GetStringForName throws on missing translations. So you want
to use a try..catch block instead and return this string on exceptions.

[Thomas Greiner, 2013/05/27 13:03:16]

Done.

-        return getText(message, args);
+        }
       }
     };
   })();
 }
 
 // Loads and inserts i18n strings into matching elements. Any inner HTML already in the
 // element is parsed as JSON and used as parameters to substitute into placeholders in the
 // i18n message.
 function loadI18nStrings()
 {
@@ skipping 20 matching lines @@
   var timeString = d.toLocaleTimeString();
 
   var now = new Date();
   if (d.toDateString() == now.toDateString())
     return [timeString];
   else
     return [timeString, d.toLocaleDateString()];
 }
 
 // Fill in the strings as soon as possible
-// Script could be injected after DOMContentLoaded fired

[Wladimir Palant, 2013/05/27 11:02:00]

Nope, it could not - if firstRun.js is coded cleanly ;)
Please just use DOMContentLoaded.

[Thomas Greiner, 2013/05/27 13:03:16]

Done.

-if (document.readyState != "loading")
-  loadI18nStrings();
-else
-  window.addEventListener("DOMContentLoaded", loadI18nStrings, true);
+window.addEventListener("DOMContentLoaded", loadI18nStrings, true);