Issue 4864 - Start using ESLint for adblockpluschrome

include.preload.js

Index: include.preload.js
diff --git a/include.preload.js b/include.preload.js
index 1c28cb6bc9ac88a2703015ab49d5978a0ee3c6e9..5709f666b50f3303e93e1f2826bd3cc21986150e 100644
--- a/include.preload.js
+++ b/include.preload.js
@@ -15,19 +15,24 @@
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+/* globals ElemHideEmulation, splitSelector */
+
 "use strict";
 
-const typeMap = {
-  "img": "IMAGE",
-  "input": "IMAGE",
-  "picture": "IMAGE",
-  "audio": "MEDIA",
-  "video": "MEDIA",
-  "frame": "SUBDOCUMENT",
-  "iframe": "SUBDOCUMENT",
-  "object": "OBJECT",
-  "embed": "OBJECT"
-};
+// This variable is also used by our other content scripts.

[Sebastian Noack, 2017/02/09 01:04:50]

This comment is kinda redundant with the /* globals ... */ comment in
composer.postload.js. But as I said there, these global variables aren't great
in particular in regard to the additional boilerplate. This should rather be a
module export.

[wspee, 2017/03/02 10:27:13]

You might want to also comment on this? But as far as I got it from the other
similar comments this is going to stay this way?

[kzar, 2017/03/07 11:41:35]

Yes, I think for now this stuff will stay as it is. We need to get the ESLint
changes to these repositories pushed soon, since it's blocking other people from
collaborating or at least making it much harder.

I do agree with Sebastian that we should reduce the use of globals via modules,
and I'm going to keep #4864 open until we've done that. When Sebastian's back
I'll work with him on that.

