[adblockpluschrome] Issue 3596 - Added support for CSS property filters to devtools panel

include.preload.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
@@ skipping 174 matching lines @@
   try
   {
     return element.contentDocument;
   }
   catch (e)
   {
     return null;
   }
 }
 
-function ElementHidingTracer(selectors)
+function ElementHidingTracer()
 {
-  this.selectors = selectors;
+  this.selectors = [];
+  this.filters = [];
 
   this.changedNodes = [];
   this.timeout = null;
+  this.started = false;
 
   this.observer = new MutationObserver(this.observe.bind(this));
   this.trace = this.trace.bind(this);
-
-  if (document.readyState == "loading")
-    document.addEventListener("DOMContentLoaded", this.trace);
-  else
-    this.trace();
 }
 ElementHidingTracer.prototype = {
-  checkNodes: function(nodes)
+  start: function()
   {
-    var matchedSelectors = [];
+    let _start = () => {
+      this.trace();

[Sebastian Noack, 2017/02/13 15:09:17]

Please correct me if I'm wrong, but it seems this is the only place where
trace() is called. So if you just set the started property inside the trace()
function, we wouldn't need this wrapper.

[wspee, 2017/02/14 09:28:40]

Done.

+      this.started = true;
+    };
 
-    // Find all selectors that match any hidden element inside the given nodes.
-    for (var i = 0; i < this.selectors.length; i++)
+    if (document.readyState == "loading")
+      document.addEventListener("DOMContentLoaded", _start);
+    else
+      _start();
+  },
+
+ addFilters: function(selectors, filters)
+  {
+    if (this.started)
+      window.setTimeout(() => {
+        this.checkNodes([document], selectors, filters);
+      }, 0);
+
+    Array.prototype.push.apply(this.selectors, selectors);

[Sebastian Noack, 2017/02/13 15:09:17]

As mentioned before, please use the rest operator instead of .apply():

  this.selectors.push(...selectors);
  this.filters.push(...filters);

[wspee, 2017/02/14 09:28:40]

Done.

+    Array.prototype.push.apply(this.filters, filters);
+  },
+
+  checkNodes: function(nodes, selectors, filters)
+  {
+    let matchedFilters = [];
+
+    for (let i = 0; i < selectors.length; i++)
     {
-      var selector = this.selectors[i];
+      let selector = selectors[i];

[Sebastian Noack, 2017/02/13 15:09:17]

It seems "selector" and "filter" are only used once each. So these temporary
variables seem unnecessary. Instead you could inline selectors[i]/filters[i].

[wspee, 2017/02/14 09:28:40]

Done.

+      let filter = filters[i];
 
-      for (var j = 0; j < nodes.length; j++)
+      nodes: for (let node of nodes)
       {
-        var elements = nodes[j].querySelectorAll(selector);
-        var matched = false;
+        let elements = node.querySelectorAll(selector);
 
-        for (var k = 0; k < elements.length; k++)
+        for (let element of elements)
         {
           // Only consider selectors that actually have an effect on the
           // computed styles, and aren't overridden by rules with higher
           // priority, or haven't been circumvented in a different way.
-          if (getComputedStyle(elements[k]).display == "none")
+          if (getComputedStyle(element).display == "none")
           {
-            matchedSelectors.push(selector);
-            matched = true;
-            break;
+            matchedFilters.push(filter.replace(/^.*?##/, ""));
+            break nodes;
           }
         }
-
-        if (matched)
-          break;
       }
     }
 
-    if (matchedSelectors.length > 0)
+    if (matchedFilters.length > 0)
       ext.backgroundPage.sendMessage({
         type: "devtools.traceElemHide",
-        selectors: matchedSelectors
+        selectors: matchedFilters
       });
   },
 
   onTimeout: function()
   {
-    this.checkNodes(this.changedNodes);
+    this.checkNodes(this.changedNodes, this.selectors, this.filters);
     this.changedNodes = [];
     this.timeout = null;
   },
 
   observe: function(mutations)
   {
     // Forget previously changed nodes that are no longer in the DOM.
     for (var i = 0; i < this.changedNodes.length; i++)
     {
       if (!document.contains(this.changedNodes[i]))
@@ skipping 42 matching lines @@
 
     // Check only nodes whose descendants have changed, and not more often
     // than once a second. Otherwise large pages with a lot of DOM mutations
     // (like YouTube) freeze when the devtools panel is active.
     if (this.timeout == null)
       this.timeout = setTimeout(this.onTimeout.bind(this), 1000);
   },
 
   trace: function()
   {
-    this.checkNodes([document]);
+    this.checkNodes([document], this.selectors, this.filters);
 
     this.observer.observe(
       document,
       {
         childList: true,
         attributes: true,
         subtree: true
       }
     );
   },
@@ skipping 157 matching lines @@
             var shadow = shadowRoot(this);
             return shadow == ourShadowRoot ? null : shadow;
           }
         });
       }, null);
     }
 
     return shadow;
   },
 
