[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 173 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(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)
         {
           // 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(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;
       }
     }
 
     if (matchedSelectors.length > 0)
       ext.backgroundPage.sendMessage({
         type: "devtools.traceElemHide",
         selectors: matchedSelectors
       });
   },
 
   onTimeout()
   {
-    this.checkNodes(this.changedNodes);
+    this.checkNodes(this.changedNodes, this.selectors, this.filters);
     this.changedNodes = [];
     this.timeout = null;
   },
 
   observe(mutations)
   {
     // Forget previously changed nodes that are no longer in the DOM.
     for (let i = 0; i < this.changedNodes.length; i++)
     {
       if (!document.contains(this.changedNodes[i]))
@@ skipping 41 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()
   {
-    this.checkNodes([document]);
+    this.checkNodes([document], this.selectors, this.filters);
 
     this.observer.observe(
       document,
       {
         childList: true,
         attributes: true,
         subtree: true
       }
     );
   },
@@ skipping 157 matching lines @@
             let shadow = shadowRoot(this);
             return shadow == ourShadowRoot ? null : shadow;
           }
         });
       }, null);
     }
 
     return shadow;
   },
 
-  addSelectors(selectors)
+  addSelectors(selectors, filters)
   {
     if (selectors.length == 0)
       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
                    || document.documentElement).appendChild(this.style);
 
       // It can happen that the frame already navigated to a different
       // document while we were waiting for the background page to respond.
       // In that case the sheet property will stay null, after addind the
       // <style> element to the shadow DOM.
       if (!this.style.sheet)
         return;
     }
 
     // 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
     // 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 (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;
 
       if (this.style && this.style.parentElement)
         this.style.parentElement.removeChild(this.style);
       this.style = null;
 
-      this.addSelectors(selectors.selectors);
+      if (response.trace)
+        this.tracer = new ElementHidingTracer();
+
+      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.

+
       this.elemHideEmulation.apply();
-
-      if (selectors.trace)
-        this.tracer = new ElementHidingTracer(selectors.selectors);
-    }.bind(this);
-
-    ext.backgroundPage.sendMessage({type: "get-selectors"}, response =>
-    {
-      selectors = response;
-      checkLoaded();
-    });
-

[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();
     });
   }
 };
 
 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.apply();
 
   document.addEventListener("error", event =>
   {
     checkCollapse(event.target);
   }, true);
 
   document.addEventListener("load", event =>
   {
     let element = event.target;
     if (/^i?frame$/.test(element.localName))
       checkCollapse(element);
   }, true);
 }