+let elemhide;
+
+const typeMap = new Map([
+  ["img", "IMAGE"],
+  ["input", "IMAGE"],
+  ["picture", "IMAGE"],
+  ["audio", "MEDIA"],
+  ["video", "MEDIA"],
+  ["frame", "SUBDOCUMENT"],
+  ["iframe", "SUBDOCUMENT"],
+  ["object", "OBJECT"],
+  ["embed", "OBJECT"]
+]);
 
 function getURLsFromObjectElement(element)
 {
@@ -41,9 +46,9 @@ function getURLsFromObjectElement(element)
       continue;
 
     let name = child.getAttribute("name");
-    if (name != "movie"  && // Adobe Flash
+    if (name != "movie" &&  // Adobe Flash
         name != "source" && // Silverlight
-        name != "src"    && // Real Media + Quicktime
+        name != "src" &&    // Real Media + Quicktime
         name != "FileName") // Windows Media
       continue;
 
@@ -84,7 +89,7 @@ function getURLsFromMediaElement(element)
   for (let child of element.children)
   {
     if (child.localName == "source" || child.localName == "track")
-      urls.push.apply(urls, getURLsFromAttributes(child));
+      urls.push(...getURLsFromAttributes(child));
   }
 
   if (element.poster)
@@ -124,7 +129,7 @@ function getURLsFromElement(element)
 
 function checkCollapse(element)
 {
-  let mediatype = typeMap[element.localName];
+  let mediatype = typeMap.get(element.localName);
   if (!mediatype)
     return;
 
@@ -135,8 +140,8 @@ function checkCollapse(element)
   ext.backgroundPage.sendMessage(
     {
       type: "filters.collapse",
-      urls: urls,
-      mediatype: mediatype,
+      urls,
+      mediatype,
       baseURL: document.location.href
     },
 
@@ -238,10 +243,12 @@ ElementHidingTracer.prototype = {
     }
 
     if (matchedSelectors.length > 0)
+    {
       ext.backgroundPage.sendMessage({
         type: "devtools.traceElemHide",
         selectors: matchedSelectors
       });
+    }
   },
 
   onTimeout()
@@ -345,18 +352,18 @@ function runInPageContext(fn, arg)
 // [1] - https://bugs.chromium.org/p/chromium/issues/detail?id=129353
 function wrapWebSocket()
 {
-  let eventName = "abpws-" + Math.random().toString(36).substr(2);
+  let randomEventName = "abpws-" + Math.random().toString(36).substr(2);
 
-  document.addEventListener(eventName, event =>
+  document.addEventListener(randomEventName, event =>
   {
     ext.backgroundPage.sendMessage({
       type: "request.websocket",
       url: event.detail.url
     }, block =>
     {
-      document.dispatchEvent(
-        new CustomEvent(eventName + "-" + event.detail.url, {detail: block})
-      );
+      document.dispatchEvent(new CustomEvent(
+        randomEventName + "-" + event.detail.url, {detail: block}

[Sebastian Noack, 2017/02/09 01:04:50]

If you rename the argument to "detail", you can use the shorthand syntax here. 

[kzar, 2017/02/20 10:27:31]

I think block is a better name for the variable though, since it's a boolean
indicating if the request should be blocked or not.

+      ));
     });
   });
 
@@ -365,11 +372,13 @@ function wrapWebSocket()
     // As far as possible we must track everything we use that could be
     // sabotaged by the website later in order to circumvent us.
     let RealWebSocket = WebSocket;
-    let closeWebSocket = Function.prototype.call.bind(RealWebSocket.prototype.close);
+    let RealCustomEvent = window.CustomEvent;
+    let closeWebSocket = Function.prototype.call.bind(
+      RealWebSocket.prototype.close
+    );
     let addEventListener = document.addEventListener.bind(document);
     let removeEventListener = document.removeEventListener.bind(document);
     let dispatchEvent = document.dispatchEvent.bind(document);
-    let CustomEvent = window.CustomEvent;
 
     function checkRequest(url, callback)
     {
@@ -381,12 +390,10 @@ function wrapWebSocket()
       }
       addEventListener(incomingEventName, listener);
 
-      dispatchEvent(new CustomEvent(eventName, {
-        detail: {url: url}
-      }));
+      dispatchEvent(new RealCustomEvent(eventName, {detail: {url}}));
     }
 
-    function WrappedWebSocket(url)
+    function WrappedWebSocket(url, ...args)
     {
       // Throw correct exceptions if the constructor is used improperly.
       if (!(this instanceof WrappedWebSocket)) return RealWebSocket();
@@ -396,7 +403,7 @@ function wrapWebSocket()
       if (arguments.length == 1)
         websocket = new RealWebSocket(url);
       else
-        websocket = new RealWebSocket(url, arguments[1]);
+        websocket = new RealWebSocket(url, args[0]);
 
       checkRequest(websocket.url, blocked =>
       {
@@ -407,7 +414,7 @@ function wrapWebSocket()
       return websocket;
     }
     WrappedWebSocket.prototype = RealWebSocket.prototype;
-    WebSocket = WrappedWebSocket.bind();
+    window.WebSocket = WrappedWebSocket.bind();
     Object.defineProperties(WebSocket, {
       CONNECTING: {value: RealWebSocket.CONNECTING, enumerable: true},
       OPEN: {value: RealWebSocket.OPEN, enumerable: true},
@@ -417,7 +424,7 @@ function wrapWebSocket()
     });
 
     RealWebSocket.prototype.constructor = WebSocket;
-  }, eventName);
+  }, randomEventName);
 }
 
 function ElemHide()
@@ -470,14 +477,15 @@ ElemHide.prototype = {
         let ourShadowRoot = document.documentElement.shadowRoot;
         if (!ourShadowRoot)
           return;
-        let desc = Object.getOwnPropertyDescriptor(Element.prototype, "shadowRoot");
+        let desc = Object.getOwnPropertyDescriptor(Element.prototype,
+                                                   "shadowRoot");
         let shadowRoot = Function.prototype.call.bind(desc.get);
 
         Object.defineProperty(Element.prototype, "shadowRoot", {
           configurable: true, enumerable: true, get()
           {
-            let shadow = shadowRoot(this);
-            return shadow == ourShadowRoot ? null : shadow;
+            let thisShadow = shadowRoot(this);
+            return thisShadow == ourShadowRoot ? null : shadow;
           }
         });
       }, null);
@@ -498,8 +506,8 @@ ElemHide.prototype = {
       // <html> element. If we have injected a style element before that
       // has been removed (the sheet property is null), create a new one.
       this.style = document.createElement("style");
-      (this.shadow || document.head
-                   || document.documentElement).appendChild(this.style);
+      (this.shadow || document.head ||
+       document.documentElement).appendChild(this.style);

[Sebastian Noack, 2017/02/09 01:04:50]

Is the fallback to document.documentElement even still necessary?

[kzar, 2017/02/20 10:27:31]

I have no idea.

 
       // It can happen that the frame already navigated to a different
       // document while we were waiting for the background page to respond.
@@ -581,9 +589,7 @@ if (document instanceof HTMLDocument)
   checkSitekey();
   wrapWebSocket();
 
-  // This variable is also used by our other content scripts, outside of the
-  // current scope.
-  var elemhide = new ElemHide();
+  elemhide = new ElemHide();
   elemhide.apply();
 
   document.addEventListener("error", event =>