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

chrome/content/ui/ext/common.js

 /*

[Thomas Greiner, 2014/10/08 10:40:43]

I assume you implemented this before we changed the ext.* API to use pages
instead of tabs. To ensure that interfaces match up when backporting those
changes, adapt the code accordingly.

[saroyanm, 2014/10/10 12:05:58]

Adapted, but have a little doubt.

[Thomas Greiner, 2014/10/13 13:18:23]

Why is that?

  * 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 implementation of platform specific general classes and API

[Thomas Greiner, 2014/10/08 10:40:43]

What about "Implemenation of Firefox-specific general classes"?

[saroyanm, 2014/10/10 12:05:58]

Done.

+ * @fileOverview Implemenation of Firefox-specific general classes
  */
 
 (function()
 {
-  let UI = require("ui").UI;
+  var UI = require("ui").UI;
   
-  /**
-   * Tab class 
-   * @param {Object} tab an object with url parameter

[Thomas Greiner, 2014/10/08 10:40:43]

Nit: It's a property and not a parameter.

-   */
-  function Tab(tab)
-  {
-    this._url = tab.url;
-  }
-  Tab.prototype = {
-    _url: null,
-    
-    get url()
-    {
-      return this._url;
-    },
-    
-    sendMessage: function(message, responseCallback)
-    {
-      responseCallback();
-    }
-  };
-
-  /**
-   * Window class 
-   * @param {Object} win Window object
-   */
-  function Window(win)
-  {
-    this._window = win;
-  }
-  Window.prototype = {
-    _window: null,
-    
-    /**
-     * Pass Tab object of current active Window to callback function.

[Thomas Greiner, 2014/10/08 10:40:43]

The text should describe the purpose of the function and not the exact inner
workings so my suggestion would be "Get currently active tab".

You can mention that it's passed to the callback function in the parameter
description below.

-     * @param {function} callback
-     */
-    getActiveTab: function(callback)
-    {
-      let location = UI.getCurrentLocation(this._window);
-      let tab = {
-        url: location.spec
-      };
-      callback(new Tab(tab));
-    }
-  };
-  
-  let backgroundPage = {
+  var backgroundPage = {
     extractHostFromURL: function(url)
     {
       try
       {
         return Utils.unwrapURL(url).host;
       }
       catch(e)
       {
         return null;
       }
     },
     
     openOptions: function()
     {
       UI.openFiltersDialog();
     },
     
-    require: function()
-    {
-      return require;
-    }
+    require: require
   };
   
   // Randomize URI to work around bug 719376
-  let pageName = location.pathname.replace(/.*\//, '').replace(/\..*?$/, '');
-  let stringBundle = Services.strings.createBundle("chrome://adblockplus/locale/" + pageName +
+  var pageName = location.pathname.replace(/.*\//, '').replace(/\..*?$/, '');
+  var stringBundle = Services.strings.createBundle("chrome://adblockplus/locale/" + pageName +
     ".properties?" + Math.random());
   
   
   ext = {
     backgroundPage: {
       getWindow: function()
       {
         return backgroundPage;
       }
     },
     
     i18n: {
       /**
-       * get locale string

[Thomas Greiner, 2014/10/08 10:40:43]

Nit: Start with a capital letter.

+       * Get locale string
        * @param {String} key name of string
        * @param {Array} args parameters to pass
+       * @return {String} string or null if translation is missing
        */

[Thomas Greiner, 2014/10/08 10:40:43]

Please document only where it's appropriate. Apart from that it's missing the
return value.

[saroyanm, 2014/10/10 12:05:58]

Done.

       getMessage: function(key, args)
       {
         try
         {
-          return (args ? stringBundle.formatStringFromName(key, args, args.length) : stringBundle.GetStringFromName(key));
+          return stringBundle.GetStringFromName(key);
         }
         catch(e)
         {
           Cu.reportError("Missing translation: " + key);
+          return null;
         }
       }
     },
     
-    windows: {
-      /**
-       * pass focused window object to callback function
-       * @param {function} callback

[Thomas Greiner, 2014/10/08 10:40:43]

Nit: Function and description should start with a capital letter.

-       */
-      getLastFocused: function(callback)
+    pages: {
+      query: function(info, callback)
       {
-        let win = UI.currentWindow;
-        callback(new Window(UI.currentWindow));
+        var location = UI.getCurrentLocation(UI.currentWindow);
+        if (info.active && info.lastFocusedWindow && location)
+        {
+          callback([{
+            url: location.spec,
+            sendMessage: (message, responseCallback) => responseCallback()
+          }]);
+        }
+        else
+          callback([]);
       }
     }
   };
 })();