Issue 6034 - :-abp-contains() accept a regular expression

lib/content/elemHideEmulation.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present 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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 "use strict";
 
-const {filterToRegExp, splitSelector} = require("../common");
+const {textToRegExp, filterToRegExp, splitSelector} = require("../common");
 
 let MIN_INVOCATION_INTERVAL = 3000;
 const MAX_SYNCHRONOUS_PROCESSING_TIME = 50;
 const abpSelectorRegexp = /:-abp-([\w-]+)\(/i;
 
 /** 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)
@@ skipping 84 matching lines @@
     let priority = rule.style.getPropertyPriority(property);
     styles.push(`${property}: ${value}${priority ? " !" + priority : ""};`);
   }
   styles.sort();
   return {
     style: styles.join(" "),
     subSelectors: splitSelector(rule.selectorText)
   };
 }
 
+let scopeSupported = null;
+
+function tryQuerySelector(subtree, selector, all)
+{
+  let elements = null;
+  try
+  {
+    elements = all ? subtree.querySelectorAll(selector) :
+      subtree.querySelector(selector);
+    scopeSupported = true;
+  }
+  catch (e)
+  {
+    // Edge doesn't support ":scope"
+    scopeSupported = false;
+  }
+  return elements;
+}
+
 /**
- * Check if a text argument is a regexp.
- * @param {string} text argument.

[Manish Jethani, 2018/01/09 07:53:02]

This should be:

  @param {string} text the text argument

Right?

[hub, 2018/01/09 18:33:06]

Done.

- * @return {?string} an unescaped RegExp string. null if it is not a
- * regexp, ie not surrounded by '/'.
+ * Query selector. If it is relative, will try :scope.
+ * @param {Node} subtree the element to query selector
+ * @param {string} selector the selector to query
+ * @param {bool} [all=false] true to perform querySelectorAll()
+ * @returns {?(Node|NodeList)} result of the query. null in case of error.
  */
-function checkRegExpParameter(text)
-{
-  let regexpString = null;
-  if (text.length >= 2 && text[0] == "/" &&
-      text[text.length - 1] == "/")
-  {
-    regexpString = text.slice(1, -1)
-      .replace("\\7B ", "{").replace("\\7D ", "}");

[Manish Jethani, 2018/01/09 07:53:02]

Why is this needed? If it's for legacy reasons, maybe we can drop it now for
-abp-contains

[hub, 2018/01/09 18:33:06]

We still want to have a valid CSS selector syntax.

[Manish Jethani, 2018/01/10 11:22:14]

Acknowledged.

[Manish Jethani, 2018/01/16 12:00:22]

Why does this have to be valid CSS selector syntax, since we're not actually
generating a CSS selector here? Unlike -abp-has (which does take a selector),
this only takes a string.

Imagine CSS had a contains pseudo-class [1]:

  div:contains("Hello {}") {
    display: none !important;
  }

Even if the quotes were optional, the parser should have no trouble parsing the
"{" correctly. The escaping is only required for CSS identifiers, not for plain
strings. [2]

If we do support escape sequences, we should support them properly [3], though
like I said it shouldn't be required in this case. We can simply accept the
string as a JavaScript regular expression without any CSS escaping rules applied
to it.

For what it's worth, a backslash in a regular expression would have to be
escaped with another backslash, so we would be looking at "\\\\7B " rather than
"\\7B "

[1]:
https://www.w3.org/Style/CSS/Test/CSS3/Selectors/20011105/html/full/flat/css3...
[2]: https://www.w3.org/TR/css3-values/#string-value
[3]: https://mathiasbynens.be/notes/css-escapes

-  }
-  return regexpString;
+function scopedQuerySelector(subtree, selector, all)
+{
+  if (selector[0] == ">")
+  {
+    selector = ":scope" + selector;
+    if (scopeSupported)
+    {
+      return all ? subtree.querySelectorAll(selector) :
+        subtree.querySelector(selector);
+    }
+    if (scopeSupported == null)
+      return tryQuerySelector(subtree, selector, all);
+    return null;
+  }
+  return all ? subtree.querySelectorAll(selector) :
+    subtree.querySelector(selector);
+}
+
+function scopedQuerySelectorAll(subtree, selector)
+{
+  return scopedQuerySelector(subtree, selector, true);
+}
+
+const regexpRegexp = /^\/(.*)\/([im]*)$/;
+
+/**
+ * Make a regular expression from a text argument. If it can be parsed as a
+ * regular expression, parse it and the flags.
+ * @param {string} text the text argument.
+ * @return {?RegExp} a RegExp object or null in case of error.
+ */
+function makeRegExpParameter(text)
+{
+  let [, pattern, flags] =
+      regexpRegexp.exec(text) || [undefined, textToRegExp(text)];
+
+  try
+  {
+    return new RegExp(pattern, flags);
+  }
+  catch (e)
+  {
+  }
+  return null;
 }
 
 function* evaluate(chain, index, prefix, subtree, styles)
 {
   if (index >= chain.length)
   {
     yield prefix;
     return;
   }
   for (let [selector, element] of
@@ skipping 23 matching lines @@
    * @param {Node} subtree the subtree we work on.
    * @param {StringifiedStyle[]} styles the stringified style objects.
    */
   *getSelectors(prefix, subtree, styles)
   {
     yield [prefix + this._selector, subtree];
   }
 };
 
 const incompletePrefixRegexp = /[\s>+~]$/;
-const relativeSelectorRegexp = /^[>+~]/;
 
 function HasSelector(selectors)
 {
   this._innerSelectors = selectors;
 }
 
 HasSelector.prototype = {
   requiresHiding: true,
 
   get dependsOnStyles()
@@ skipping 10 matching lines @@
   /**
    * Generator function returning selected elements.
    * @param {string} prefix the prefix for the selector.
    * @param {Node} subtree the subtree we work on.
    * @param {StringifiedStyle[]} styles the stringified style objects.
    */
   *getElements(prefix, subtree, styles)
   {
     let actualPrefix = (!prefix || incompletePrefixRegexp.test(prefix)) ?
         prefix + "*" : prefix;
-    let elements = subtree.querySelectorAll(actualPrefix);
-    for (let element of elements)
-    {
-      let iter = evaluate(this._innerSelectors, 0, "", element, styles);
-      for (let selector of iter)
-      {
-        if (selector == null)
+    let elements = scopedQuerySelectorAll(subtree, actualPrefix);
+    if (elements)
+    {
+      for (let element of elements)
+      {
+        let iter = evaluate(this._innerSelectors, 0, "", element, styles);
+        for (let selector of iter)
         {
-          yield null;
-          continue;
-        }
-        if (relativeSelectorRegexp.test(selector))
-          selector = ":scope" + selector;
-        try
-        {
-          if (element.querySelector(selector))
+          if (selector == null)
+            yield null;
+          else if (scopedQuerySelector(element, selector))
             yield element;
         }
-        catch (e)
-        {
-          // :scope isn't supported on Edge, ignore error caused by it.
-        }
+        yield null;
       }
-      yield null;
     }
   }
 };
 
 function ContainsSelector(textContent)
 {
-  let regexpString = checkRegExpParameter(textContent);
-  if (regexpString)
-    this._regexp = new RegExp(regexpString);

[Manish Jethani, 2018/01/09 07:53:02]

We have to consider what happens if the regular expression is invalid. For
example /(foo/ which contains an unterminated group. There are some options for
how to deal with this, but I think we should just catch the error here and set
this._regexp to null. Since this._text is null too, it'll evaluate to false in
this.match.

[lainverse, 2018/01/09 10:49:22]

I think it should be either "RegExp(regexpString, 'i')" to be case-insensitive
or checkRegExpParameter should support regexp flags defined right in the filter
and return new RegExp instead of just a string.

-  else
-    this._text = textContent;
+  this._regexp = makeRegExpParameter(textContent);
 }
 
 ContainsSelector.prototype = {
   requiresHiding: true,
 
-  match(element)
-  {
-    if (this._regexp)
-      return this._regexp.test(element.textContent);
-
-    return element.textContent.includes(this._text);

[lainverse, 2018/01/09 10:49:22]

I wasn't quite sure that "string.includes()" actually better than
"regexp.test()" and made this test to check:
https://jsperf.com/compare-regexp-test-and-string-include/1

...and it looks like "string.includes()" 30~40 percent slower both in Google
Chrome 63 and Firefox 58 DE on my system.

So, it might be a good idea to construct new RegExp even from a simple string
and use .test() in all cases. And drop this function.

[hub, 2018/01/09 18:33:06]

I'll rewrite this part taking the comments above into account aswell. Looks like
I'll use regexp matching even for plain text.

-  },
-
   *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 (this.match(element))
-        yield element;
-      else
-        yield null;
+
+    let elements = scopedQuerySelectorAll(subtree, actualPrefix);
+    if (elements)
+    {
+      for (let element of elements)
+      {
+        if (this._regexp && this._regexp.test(element.textContent))
+          yield element;
+        else
+          yield null;
+      }
     }
   }
 };
 
 function PropsSelector(propertyExpression)
 {
-  let regexpString = checkRegExpParameter(propertyExpression);
-  if (!regexpString)
+  let regexpString;
+  if (propertyExpression.length >= 2 && propertyExpression[0] == "/" &&
+      propertyExpression[propertyExpression.length - 1] == "/")
+  {
+    regexpString = propertyExpression.slice(1, -1)
+      .replace("\\7B ", "{").replace("\\7D ", "}");
+  }
+  else
     regexpString = filterToRegExp(propertyExpression);
 
   this._regexp = new RegExp(regexpString, "i");
 }
 
 PropsSelector.prototype = {
   preferHideWithSelector: true,
   dependsOnStyles: true,
 
   *findPropsSelectors(styles, prefix, regexp)
@@ skipping 329 matching lines @@
           characterData: true,
           subtree: true
         }
       );
       this.document.addEventListener("load", this.onLoad.bind(this), true);
     }
   }
 };
 
 exports.ElemHideEmulation = ElemHideEmulation;