Issue 3143 - Filter elements with :-abp-has()

chrome/content/elemHideEmulation.js

Index: chrome/content/elemHideEmulation.js
===================================================================
--- a/chrome/content/elemHideEmulation.js
+++ b/chrome/content/elemHideEmulation.js
@@ -54,172 +54,360 @@
       }
     }
   }
 
   selectors.push(selector.substring(start));
   return selectors;
 }
 
-function ElemHideEmulation(window, getFiltersFunc, addSelectorsFunc)
+/** Return position of node from parent.
+ * @param {Node} node the node to find the position of.
+ * @return {number} One-based index like for :nth-child(), or 0 on error.
+ */
+function positionInParent(node)
+{
+  let {children} = node.parentNode;
+  for (let i = 0; i < children.length; i++)
+    if (children[i] == node)
+      return i + 1;
+  return 0;
+}
+
+function makeSelector(node, selector)
+{
+  if (!node.parentElement)
+  {
+    let newSelector = ":root";
+    if (selector)
+      newSelector += " > ";
+    return newSelector + selector;
+  }
+  let idx = positionInParent(node);
+  if (idx > 0)
+  {
+    let newSelector = `${node.tagName}:nth-child(${idx})`;
+    if (selector)
+      newSelector += " > ";
+    return makeSelector(node.parentElement, newSelector + selector);
+  }
+
+  return selector;
+}
+
+function parseSelectorContent(content)
+{
+  let parens = 1;
+  let quote = null;
+  let i;
+  for (i = 0; i < content.length; i++)
+  {
+    let c = content[i];
+    if (c == "\\")
+    {
+      // Ignore escaped characters
+      i++;
+    }
+    else if (quote)
+    {
+      if (c == quote)
+        quote = null;
+    }
+    else if (c == "'" || c == '"')
+      quote = c;
+    else if (c == "(")
+      parens++;
+    else if (c == ")")
+    {
+      parens--;
+      if (parens == 0)
+        break;
+    }
+  }
+
+  if (parens > 0)
+    return null;
+  return {text: content.substr(0, i), end: i};
+}
+
+/** Parse the selector
+ * @param {string} selector the selector to parse
+ * @param {Number} level the depth level. 0 is top
+ * @return {Object} selectors is an array of objects,
+ * or null in case of errors. hide is true if we'll hide
+ * elements instead of styles..
+ */
+function parseSelector(selector, level = 0)

[Wladimir Palant, 2017/06/07 08:32:58]

Level parameter is no longer used.

[hub, 2017/06/07 14:15:08]

I meant to remove it. it is gone now.

