Issue 4871 - Start using ESLint for adblockplusui

common.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 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/>.
  */
 
+/* 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);
+};
+
+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)
   {
-    return document.getElementById(id);
+    glassPane = document.createElement("div");
+    glassPane.setAttribute("id", "glass-pane");
+    document.body.appendChild(glassPane);
   }
 
-  global.getDocLink = function(link, callback)
+  let iframe = E("share-popup");
+  if (!iframe)
   {
-    ext.backgroundPage.sendMessage({
-      type: "app.get",
-      what: "doclink",
-      link: link
-    }, callback);
+    iframe = document.createElement("iframe");
+    iframe.setAttribute("id", "share-popup");
+    iframe.setAttribute("scrolling", "no");
+    glassPane.appendChild(iframe);
   }
 
-  global.checkShareResource = function(url, callback)
+  // 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)
   {
-    ext.backgroundPage.sendMessage(
+    try
     {
-      type: "filters.blocked",
-      url: url,
-      requestType: "SCRIPT",
-      docDomain: "adblockplus.org",
-      thirdParty: true
-    }, callback);
+      let Ci = Components.interfaces;
+      let 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
+        );
+      }
+    }
+    catch (ex)
+    {
+      console.error(ex);
+    }
   }
 
-  global.openSharePopup = function(url)
+  let popupMessageReceived = false;
+  function resizePopup(width, height)
   {
-    var glassPane = E("glass-pane");
-    if (!glassPane)
+    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;
+
+    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)
     {
-      glassPane = document.createElement("div");
-      glassPane.setAttribute("id", "glass-pane");
-      document.body.appendChild(glassPane);
+      let rootElement = iframe.contentDocument.documentElement;
+      let {width, height} = rootElement.dataset;
+      if (width && height)
+        resizePopup(width, height);
     }
 
-    var iframe = E("share-popup");
-    if (!iframe)
+    if (popupMessageReceived)
     {
-      iframe = document.createElement("iframe");
-      iframe.setAttribute("id", "share-popup");
-      iframe.setAttribute("scrolling", "no");
-      glassPane.appendChild(iframe);
+      iframe.className = "visible";
+
+      let popupCloseListener = function()
+      {
+        iframe.className = glassPane.className = "";
+        document.removeEventListener("click", popupCloseListener);
+      };
+      document.addEventListener("click", popupCloseListener, false);
     }
-
-    // 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)
+    else
     {
-      try
-      {
-        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);
-        }
-      }
-      catch(ex)
-      {
-        console.error(ex);
-      }
-    }
-
-    var 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;
+      glassPane.className = "";
       window.removeEventListener("message", popupMessageListener);
     }
 
-    var popupMessageListener = function(event)
-    {
-      if (!/[.\/]adblockplus\.org$/.test(event.origin)
-        || !("width" in event.data)
-        || !("height" in event.data))
-        return;
+    iframe.removeEventListener("load", popupLoadListener);
+  };
+  iframe.addEventListener("load", popupLoadListener, false);
 
-      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()
-    {
-      if (!popupMessageReceived && isGecko)
-      {
-        var rootElement = iframe.contentDocument.documentElement;
-        var width = rootElement.dataset.width;
-        var height = rootElement.dataset.height;
-        if (width && height)
-          resizePopup(width, height);
-      }
-
-      if (popupMessageReceived)
-      {
-        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.removeEventListener("load", popupLoadListener);
-    };
-    iframe.addEventListener("load", popupLoadListener, false);
-
-    iframe.src = url;
-    glassPane.className = "visible";
-  }
-})(this);
+  iframe.src = url;
+  glassPane.className = "visible";
+};