Issue 5249 - Implement :-abp-contains()

chrome/content/elemHideEmulation.js

Index: chrome/content/elemHideEmulation.js
===================================================================
--- a/chrome/content/elemHideEmulation.js
+++ b/chrome/content/elemHideEmulation.js
@@ -225,16 +225,41 @@
         // 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 ContainsSelector(textContent)
+{
+  this._text = textContent;
+}
+
+ContainsSelector.prototype = {
+  requiresHiding: true,
+
+  *getSelectors(prefix, subtree, stylesheet)
+  {
+    for (let element of this.getElements(prefix, subtree, stylesheet))
+      yield [makeSelector(element, ""), subtree];
+  },
+
+  *getElements(prefix, subtree, stylesheet)
+  {
+    let actualPrefix = (!prefix || incompletePrefixRegexp.test(prefix)) ?
+        prefix + "*" : prefix;
+    let elements = subtree.querySelectorAll(actualPrefix);
+    for (let element of elements)
+      if (element.textContent == this._text)

[Wladimir Palant, 2017/06/28 10:47:24]

An exact match is definitely not what we want here, especially if you think
about whitespace. Just confirmed by looking at the implementation in Sizzle: a
case-sensitive substring match is the expected behavior. In other words:

  if (element.textContent.contains(this._text))

As to your concerns about this behavior being greedy: yes, if we have a selector
like `:contains(Advertisement)` without any further qualifiers we'll potentially
hide the document element. While it doesn't make a whole lot of sense, this is
the correct behavior IMHO. If overshooting like this isn't the idea, the
selector should have additional qualifiers to limit the number of potentially
matching elements.

I think that we can catch this issue by rejecting chains consisting only of a
single ContainsSelector instance - these never make sense. On the top level,
such filters will either hide nothing (text not found on the page) or the entire
document element. Inside an :-abp-has() pseudo they don't make sense either,
:-abp-has() should simply be replaced by :-abp-contains() to achieve the same
effect. Selectors like `#sidebar > :contains(Advertisement)` or
`:contains(Advertisement) + .block` are valid however as they will only apply to
specific elements (the latter being quite suboptimal performance-wise but
still).

As to your considerations about textContent vs. innerText, getting the text "as
rendered" is a very complicated affair and switching to a bogus IE-compat
feature won't save us here. A quick test in Firefox shows inconsistent handling
of display:none (hidden nodes as children are ignored but element.innerText of a
hidden node returns text content). Also, tricks like `font-size:0` or `::before`
pseudo aren't being considered here. So dealing with potential circumvention
isn't trivial, let's wait and see what shows up in the wild first.

[hub, 2017/06/28 14:39:10]

Done.

+        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 ", "}");
@@ -318,16 +343,18 @@
       selectors.push(new PropsSelector(content.text));
     else if (match[1] == "has")
     {
       let hasSelectors = this.parseSelector(content.text);
       if (hasSelectors == null)
         return null;
       selectors.push(new HasSelector(hasSelectors));
     }
+    else if (match[1] == "contains")
+      selectors.push(new ContainsSelector(content.text));
     else
     {
       // this is an error, can't parse selector.
       this.window.console.error(
         new SyntaxError("Failed to parse Adblock Plus " +
                         `selector ${selector}, invalid ` +
                         `pseudo-class :-abp-${match[1]}().`));
       return null;