-  addSelectors: function(selectors)
+  addSelectors: function(selectors, filters)
   {
-    if (selectors.length == 0)
+    if (!selectors.length)
       return;
 
+    if (this.tracer)
+    {
+      if (!filters)
+      {
+        filters = [];
+        Array.prototype.push.apply(filters, selectors);

[Sebastian Noack, 2017/02/13 15:09:17]

As indicated before, I'm not sure if it is a good idea to redundantly store the
selectors as filters, when we don't know the actual filters. This is a leaky
abstraction. While it works for the code above, it raises wrong assumptions. Not
to mention the redundancy in the data structure.

I'd rather create an array which holds the value of undefined for each selector
we don't know the filter of (i.e. `new Array(selectors.length)`), and handle
that case explicitly, when extracting the original selectors.

Moreover, I feel that this logic belongs into the addFilter() method.

[wspee, 2017/02/14 09:28:40]

Done.

+      }
+      this.tracer.addFilters(selectors, filters);
+    }
+
     if (!this.style)
     {
       // Create <style> element lazily, only if we add styles. Add it to
       // the shadow DOM if possible. Otherwise fallback to the <head> or
       // <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);
 
@@ skipping 29 matching lines @@
     {
       var selector = selectors.slice(i, i + this.selectorGroupSize).join(", ");
       this.style.sheet.insertRule(selector + "{display: none !important;}",
                                   this.style.sheet.cssRules.length);
     }
   },
 
   apply: function()
   {
     var selectors = null;
-    var elemHideEmulationLoaded = false;
 
     var checkLoaded = function()
     {
-      if (!selectors || !elemHideEmulationLoaded)
-        return;
-
       if (this.tracer)
         this.tracer.disconnect();
       this.tracer = null;
 
+      if (selectors.trace)
+        this.tracer = new ElementHidingTracer();
+
       if (this.style && this.style.parentElement)
         this.style.parentElement.removeChild(this.style);
       this.style = null;
 
-      this.addSelectors(selectors.selectors);
+      if (selectors.selectors)
+        this.addSelectors(selectors.selectors);
+
       this.elemHideEmulation.apply();
 
-      if (selectors.trace)
-        this.tracer = new ElementHidingTracer(selectors.selectors);
+      if (this.tracer)
+        this.tracer.start();
     }.bind(this);
 
-    ext.backgroundPage.sendMessage({type: "get-selectors"}, function(response)
+    ext.backgroundPage.sendMessage({type: "get-selectors"}, response =>
     {
       selectors = response;
-      checkLoaded();
-    });
-
-    this.elemHideEmulation.load(function()
-    {
-      elemHideEmulationLoaded = true;
-      checkLoaded();
+      this.elemHideEmulation.load(checkLoaded);
     });
   }
 };
 
 if (document instanceof HTMLDocument)
 {
   checkSitekey();
   wrapWebSocket();
 
   var elemhide = new ElemHide();
   elemhide.apply();
 
   document.addEventListener("error", function(event)
   {
     checkCollapse(event.target);
   }, true);
 
   document.addEventListener("load", function(event)
   {
     var element = event.target;
     if (/^i?frame$/.test(element.localName))
       checkCollapse(element);
   }, true);
 }