Issue 4871 - Start using ESLint for adblockplusui

common.js

Index: common.js
diff --git a/common.js b/common.js
index 8168515dcf6071d559f96ce9d0371fb34199ce8c..720b83b50206e66fecc25bd26266091399a6bdf5 100644
--- a/common.js
+++ b/common.js
@@ -15,142 +15,140 @@
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+/* globals Components, E */

[Thomas Greiner, 2017/03/01 17:39:34]

Detail: You're also defining `E` as a local variable via `function E(id)` so do
you even need to specify it here?

[kzar, 2017/03/07 12:48:31]

Well as it is with `window.E = function E(id)` then yes it's somehow required.
But I wonder why we don't simply do `function E(id)` instead, which removes the
need to specify it in the globals comment too. Can you think of any reason to
also assign it to window.E? (Same for the other functions below too.)

[Thomas Greiner, 2017/03/07 13:33:00]

Since we no longer have it inside a closure we no longer need to explicitly
assign it to `window`. Therefore I agree with that change.

+
 "use strict";
 
-(function(global)
+window.E = function E(id)
 {
-  global.E = function E(id)
-  {
-    return document.getElementById(id);
-  }
+  return document.getElementById(id);
+};
 
-  global.getDocLink = function(link, callback)
+window.getDocLink = function(link, callback)
+{
+  ext.backgroundPage.sendMessage({
+    type: "app.get",
+    what: "doclink",
+    link
+  }, callback);
+};
+
+window.checkShareResource = function(url, callback)
+{
+  ext.backgroundPage.sendMessage({
+    type: "filters.blocked",
+    url,
+    requestType: "SCRIPT",
+    docDomain: "adblockplus.org",
+    thirdParty: true
+  }, callback);
+};
+
+window.openSharePopup = function(url)

[Thomas Greiner, 2017/03/01 17:39:34]

Interesting that we're not using arrow functions for such assignments. Is this
expected?

[kzar, 2017/03/07 12:48:31]

Well I've changed this to a regular named function now, see above.

+{
+  let glassPane = E("glass-pane");
+  if (!glassPane)
   {
-    ext.backgroundPage.sendMessage({
-      type: "app.get",
-      what: "doclink",
-      link: link
-    }, callback);
+    glassPane = document.createElement("div");
+    glassPane.setAttribute("id", "glass-pane");
+    document.body.appendChild(glassPane);
   }
 
-  global.checkShareResource = function(url, callback)
+  let iframe = E("share-popup");
+  if (!iframe)
   {
-    ext.backgroundPage.sendMessage(
-    {
-      type: "filters.blocked",
-      url: url,
-      requestType: "SCRIPT",
-      docDomain: "adblockplus.org",
-      thirdParty: true
-    }, callback);
+    iframe = document.createElement("iframe");
+    iframe.setAttribute("id", "share-popup");
+    iframe.setAttribute("scrolling", "no");
+    glassPane.appendChild(iframe);
   }
 
-  global.openSharePopup = function(url)
+  // Firefox 38+ no longer allows messaging using postMessage so we need
+  // to have a fake top level frame to avoid problems with scripts that try to
+  // communicate with the first-run page
+  let isGecko = ("Components" in window);
+  if (isGecko)
   {
-    var glassPane = E("glass-pane");
-    if (!glassPane)
-    {
-      glassPane = document.createElement("div");
-      glassPane.setAttribute("id", "glass-pane");
-      document.body.appendChild(glassPane);
-    }
-
-    var iframe = E("share-popup");
-    if (!iframe)
+    try
     {
-      iframe = document.createElement("iframe");
-      iframe.setAttribute("id", "share-popup");
-      iframe.setAttribute("scrolling", "no");
-      glassPane.appendChild(iframe);
-    }
+      let Ci = Components.interfaces;
+      let docShell = iframe.contentWindow
+        .QueryInterface(Ci.nsIInterfaceRequestor)
+        .getInterface(Ci.nsIDocShell);
 
-    // Firefox 38+ no longer allows messaging using postMessage so we need
-    // to have a fake top level frame to avoid problems with scripts that try to
-    // communicate with the first-run page
-    var isGecko = ("Components" in window);
-    if (isGecko)
-    {
-      try
+      if (typeof docShell.frameType != "undefined")
       {
-        var Ci = Components.interfaces;
-        var docShell = iframe.contentWindow
-          .QueryInterface(Ci.nsIInterfaceRequestor)
-          .getInterface(Ci.nsIDocShell);
-
-        if (typeof docShell.frameType != "undefined")
-        {
-          // Gecko 47+
-          docShell.frameType = docShell.FRAME_TYPE_BROWSER;
-        }
-        else
-        {
-          // Legacy branch
-          docShell.setIsBrowserInsideApp(Ci.nsIScriptSecurityManager.UNKNOWN_APP_ID);
-        }
+        // Gecko 47+
+        docShell.frameType = docShell.FRAME_TYPE_BROWSER;
       }
-      catch(ex)
+      else
       {
-        console.error(ex);
+        // Legacy branch
+        docShell.setIsBrowserInsideApp(
+          Ci.nsIScriptSecurityManager.UNKNOWN_APP_ID
+        );
       }
     }
-
-    var popupMessageReceived = false;
-    function resizePopup(width, height)
+    catch (ex)
     {
-      iframe.width = width;
-      iframe.height = height;
-      iframe.style.marginTop = -height / 2 + "px";
-      iframe.style.marginLeft = -width / 2 + "px";
-      popupMessageReceived = true;
-      window.removeEventListener("message", popupMessageListener);
+      console.error(ex);
     }
+  }
+
+  let popupMessageReceived = false;
+  function resizePopup(width, height)
+  {
+    iframe.width = width;
+    iframe.height = height;
+    iframe.style.marginTop = -height / 2 + "px";
+    iframe.style.marginLeft = -width / 2 + "px";
+    popupMessageReceived = true;
+    window.removeEventListener("message", popupMessageListener);
+  }
+
+  let popupMessageListener = function(event)
+  {
+    if (!/[./]adblockplus\.org$/.test(event.origin) ||
+        !("width" in event.data) || !("height" in event.data))
+      return;
 
-    var popupMessageListener = function(event)
+    resizePopup(event.data.width, event.data.height);
+  };
+  // Firefox requires last parameter to be true to be triggered by
+  // unprivileged pages
+  window.addEventListener("message", popupMessageListener, false, true);
+
+  let popupLoadListener = function()
+  {
+    if (!popupMessageReceived && isGecko)
     {
-      if (!/[.\/]adblockplus\.org$/.test(event.origin)
-        || !("width" in event.data)
-        || !("height" in event.data))
-        return;
-
-      resizePopup(event.data.width, event.data.height);
-    };
-    // Firefox requires last parameter to be true to be triggered by
-    // unprivileged pages
-    window.addEventListener("message", popupMessageListener, false, true);
-
-    var popupLoadListener = function()
+      let rootElement = iframe.contentDocument.documentElement;
+      let {width, height} = rootElement.dataset;
+      if (width && height)
+        resizePopup(width, height);
+    }
+
+    if (popupMessageReceived)
     {
-      if (!popupMessageReceived && isGecko)
-      {
-        var rootElement = iframe.contentDocument.documentElement;
-        var width = rootElement.dataset.width;
-        var height = rootElement.dataset.height;
-        if (width && height)
-          resizePopup(width, height);
-      }
+      iframe.className = "visible";
 
-      if (popupMessageReceived)
+      let popupCloseListener = function()
       {
-        iframe.className = "visible";
-
-        var popupCloseListener = function()
-        {
-          iframe.className = glassPane.className = "";
-          document.removeEventListener("click", popupCloseListener);
-        };
-        document.addEventListener("click", popupCloseListener, false);
-      }
-      else
-      {
-        glassPane.className = "";
-        window.removeEventListener("message", popupMessageListener);
-      }
+        iframe.className = glassPane.className = "";
+        document.removeEventListener("click", popupCloseListener);
+      };
+      document.addEventListener("click", popupCloseListener, false);
+    }
+    else
+    {
+      glassPane.className = "";
+      window.removeEventListener("message", popupMessageListener);
+    }
 
-      iframe.removeEventListener("load", popupLoadListener);
-    };
-    iframe.addEventListener("load", popupLoadListener, false);
+    iframe.removeEventListener("load", popupLoadListener);
+  };
+  iframe.addEventListener("load", popupLoadListener, false);
 
-    iframe.src = url;
-    glassPane.className = "visible";
-  }
-})(this);
+  iframe.src = url;
+  glassPane.className = "visible";
+};