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
@@ -1,12 +1,15 @@
 // We are currently limited to ECMAScript 5 in this file, because it is being
 // used in the browser tests. See https://issues.adblockplus.org/ticket/4796

[Wladimir Palant, 2017/04/25 10:57:52]

This is really a PITA. Given that https://issues.adblockplus.org/ticket/4796 is
no longer blocked, I wonder whether we can fix it before changing this script.

[hub, 2017/04/25 19:42:47]

If issue 4796 lands before this, then I'll fix it.

[Wladimir Palant, 2017/04/26 00:03:09]

As I suggested on IRC, it would probably be easiest if you work on issue 4796 -
I can answer any questions you might have on that. Otherwise I'll try to find
time, adding more ECMAScript 5 code here doesn't seem to be a good idea to me.

[hub, 2017/04/27 17:43:04]

Newer patch is now ES6 based on top of your patch. Consider this done.

 
-var propertySelectorRegExp = /\[\-abp\-properties=(["'])([^"']+)\1\]/;
+var propertySelectorRegExp = /\[-abp-properties=(["'])([^"']+)\1\]/;
+var abpSelectorRegExp = /\[-abp-selector=(["'])(.+)\1\]/;
+var pseudoClassHasSelectorRegExp = /:has\((.*)\)/;
+var pseudoClassPropsSelectorRegExp = /:-abp-properties\((["'])([^"']+)\1\)/;

[Wladimir Palant, 2017/04/25 11:30:46]

Having :has() but :-abp-properties() is inconsistent. Either we say that
[-abp-selector="..."] wrapper is sufficient as a warning and we can have our
custom pseudos inside unprefixed. Or we say that we want to indicate our custom
pseudos explicitly and then it should be :-abp-has() as well. I don't think that
we can claim standards compliance and compatibility with future browser
implementations for our :has() pseudo, so it isn't any better off than
:properties(). If you ask me, it's probably better to have :-abp-has() and
:-abp-properties().

[hub, 2017/04/25 19:42:47]

OK.

 
 function splitSelector(selector)
 {
   if (selector.indexOf(",") == -1)
     return [selector];
 
   var selectors = [];
   var start = 0;
@@ -36,132 +39,341 @@
       }
     }
   }
 
   selectors.push(selector.substring(start));
   return selectors;
 }
 
-function ElemHideEmulation(window, getFiltersFunc, addSelectorsFunc)
+const ABP_ATTR = "abp-0ac791f0-a03b-4f2c-a935-a285bc2e668e";
+const ABP_ATTR_SEL = "[" + ABP_ATTR + "] ";
+
+function matchChildren(e, selector)
+{
+  var subElement;
+  var newSelector = ABP_ATTR_SEL + selector;
+  var parentNode = e.parentNode || document;
+
+  e.setAttribute(ABP_ATTR, "");
+  subElement = parentNode.querySelector(newSelector);
+  e.removeAttribute(ABP_ATTR);

[Wladimir Palant, 2017/04/25 10:57:52]

This hack is problematic. Even if we make the attribute random, the website can
get notified about us adding it via mutation observers and mess with us (e.g. by
removing it again immediately). Given that the point is creating a selector to
uniquely identify an element, there is a better solution - we can generate a
selector of the form :root > :nth-child(2) > :nth-child(8) > :nth-child(1).
Given that the browser evaluates CSS selectors starting at the end, this won't
even be noticeable slower than what you have here. Note that these selectors
won't stay valid if the document structure changes, but they don't have to - as
long as the code is evaluated synchronously and we don't touch the DOM the
website cannot become active and change something.

[hub, 2017/04/25 19:42:47]

I was thinking we could generate an actually random attribute to counter this,
but this would just be raising the bar.

[hub, 2017/04/27 17:43:04]

On 2017/04/25 19:42:47, hub wrote:

Newer patch removed this and followed your other suggestion.

+
+  return subElement != null;
+}
+
+function selectChildren(e, selector)
+{
+  var subElements;
+  var newSelector = ABP_ATTR_SEL + selector;
+  var parentNode = e.parentNode || document;
+
+  e.setAttribute(ABP_ATTR, "");
+  subElements = parentNode.querySelectorAll(newSelector);
+  e.removeAttribute(ABP_ATTR);
+
+  return subElements;
+}
+
+/** Unwrap the pattern out of a [-abp-selector=''] if necessary
+ * @param {object} pattern - The pattern object to unwrap
+ * @return {object} the parsed pattern object or undefined if nothing parse.
+ */
+function unwrapPattern(pattern)

[Wladimir Palant, 2017/04/25 10:57:53]

This unwrapping belongs into lib/filterClasses.js IMHO - it needs to recognize
emulation filters by the fact that their selector matches
/^\[-abp-selector=(["']).*\1\]$/, so it can return the "inner part" as the
actual selector. The content script shouldn't get the wrapper at all. Note that
IMHO the entire selector should be wrapped in [-abp-selector="..."] because the
entire selector will be processed via our custom logic.

[hub, 2017/04/25 19:42:47]

ok, moving it.

+{
+  var match = abpSelectorRegExp.exec(pattern.selector);
+  if (match)
+  {
+    var prefix = pattern.selector.substr(0, match.index).trim();
+    var suffix = pattern.selector.substr(match.index + match[0].length).trim();
+    var selector = prefix + match[2] + suffix;
+    return parsePattern({selector: selector}, true);
+  }
+
+  return parsePattern(pattern, false);
+}
+
+function parsePattern(pattern, unwrapped)
+{
+  var selector = pattern.selector;
+  var match;
+
+  if (unwrapped)
+  {
+    // Seems that the pattern should be unwrapped.
+    // This is a fundamental error.
+    if (selector.indexOf("[-abp-selector=") != -1)
+      return;
+
+    match = pseudoClassHasSelectorRegExp.exec(selector);

[Wladimir Palant, 2017/04/25 10:57:52]

The syntax is getting too complex, parsing CSS with regular expressions won't
work. For example, your current regular expression will mistreat "p:has(div) >
span:not(.foo)". If you switch to non-greedy expressions you will get issues
with "p:has(div:not(.foo))". There is no way around it - we need to count
parentheses properly and consider quotation marks and backslashes. Feel free to
test which one is faster in JavaScript - going though characters one by one like
you would do in C++ or using `selector.replace(/([^()"'\\]+)|([()"']|(\\.))/g,
match => {...})`.

+    if (match)
+      return {
+        type: "has",
+        text: pattern.text,
+        elementMatcher: new PseudoHasMatcher(match[1]),
+        prefix: selector.substr(0, match.index).trim(),
+        suffix: selector.substr(match.index + match[0].length).trim()
+      };
+    match = pseudoClassPropsSelectorRegExp.exec(selector);
+  }
+
+  if (!match)
+    match = propertySelectorRegExp.exec(selector);

[Wladimir Palant, 2017/04/25 10:57:52]

I don't think that we should have this special handling for the legacy property
filter syntax here. We can instead modify ElemHideBase.fromText() in
lib/filterClasses.js in such a way that it converts such filters immediately -
'example.com##foo [-abp-properties="foobar"] bar' should become
'example.com##[-abp-selector="foo :-abp-properties(foobar) bar"]' and show up in
the filter list like that. So when we are working with filters we can assume
that they always have the current syntax.

[hub, 2017/04/25 19:42:47]

good idea. Like unwrap above.

+  if (match)
+  {
+    var regexpString;
+    var propertyExpression = match[2];
+    if (propertyExpression.length >= 2 && propertyExpression[0] == "/" &&
+        propertyExpression[propertyExpression.length - 1] == "/")
+      regexpString = propertyExpression.slice(1, -1)
+      .replace("\\x7B ", "{").replace("\\x7D ", "}");
+    else
+      regexpString = filterToRegExp(propertyExpression);
+    return {
+      type: "props",
+      text: pattern.text,
+      regexp: new RegExp(regexpString, "i"),
+      prefix: selector.substr(0, match.index),
+      suffix: selector.substr(match.index + match[0].length)
+    };
+  }
+}
+
+function matchStyleProps(style, rule, pattern, selectors, filters)
+{
+  if (pattern.regexp.test(style))
+  {
+    var subSelectors = splitSelector(rule.selectorText);
+    for (var i = 0; i < subSelectors.length; i++)
+    {
+      var subSelector = subSelectors[i];
+      selectors.push(pattern.prefix + subSelector + pattern.suffix);
+      filters.push(pattern.text);
+    }
+  }
+}
+
+function findPropsSelectors(stylesheet, patterns, selectors, filters)
+{
+  var rules = stylesheet.cssRules;
+  if (!rules)
+    return;
+
+  for (var i = 0; i < rules.length; i++)
+  {
+    var rule = rules[i];
+    if (rule.type != rule.STYLE_RULE)
+      continue;
+
+    var style = stringifyStyle(rule.style);
+    for (var j = 0; j < patterns.length; j++)
+    {
+      matchStyleProps(style, rule, patterns[j], selectors, filters);
+    }
+  }
+}
+
+/**
+ * Match the selector @pattern containing :has() starting from @node.
+ * @param {string} pattern - the pattern to match
+ * @param {object} node - the top level node.
+ */
+function pseudoClassHasMatch(pattern, node, stylesheets, elements, filters)
+{
+  // select element for the prefix pattern. Or just use node.
+  var haveEl = pattern.prefix ? node.querySelectorAll(pattern.prefix) : [ node ];

[Wladimir Palant, 2017/04/25 10:57:53]

This will work correctly for selectors like "div > span:has(...)" - but you
should also consider the scenario where we have :has() without additional
qualifiers, e.g. "div > span > :has(...)". Looking at
https://drafts.csswg.org/selectors-3/#selectors, we need special handling for
combinator selectors. So if prefix ends with [\s>+~] we need to add "*" to it.
For my example you would run node.querySelectorAll("div > span > *") here.

There is also the question how we are supposed to deal with selectors like
"div:has(...):not(...)". We could detect :has() not being followed by a
combinator and reorder it so that we effectively process
"div:not(...):has(...)". It should be easier however to mandate that :has()
should always be the last specifier before a combinator and merely enforce it
here. So if we find :has() that is not at the end of the selector and is not
followed by [\s>+~] we should warn about an invalid selector and ignore it.

[hub, 2017/04/25 19:42:47]

I naively assumed that since these are filters, we could have a more restricted
syntax, at least on the extension. Even though sanitisation of the input isn't
performed at the moment.

[Wladimir Palant, 2017/04/26 00:03:09]

Feel free to suggest a meaningful subset of the syntax that we can parse in a
simpler fashion and enforce. Note that enforcing is important, people will
expect things to work as with any CSS selectors - if they don't, we at least
need to produce a meaningful error message.

+  for (var j = 0; j < haveEl.length; j++)
+  {
+    var matched = pattern.elementMatcher.match(haveEl[j], stylesheets);
+    if (!matched)
+      continue;
+
+    if (pattern.suffix)
+    {
+      var subElements = selectChildren(haveEl[j], pattern.suffix);
+      if (subElements)
+      {
+        for (var k = 0; k < subElements.length; k++)
+        {
+          elements.push(subElements[k]);
+          filters.push(pattern.text);
+        }
+      }
+    }
+    else
+    {
+      elements.push(haveEl[j]);
+      filters.push(pattern.text);
+    }
+  }
+}
+
+function stringifyStyle(style)
+{
+  var styles = [];
+  for (var i = 0; i < style.length; i++)
+  {
+    var property = style.item(i);
+    var value = style.getPropertyValue(property);
+    var priority = style.getPropertyPriority(property);
+    styles.push(property + ": " + value + (priority ? " !" + priority : "") + ";");
+  }
+  styles.sort();
+  return styles.join(" ");
+}
+
+/** matcher for the pseudo CSS4 class :has()
+ * For those browser that don't have it yet.
+ */
+function PseudoHasMatcher(selector)
+{
+  this.hasSelector = selector;
+  this.parsed = parsePattern({selector: this.hasSelector}, true);
+}
+
+PseudoHasMatcher.prototype = {
+  match: function(elem, stylesheets)
+  {
+    var selectors = [];
+
+    if (this.parsed)
+    {
+      var filters = []; // don't need this, we have a partial filter.
+      if (this.parsed.type == "has")
+      {
+        var child = elem.firstChild;
+        while (child)
+        {
+          if (child.nodeType === Node.ELEMENT_NODE)
+          {
+            var matches = [];
+            pseudoClassHasMatch(this.parsed, child, stylesheets, matches, filters);
+            if (matches.length > 0)
+              return true;
+          }
+          child = child.nextSibling;
+        }
+        return false;
+      }
+      if (this.parsed.type == "props")
+      {
+        for (var i = 0; i < stylesheets.length; i++)
+          findPropsSelectors(stylesheets[i], [this.parsed], selectors, filters);

[Wladimir Palant, 2017/04/25 10:57:52]

We will be going through the stylesheets multiple times, this is rather
suboptimal.

+      }
+    }
+    else
+    {
+      selectors = [this.hasSelector];
+    }
+
+    var matched = false;
+    // look up for all elements that match the :has().
+    for (var k = 0; k < selectors.length; k++)
+    {
+      try
+      {
+        matched = matchChildren(elem, selectors[k]);
+        if (matched)
+          break;
+      }
+      catch (e)
+      {
+        console.error("Exception with querySelector()", selectors[k], e);
+      }
+    }
+    return matched;
+  }
+};
+
+function ElemHideEmulation(window, getFiltersFunc, addSelectorsFunc, hideElementsFunc)
 {
   this.window = window;
   this.getFiltersFunc = getFiltersFunc;
   this.addSelectorsFunc = addSelectorsFunc;
+  this.hideElementsFunc = hideElementsFunc;

[Wladimir Palant, 2017/04/25 10:57:53]

I guess that hideElementsFunc should do some kind of DOM modification. If
somehow possible, we should avoid that, already because we might end up fighting
mutation observers then. Ideal solution would generate selectors for the
elements to be hidden and pass them to the existing addSelectorsFunc.

 }
 
 ElemHideEmulation.prototype = {
-  stringifyStyle: function(style)
-  {
-    var styles = [];
-    for (var i = 0; i < style.length; i++)
-    {
-      var property = style.item(i);
-      var value    = style.getPropertyValue(property);
-      var priority = style.getPropertyPriority(property);
-      styles.push(property + ": " + value + (priority ? " !" + priority : "") + ";");
-    }
-    styles.sort();
-    return styles.join(" ");
-  },
 
   isSameOrigin: function(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: function(stylesheet, selectors, filters)
+  findPseudoClassHasElements: function(node, stylesheets, elements, filters)
   {
-    // Explicitly ignore third-party stylesheets to ensure consistent behavior
-    // between Firefox and Chrome.
-    if (!this.isSameOrigin(stylesheet))
-      return;
-
-    var rules = stylesheet.cssRules;
-    if (!rules)
-      return;
-
-    for (var i = 0; i < rules.length; i++)
+    for (var i = 0; i < this.pseudoHasPatterns.length; i++)
     {
-      var rule = rules[i];
-      if (rule.type != rule.STYLE_RULE)
-        continue;
-
-      var style = this.stringifyStyle(rule.style);
-      for (var j = 0; j < this.patterns.length; j++)
-      {
-        var pattern = this.patterns[j];
-        if (pattern.regexp.test(style))
-        {
-          var subSelectors = splitSelector(rule.selectorText);
-          for (var k = 0; k < subSelectors.length; k++)
-          {
-            var subSelector = subSelectors[k];
-            selectors.push(pattern.prefix + subSelector + pattern.suffix);
-            filters.push(pattern.text);
-          }
-        }
-      }
+      pseudoClassHasMatch(this.pseudoHasPatterns[i], node, stylesheets, elements, filters);
     }
   },
 
   addSelectors: function(stylesheets)
   {
     var selectors = [];
     var filters = [];
     for (var i = 0; i < stylesheets.length; i++)
-      this.findSelectors(stylesheets[i], selectors, filters);
+    {
+      // Explicitly ignore third-party stylesheets to ensure consistent behavior
+      // between Firefox and Chrome.
+      if (!this.isSameOrigin(stylesheets[i]))
+        continue;
+      findPropsSelectors(stylesheets[i], this.propSelPatterns, selectors, filters);
+    }
     this.addSelectorsFunc(selectors, filters);
   },
 
+  hideElements: function(stylesheets)
+  {
+    var elements = [];
+    var filters = [];
+    this.findPseudoClassHasElements(document, stylesheets, elements, filters);
+    this.hideElementsFunc(elements, filters);
+  },
+
   onLoad: function(event)
   {
     var stylesheet = event.target.sheet;
     if (stylesheet)
       this.addSelectors([stylesheet]);
+    this.hideElements([stylesheet]);
   },
 
   apply: function()
   {
     this.getFiltersFunc(function(patterns)
     {
-      this.patterns = [];
+      this.propSelPatterns = [];
+      this.pseudoHasPatterns = [];
       for (var i = 0; i < patterns.length; i++)
       {
         var pattern = patterns[i];
-        var match = propertySelectorRegExp.exec(pattern.selector);
-        if (!match)
+        var parsed = unwrapPattern(pattern);
+        if (parsed == undefined)
           continue;
-
-        var propertyExpression = match[2];
-        var 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(match.index + match[0].length)
-        });
+        if (parsed.type == "props")
+        {
+          this.propSelPatterns.push(parsed);
+        }
+        else if (parsed.type == "has")
+        {
+          this.pseudoHasPatterns.push(parsed);
+        }
       }
 
-      if (this.patterns.length > 0)
+      if (this.pseudoHasPatterns.length > 0 || this.propSelPatterns.length > 0)
       {
         var document = this.window.document;
         this.addSelectors(document.styleSheets);
+        this.hideElements(document.styleSheets);
         document.addEventListener("load", this.onLoad.bind(this), true);
       }
     }.bind(this));
   }
 };