+{
+  if (selector.length == 0)
+    return {selectors: [], hide: false};
+
+  let match = abpSelectorRegexp.exec(selector);
+  if (!match)
+    return {selectors: [new PlainSelector(selector)], hide: false};
+
+  let hide = false;
+  let selectors = [];
+  let suffixStart = match.index;
+  if (suffixStart > 0)
+    selectors.push(new PlainSelector(selector.substr(0, match.index)));

[Wladimir Palant, 2017/06/07 08:32:58]

I meant - suffixStart variable shouldn't be declared/assigned here at all.

[hub, 2017/06/07 14:15:08]

done

+
+  let startIndex = match.index + match[0].length;
+  let content = parseSelectorContent(selector.substr(startIndex));
+  if (content == null)
+  {
+    console.error(new SyntaxError("Failed parsing AdBlock Plus " +
+                                  `selector ${selector}, didn't ` +
+                                  "find closing parenthesis."));
+    return {selectors: null, hide: false};
+  }
+  if (match[1] == "properties")
+    selectors.push(new PropsSelector(content.text));
+  else if (match[1] == "has")
+  {
+    let hasSelector = new HasSelector(content.text);
+    if (!hasSelector.valid())
+      return {selectors: null, hide: false};
+    selectors.push(hasSelector);
+    hide = true;
+  }
+  else
+  {
+    // this is an error, can't parse selector.
+    console.error(new SyntaxError("Failed parsing AdBlock Plus " +
+                                  `selector ${selector}, invalid ` +
+                                  `pseudo-class -abp-${match[1]}().`));
+    return {selectors: null, hide: false};
+  }
+
+  suffixStart = startIndex + content.end + 1;
+
+  let suffix = parseSelector(selector.substr(suffixStart), level);
+  if (suffix.selectors == null)
+    return {selectors: null, hide: false};
+
+  selectors.push(...suffix.selectors);
+  hide |= suffix.hide;

[Wladimir Palant, 2017/06/07 08:32:58]

Using numerical operators on boolean values isn't a good idea in JavaScript,
this should really be:

  hide = hide || suffix.hide;

Either way, dragging this state all the way through introduces significant
complexity which IMHO isn't worth it. How about you return selectors as you did
before and when you need to decide whether a particular chain should hide you do
the following:

  let hide = selectors.some(s => s instanceof HasSelector);

If I see it correctly, this check needs to be performed only once per chain, so
not worth optimizing.

[hub, 2017/06/07 14:15:09]

The problem with moving this out is that the decision of hiding is performed
somewhere else and not in the parse selector logic. 
When I implemented :-abp-contains (it is another patch), I kept the changes in
here. This is a different tradeoff I guess, but I prefer it that way.

I have changed this line though, to do as you suggested.

[Wladimir Palant, 2017/06/07 14:53:41]

Assuming that I understood this sentence correctly, you can still generalize
this by adding a requiresHiding property to the prototype of HasSelector and
such - it would be true for HasSelector and false for everything else. You could
still replace this whole lot of complexity by four lines of code where three are
trivial. And you replace `if (!pattern.hide)` (the only place where this
property is actually used) by:

  if (!selectors.some(s => s.requiresHiding))

[hub, 2017/06/08 00:17:19]

done

+
+  return {selectors, hide};
+}
+
+function stringifyStyle(style)
+{
+  let styles = [];
+  for (let i = 0; i < style.length; i++)
+  {
+    let property = style.item(i);
+    let value = style.getPropertyValue(property);
+    let priority = style.getPropertyPriority(property);
+    styles.push(property + ": " + value + (priority ? " !" + priority : "") +
+                ";");
+  }
+  styles.sort();
+  return styles.join(" ");
+}
+
+function* evaluate(chain, index, prefix, subtree, styles)
+{
+  if (index >= chain.length)
+  {
+    yield prefix;
+    return;
+  }
+  for (let [selector, element] of
+       chain[index].getSelectors(prefix, subtree, styles))
+    yield* evaluate(chain, index + 1, selector, element, styles);
+}
+
+function PlainSelector(selector)
+{
+  this._selector = selector;
+}
+
+PlainSelector.prototype = {
+  /**
+   * Generator function returning a pair of selector
+   * string and subtree.
+   * @param {string} prefix the prefix for the selector.
+   * @param {Node} subtree the subtree we work on.
+   * @param {Array} styles the stringified stylesheet objects.
+   */
+  *getSelectors(prefix, subtree, styles)
+  {
+    yield [prefix + this._selector, subtree];
+  }
+};
+
+const incompletePrefixRegexp = /[\s>+~]$/;
+
+function HasSelector(selector, level = 0)
+{
+  let inner = parseSelector(selector, level + 1);
+  this._innerSelectors = inner ? inner.selectors : null;
+}
+
+HasSelector.prototype = {
+  valid()
+  {
+    return this._innerSelectors != null;
+  },
+
+  *getSelectors(prefix, subtree, styles)
+  {
+    for (let element of this.getElements(prefix, subtree, styles))
+      yield [makeSelector(element, ""), element];
+  },
+
+  /**
+   * Generator function returning selected elements.
+   * @param {string} prefix the prefix for the selector.
+   * @param {Node} subtree the subtree we work on.
+   * @param {Array} styles the stringified stylesheet objects.
+   */
+  *getElements(prefix, subtree, styles)
+  {
+    let actualPrefix = (!prefix || incompletePrefixRegexp.test(prefix)) ?
+        prefix + "*" : prefix;
+    let elements = subtree.querySelectorAll(actualPrefix);
+    for (let element of elements)
+    {
+      let newPrefix = makeSelector(element, "");
+      let iter = evaluate(this._innerSelectors, 0, newPrefix + " ",
+                          element, styles);
+      for (let selector of iter)
+        // we insert a space between the two. It becomes a no-op if selector
+        // doesn't have a combinator
+        if (subtree.querySelector(selector))
+          yield element;
+    }
+  }
+};
+
+function PropsSelector(propertyExpression)
+{
+  let regexpString;
+  if (propertyExpression.length >= 2 && propertyExpression[0] == "/" &&
+      propertyExpression[propertyExpression.length - 1] == "/")
+  {
+    regexpString = propertyExpression.slice(1, -1)
+      .replace("\\x7B ", "{").replace("\\x7D ", "}");
+  }
+  else
+    regexpString = filterToRegExp(propertyExpression);
+
+  this._regexp = new RegExp(regexpString, "i");
+}
+
+PropsSelector.prototype = {
+  *findPropsSelectors(styles, prefix, regexp)
+  {
+    for (let style of styles)
+      if (regexp.test(style.style))
+        for (let subSelector of style.subSelectors)
+          yield prefix + subSelector;
+  },
+
+  *getSelectors(prefix, subtree, styles)
+  {
+    for (let selector of this.findPropsSelectors(styles, prefix, this._regexp))
+      yield [selector, subtree];
+  }
+};
+
+function ElemHideEmulation(window, getFiltersFunc, addSelectorsFunc,
+                           hideElemsFunc)
 {
   this.window = window;
   this.getFiltersFunc = getFiltersFunc;
   this.addSelectorsFunc = addSelectorsFunc;
+  this.hideElemsFunc = hideElemsFunc;
 }
 
 ElemHideEmulation.prototype = {
-  stringifyStyle(style)
-  {
-    let styles = [];
-    for (let i = 0; i < style.length; i++)
-    {
-      let property = style.item(i);
-      let value = style.getPropertyValue(property);
-      let priority = style.getPropertyPriority(property);
-      styles.push(property + ": " + value + (priority ? " !" + priority : "") +
-                  ";");
-    }
-    styles.sort();
-    return styles.join(" ");
-  },
-
   isSameOrigin(stylesheet)
   {
     try
     {
       return new URL(stylesheet.href).origin == this.window.location.origin;
     }
     catch (e)
     {
       // Invalid URL, assume that it is first-party.
       return true;
     }
   },
 
-  findSelectors(stylesheet, selectors, filters)
-  {
-    // Explicitly ignore third-party stylesheets to ensure consistent behavior
-    // between Firefox and Chrome.
-    if (!this.isSameOrigin(stylesheet))
-      return;
-
-    let rules = stylesheet.cssRules;
-    if (!rules)
-      return;
-
-    for (let rule of rules)
-    {
-      if (rule.type != rule.STYLE_RULE)
-        continue;
-
-      let style = this.stringifyStyle(rule.style);
-      for (let pattern of this.patterns)
-      {
-        if (pattern.regexp.test(style))
-        {
-          let subSelectors = splitSelector(rule.selectorText);
-          for (let subSelector of subSelectors)
-          {
-            selectors.push(pattern.prefix + subSelector + pattern.suffix);
-            filters.push(pattern.text);
-          }
-        }
-      }
-    }
-  },
-
   addSelectors(stylesheets)
   {
     let selectors = [];
     let filters = [];
+
+    let hideElements = [];
+    let filterElements = [];

[Wladimir Palant, 2017/06/07 08:32:59]

This array contains filters, not elements. How about naming these four variables
selectors, selectorFilters, elements and elementFilters? This seems consistent.

[hub, 2017/06/07 14:15:08]

make sense. done.

+
+    let cssStyles = [];
+
     for (let stylesheet of stylesheets)
-      this.findSelectors(stylesheet, selectors, filters);
+    {
+      // Explicitly ignore third-party stylesheets to ensure consistent behavior
+      // between Firefox and Chrome.
+      if (!this.isSameOrigin(stylesheet))
+        continue;
+
+      let rules = stylesheet.cssRules;
+      if (!rules)
+        continue;
+
+      for (let rule of rules)
+      {
+        if (rule.type != rule.STYLE_RULE)
+          continue;
+
+        let style = stringifyStyle(rule.style);
+        let subSelectors = splitSelector(rule.selectorText);
+        cssStyles.push({style, subSelectors});
+      }
+    }
+
+    for (let pattern of this.patterns)
+      for (let selector of evaluate(pattern.selectors,
+                                    0, "", document, cssStyles))
+        if (!pattern.hide)
+        {
+          selectors.push(selector);
+          filters.push(pattern.text);
+        }
+        else
+          for (let element of document.querySelectorAll(selector))

[Wladimir Palant, 2017/06/07 08:32:59]

Nit: we require brackets around multi-line blocks, even if these only contain a
single statement.

[hub, 2017/06/07 14:15:09]

Done.

+          {
+            hideElements.push(element);
+            filterElements.push(pattern.text);
+          }
+
     this.addSelectorsFunc(selectors, filters);
+    this.hideElemsFunc(hideElements, filterElements);
   },
 
   onLoad(event)
   {
     let stylesheet = event.target.sheet;
     if (stylesheet)
       this.addSelectors([stylesheet]);
   },
 
   apply()
   {
     this.getFiltersFunc(patterns =>
     {
       this.patterns = [];
       for (let pattern of patterns)
       {
-        let match = abpSelectorRegexp.exec(pattern.selector);
-        if (!match || match[1] != "properties")
-        {
-          console.error(new SyntaxError(
-            `Failed to parse Adblock Plus selector ${pattern.selector}, ` +
-            `invalid pseudo-class :-abp-${match[1]}().`
-          ));
-          continue;
-        }
-
-        let expressionStart = match.index + match[0].length;
-        let parens = 1;
-        let quote = null;
-        let i;
-        for (i = expressionStart; i < pattern.selector.length; i++)
-        {
-          let c = pattern.selector[i];
-          if (c == "\\")
-          {
-            // Ignore escaped characters
-            i++;
-          }
-          else if (quote)
-          {
-            if (c == quote)
-              quote = null;
-          }
-          else if (c == "'" || c == '"')
-            quote = c;
-          else if (c == "(")
-            parens++;
-          else if (c == ")")
-          {
-            parens--;
-            if (parens == 0)
-              break;
-          }
-        }
-
-        if (parens > 0)
-        {
-          console.error(new SyntaxError(
-            `Failed to parse Adblock Plus selector ${pattern.selector} ` +
-            "due to unmatched parentheses."
-          ));
-          continue;
-        }
-
-        let propertyExpression = pattern.selector.substring(expressionStart, i);
-        let regexpString;
-        if (propertyExpression.length >= 2 && propertyExpression[0] == "/" &&
-            propertyExpression[propertyExpression.length - 1] == "/")
-        {
-          regexpString = propertyExpression.slice(1, -1)
-              .replace("\\x7B ", "{").replace("\\x7D ", "}");
-        }
-        else
-          regexpString = filterToRegExp(propertyExpression);
-
-        this.patterns.push({
-          text: pattern.text,
-          regexp: new RegExp(regexpString, "i"),
-          prefix: pattern.selector.substr(0, match.index),
-          suffix: pattern.selector.substr(i + 1)
-        });
+        let {selectors, hide} = parseSelector(pattern.selector);
+        if (selectors != null && selectors.length > 0)
+          this.patterns.push({selectors, hide, text: pattern.text});
       }
 
       if (this.patterns.length > 0)
       {
         let {document} = this.window;
         this.addSelectors(document.styleSheets);
         document.addEventListener("load", this.onLoad.bind(this), true);
       }