issue 1435 - Port popup.html from Chrome/Safari/Opera to Firefox

chrome/content/ui/ext/common.js

Index: chrome/content/ui/ext/common.js
===================================================================
new file mode 100644
--- /dev/null
+++ b/chrome/content/ui/ext/common.js
@@ -0,0 +1,124 @@
+/*
+ * 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/>.
+ */
+
+/**
+ * @fileOverview Implemenation of Firefox-specific general classes
+ */
+
+(function()
+{
+  var UI = require("ui").UI;
+  
+  /**
+   * Page class
+   * @param {Object} tab an object with url property
+   */
+  function Page(tab)
+  {
+    this._url = tab.url;
+  }
+
+  Page.prototype = {
+    _url: null,
+
+    get url()
+    {
+      return this._url;
+    },
+    
+    sendMessage: function(message, responseCallback)
+    {
+      responseCallback();
+    }
+  };
+
+  var backgroundPage = {
+    extractHostFromURL: function(url)
+    {
+      try
+      {
+        return Utils.unwrapURL(url).host;
+      }
+      catch(e)
+      {
+        return null;
+      }
+    },
+    
+    openOptions: function()
+    {
+      UI.openFiltersDialog();
+    },
+    
+    require: function()
+    {
+      return require;
+    }

[Wladimir Palant, 2014/10/23 21:57:40]

Why is this a closure returning the require() function? Shouldn't it be:

   require: require

?

[saroyanm, 2014/10/27 21:49:20]

Done.

+  };
+  
+  // Randomize URI to work around bug 719376
+  var pageName = location.pathname.replace(/.*\//, '').replace(/\..*?$/, '');
+  var stringBundle = Services.strings.createBundle("chrome://adblockplus/locale/" + pageName +
+    ".properties?" + Math.random());
+  
+  
+  ext = {

[Wladimir Palant, 2014/10/23 21:57:40]

This is an undeclared variable. Please do it like this:

var ext = function()
{
  ...
  return {
    ...
  };
};

[saroyanm, 2014/10/27 21:49:20]

I've declared ext in Utils and left a comment there.

+    backgroundPage: {
+      getWindow: function()
+      {
+        return backgroundPage;
+      }
+    },
+    
+    i18n: {
+      /**
+       * Get locale string
+       * @param {String} key name of string
+       * @param {Array} args parameters to pass
+       * @return {String} string or null if translation is missing
+       */
+      getMessage: function(key, args)
+      {
+        try
+        {
+          return (args ? stringBundle.formatStringFromName(key, args, args.length) : stringBundle.GetStringFromName(key));

[Wladimir Palant, 2014/10/23 21:57:40]

Please don't use stringBundle.formatStringFromName(), we don't use that
placeholder format - it doesn't allow changing order of placeholders in
translated strings. If anything, we should use ?1? and ?2? for placeholders
(that's what we normally use in Firefox). We will need Chrome/Opera/Safari
changes however if any shared strings use these placeholders.

[saroyanm, 2014/10/27 21:49:20]

Done, please let me know if I'm missing something.

+        }
+        catch(e)
+        {
+          Cu.reportError("Missing translation: " + key);
+          return null;
+        }
+      }
+    },
+    
+    pages: {
+      query: function(info, callback)
+      {
+        var location = UI.getCurrentLocation(UI.currentWindow);
+        if (info.active && info.lastFocusedWindow && location)
+        {
+          var tab = {
+            url: location.spec
+          };
+          callback([new Page(tab)]);

[Wladimir Palant, 2014/10/23 21:57:40]

Is the Page class used anywhere else? If not, then the following should be
sufficient:

callback([{
  url: location.spec,
  sendMessage: (message, responseCallback) => responseCallback()
}]);

No need for a tab object and a Page class.

[saroyanm, 2014/10/27 21:49:20]

Done.

+        }
+        else
+          callback([]);
+      }
+    }
+  };
+})();