[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.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)
+ addSelectors: function(selectors, filters)

[Sebastian Noack, 2017/02/17 13:55:31]

Nit: The indentation is one space off here.

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

Done.

   {
-    var matchedSelectors = [];
+    if (!filters)
+      filters = new Array(selectors.length);
 
-    // 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")
+      this.checkNodes([document], selectors, filters);
+
+    this.selectors.push(...selectors);
+    this.filters.push(...filters);
+  },
+
+  checkNodes: function(nodes, selectors, filters)
+  {
+    let matchedFilters = [];
+
+    for (let i = 0; i < selectors.length; i++)
     {
-      var selector = this.selectors[i];
+      nodes: for (let node of nodes)
+      {
+        let elements = node.querySelectorAll(selectors[i]);
 
-      for (var j = 0; j < nodes.length; j++)
-      {
-        var elements = nodes[j].querySelectorAll(selector);
-        var matched = false;
-
-        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;
+            let filter = filters[i] || selectors[i];
+            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)

[Sebastian Noack, 2017/02/17 13:55:31]

I find the purpose of this check more clear when comparing the length explicitly
against zero (as this code initially did), rather than checking whether the
length evaluates to false. It's also more consistent with our existing code.

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

Done.

       return;
 
     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
@@ skipping 12 matching lines @@
     // insertion point.
     if (this.shadow)
     {
       var preparedSelectors = [];
       for (var i = 0; i < selectors.length; i++)
       {
         var subSelectors = splitSelector(selectors[i]);
         for (var j = 0; j < subSelectors.length; j++)
           preparedSelectors.push("::content " + subSelectors[j]);
       }
-      selectors = preparedSelectors;
     }
 
     // Safari only allows 8192 primitive selectors to be injected at once[1], we
     // therefore chunk the inserted selectors into groups of 200 to be safe.
     // (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 (var i = 0; i < selectors.length; i += this.selectorGroupSize)
+    for (var i = 0; i < preparedSelectors.length; i += this.selectorGroupSize)

[Sebastian Noack, 2017/02/17 13:55:31]

Wouldn't preparedSelectors be undefined, on browsers where ShadowDOM does not
exist, and therefore this.shadow is null, and therefore the conditional block in
which preparedSelectors is defined won't be executed?

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

Done.

     {
-      var selector = selectors.slice(i, i + this.selectorGroupSize).join(", ");
+      var 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: function()
   {
-    var selectors = null;
-    var elemHideEmulationLoaded = false;
+    if (this.tracer)

[Sebastian Noack, 2017/02/17 13:55:31]

We should still wait until we have a response for "get-selectors" at least,
before removing the stylesheet and tracer, in order to minimize the side effects
when element hiding is reapplied. So please move this logic inside the callback.

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

Done.

+      this.tracer.disconnect();
+    this.tracer = null;
 
-    var checkLoaded = function()
+    if (this.style && this.style.parentElement)
+      this.style.parentElement.removeChild(this.style);
+    this.style = null;
+
+    ext.backgroundPage.sendMessage({type: "get-selectors"}, selectors =>
     {
-      if (!selectors || !elemHideEmulationLoaded)
-        return;
+      if (selectors.trace)
+        this.tracer = new ElementHidingTracer();
 
-      if (this.tracer)
-        this.tracer.disconnect();
-      this.tracer = null;
+      if (selectors.selectors)
+        this.addSelectors(selectors.selectors);
 
-      if (this.style && this.style.parentElement)
-        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);
-
-    ext.backgroundPage.sendMessage({type: "get-selectors"}, function(response)
-    {
-      selectors = response;
-      checkLoaded();
-    });
-
-    this.elemHideEmulation.load(function()
-    {
-      elemHideEmulationLoaded = true;
-      checkLoaded();
+      this.elemHideEmulation.load(

[Sebastian Noack, 2017/02/17 13:55:31]

The of the load() and apply() method did only exist in the first place, for the
synchronization which we just got rid off. So while changing
elemHideEmulation.js in adblockpluscore anyway, mind merging those two
functions, to simplify the logic here (as well as in Adblock Plus for Firefox,
where we do the same).

[wspee, 2017/02/18 11:20:24]

Done, see:

https://codereview.adblockplus.org/29375888/
https://codereview.adblockplus.org/29375891/

+        this.elemHideEmulation.apply.bind(this.elemHideEmulation));
     });
   }
 };
 
 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);
 }