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

include.preload.js

Index: include.preload.js
diff --git a/include.preload.js b/include.preload.js
index 1c28cb6bc9ac88a2703015ab49d5978a0ee3c6e9..e2b95d548afbdec03a0d693dca991dac33179556 100644
--- a/include.preload.js
+++ b/include.preload.js
@@ -191,9 +191,10 @@ function getContentDocument(element)
   }
 }
 
-function ElementHidingTracer(selectors)
+function ElementHidingTracer()
 {
-  this.selectors = selectors;
+  this.selectors = [];
+  this.filters = [];
 
   this.changedNodes = [];
   this.timeout = null;
@@ -207,17 +208,24 @@ function ElementHidingTracer(selectors)
     this.trace();
 }
 ElementHidingTracer.prototype = {
-  checkNodes(nodes)
+  addSelectors(selectors, filters)
+  {
+    if (document.readyState != "loading")
+      this.checkNodes([document], selectors, filters);
+
+    this.selectors.push(...selectors);
+    this.filters.push(...filters);
+  },
+
+  checkNodes(nodes, selectors, filters)
   {
     let matchedSelectors = [];
 
-    // Find all selectors that match any hidden element inside the given nodes.
-    for (let selector of this.selectors)
+    for (let i = 0; i < selectors.length; i++)
     {
-      for (let node of nodes)
+      nodes: for (let node of nodes)
       {
-        let elements = node.querySelectorAll(selector);
-        let matched = false;
+        let elements = node.querySelectorAll(selectors[i]);
 
         for (let element of elements)
         {
@@ -226,14 +234,11 @@ ElementHidingTracer.prototype = {
           // priority, or haven't been circumvented in a different way.
           if (getComputedStyle(element).display == "none")
           {
-            matchedSelectors.push(selector);
-            matched = true;
-            break;
+            let filter = filters[i] || selectors[i];

[Sebastian Noack, 2017/02/23 11:08:23]

This is no longer necessary. You don't have to consider selectors here anymore.
Since there will always be an entry in filters.

[wspee, 2017/02/23 11:20:21]

Done.

+            matchedSelectors.push(filter.replace(/^.*?##/, ""));
+            break nodes;
           }
         }
-
-        if (matched)
-          break;
       }
     }
 
@@ -246,7 +251,7 @@ ElementHidingTracer.prototype = {
 
   onTimeout()
   {
-    this.checkNodes(this.changedNodes);
+    this.checkNodes(this.changedNodes, this.selectors, this.filters);
     this.changedNodes = [];
     this.timeout = null;
   },
@@ -308,7 +313,7 @@ ElementHidingTracer.prototype = {
 
   trace()
   {
-    this.checkNodes([document]);
+    this.checkNodes([document], this.selectors, this.filters);
 
     this.observer.observe(
       document,
@@ -486,7 +491,7 @@ ElemHide.prototype = {
     return shadow;
   },
 
-  addSelectors(selectors)
+  addSelectors(selectors, filters)
   {
     if (selectors.length == 0)
       return;
@@ -512,16 +517,19 @@ ElemHide.prototype = {
     // If using shadow DOM, we have to add the ::content pseudo-element
     // before each selector, in order to match elements within the
     // insertion point.
+    let preparedSelectors = [];
     if (this.shadow)
     {
-      let preparedSelectors = [];
       for (let selector of selectors)
       {
         let subSelectors = splitSelector(selector);
         for (let subSelector of subSelectors)
           preparedSelectors.push("::content " + subSelector);
       }
-      selectors = preparedSelectors;
+    }
+    else
+    {
+      preparedSelectors = selectors;
     }
 
     // Safari only allows 8192 primitive selectors to be injected at once[1], we
@@ -529,24 +537,21 @@ ElemHide.prototype = {
     // (Chrome also has a limit, larger... but we're not certain exactly what it
     //  is! Edge apparently has no such limit.)
     // [1] - https://github.com/WebKit/webkit/blob/1cb2227f6b2a1035f7bdc46e5ab69debb75fc1de/Source/WebCore/css/RuleSet.h#L68
-    for (let i = 0; i < selectors.length; i += this.selectorGroupSize)
+    for (let i = 0; i < preparedSelectors.length; i += this.selectorGroupSize)
     {
-      let selector = selectors.slice(i, i + this.selectorGroupSize).join(", ");
+      let selector = preparedSelectors.slice(i, i + this.selectorGroupSize).join(", ");
       this.style.sheet.insertRule(selector + "{display: none !important;}",
                                   this.style.sheet.cssRules.length);
     }
+
+    if (this.tracer)
+      this.tracer.addSelectors(selectors, filters);
   },
 
   apply()
   {
-    let selectors = null;
-    let elemHideEmulationLoaded = false;
-
-    let checkLoaded = function()
+    ext.backgroundPage.sendMessage({type: "get-selectors"}, response =>
     {
-      if (!selectors || !elemHideEmulationLoaded)
-        return;
-
       if (this.tracer)
         this.tracer.disconnect();
       this.tracer = null;
@@ -555,23 +560,12 @@ ElemHide.prototype = {
         this.style.parentElement.removeChild(this.style);
       this.style = null;
 
-      this.addSelectors(selectors.selectors);
-      this.elemHideEmulation.apply();
-
-      if (selectors.trace)
-        this.tracer = new ElementHidingTracer(selectors.selectors);
-    }.bind(this);
+      if (response.trace)
+        this.tracer = new ElementHidingTracer();
 
-    ext.backgroundPage.sendMessage({type: "get-selectors"}, response =>
-    {
-      selectors = response;
-      checkLoaded();
-    });
+      this.addSelectors(response.selectors, response.selectors);

[Sebastian Noack, 2017/02/23 11:24:03]

I wonder whether it would be nicer to handle this inside the addSelectors()
method, like that:

  this.tracer.addSelectors(selectors, filters || selectors);

That way we wouldn't have to duplicate the property lookup, and the semantics of
the second argument can be completely ignored, here. Also the code would be
slightly more concise. On the other hand, if we explicitly pass the selectors as
filters here, we avoid a logical condition. Not sure which is more preferable.

[wspee, 2017/02/23 14:24:48]

Done.

[Sebastian Noack, 2017/02/23 11:08:22]

Nit: I'd remove the blank line here (as it was before).

[wspee, 2017/02/23 11:20:21]

Done.

-    this.elemHideEmulation.load(() =>
-    {
-      elemHideEmulationLoaded = true;
-      checkLoaded();
+      this.elemHideEmulation.apply();
     });
   }
 };