Issue 1663 - Making first-run page testable in the adblockplusui repository

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
  * 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/>.
  */
 
-this.ext = function(ext)

[Thomas Greiner, 2014/12/17 11:24:44]

Why do you use `this.ext` here when we usually use `window.ext`?

[Wladimir Palant, 2014/12/17 13:00:38]

I simply don't like assuming that this code is running in window context - this
is the more universal way to access the global object.

+(function(global)
 {
   /* I18n */
 
   var getLocaleCandidates = function(selectedLocale)
   {
     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
@@ skipping 16 matching lines @@
   {
     var bidiDir = /^(ar|fa|he|ug|ur)(-|$)/.test(uiLocale) ? "rtl" : "ltr";
     var catalog = Object.create(null);
 
     catalog["@@ui_locale"] = [uiLocale.replace(/-/g, "_"), []];
     catalog["@@bidi_dir" ] = [bidiDir,  []];
 
     return catalog;
   };
 
-  var selectedLocale = navigator.language;
+  var selectedLocale = window.navigator.language;
   var match = /[?&]locale=([\w\-]+)/.exec(window.location.search);
   if (match)
     selectedLocale = match[1];
 
   var locales = getLocaleCandidates(selectedLocale);
   var catalog = initCatalog(locales[0]);
   var catalogFile = window.location.pathname.replace(/.*\//, "").replace(/\..*/, "") + ".json";
 
   var replacePlaceholder = function(text, placeholder, content)
   {
@@ skipping 14 matching lines @@
       else
         text = replacePlaceholder(text, placeholder, content);
     }
 
     return [text, placeholders];
   };
 
   var readCatalog = function(locale)
   {
     var xhr = new XMLHttpRequest();
     xhr.open("GET", "locale/" + locale + "/" + catalogFile, false);

[Thomas Greiner, 2014/12/17 11:24:44]

I suppose this is the reason why test_server.py is required to test it on
Chrome. :)

[Wladimir Palant, 2014/12/17 13:00:38]

You are correct, Chrome won't allow accessing any files unless a special command
line flag is passed.

     xhr.overrideMimeType("text/plain");
 
     try
     {
       xhr.send();
     }
     catch (e)
     {
       return;
     }
 
     if (xhr.status != 200 && xhr.status != 0)
       return;
 
     var rawCatalog = JSON.parse(xhr.responseText);
     for (var msgId in rawCatalog)
     {
       if (!(msgId in catalog))
         catalog[msgId] = parseMessage(rawCatalog[msgId]);
     }
   };
 
-  ext.i18n = {
+  if (!global.ext)
+    global.ext = {};
+
+  global.ext.i18n = {
     getMessage: function(msgId, substitutions)
     {
       while (true)
       {
         var message = catalog[msgId];
         if (message)
         {
           var text = message[0];
           var placeholders = message[1];
 
           if (!(substitutions instanceof Array))
             substitutions = [substitutions];
 
           for (var i = 0; i < placeholders.length; i++)
             text = replacePlaceholder(text, placeholders[i], substitutions[i]);
 
           return text;
         }
 
         if (locales.length == 0)
           return "";
 
         readCatalog(locales.shift());
       }
     }
   };
-  return ext;
-}(this.ext || {});

[Thomas Greiner, 2014/12/17 11:24:44]

Unless you expect this to run in a context where `window` is not the global
namespace I'd suggest that rather than passing values around all over the place
it would be simpler to have this be a function that looks like this to have all
of that code be in one place:

(function()
{
  if (!window.ext)
    window.ext = {};

  var ext = window.ext;

  ...

  ext.i18n = {
    ...
  };
})();

[Wladimir Palant, 2014/12/17 13:00:38]

It's probably cleaner to simply pass in the global object.

+})(this);