Issue 6296 - Handle relative prefix in :-abp-has()

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
@@ skipping 104 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;
+}
+
+/**
+ * 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 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);
+}
+
 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))
   {
@@ skipping 20 matching lines @@
    * @param {string} prefix the prefix for the selector.
    * @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>+~]$/;

[lainverse, 2018/01/30 18:07:28]

Since that other regexp were replaced with string comparison it might be a good
idea to replace this one too:

{
  let u = [];
  let n = 10000000;
  let c = 0;
  let r = /[\s>+~]$/;
  let incomplete = [' ','>','+','~'];
  let isIncompleteArr = c => incomplete.includes(c);
  let isIncompleteStr = c => c == ' ' || c == '>' || c == '+' || c == '~';
  for (let i = 0; i < n; i++)
  {
    if (Math.random() < .1)
      u.push("foo bar" + incomplete[Math.floor(Math.random()*4)]);
    else
      u.push("foo bar");
  }
  let s = performance.now();
  for (let i = 0; i < n; i++)
  {
    //if (r.test(u[i]))
    // if (isIncompleteArr(u[i][u[i].length-1]))
    if (isIncompleteStr(u[i][u[i].length-1]))
      c++;
  }
  let t = (performance.now() - s) | 0;
  console.log(c + " matches found in " + t + "ms");
}

In my tests function isIncompleteArr performs on par with regexp and
isIncompleteStr ~50ms faster both in Firefox and Chrome. Downside: I only check
for normal space character in both functions instead of \s as in the original
regexp. This should work fine if ABP normalize spaces in filters during import.

[Manish Jethani, 2018/01/30 20:23:20]

If we do this then I think that it should be a separate patch.

-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;
-    if (relativeSelectorRegexp.test(actualPrefix))

[Manish Jethani, 2018/01/23 16:42:11]

If I understand this correctly, "+" and "~" don't make sense in any context,
therefore they can be removed from relativeSelectorRegexp?

[hub, 2018/01/25 21:58:34]

Done.

-      actualPrefix = ":scope" + actualPrefix;
-    try
-    {
-      let elements = subtree.querySelectorAll(actualPrefix);

[lainverse, 2018/01/23 01:02:51]

As I understand if this fails it's still possible to make it work with
makeSelector(subtree) + ' ' + actualPrefix (without :scope and make sure to use
'HTML' instead of ':root' in the makeSelector in the Edge). Also, as I
understand, ':scope' won't break complete selectors, so if it's available then
it should be safe to use it all the time and leave regexp check only for
browsers which doesn't support it. Just check is it (and ':root') available with
something like this beforeahead:
function checkSelector(selector) {
  try 
  { document.querySelector(selector); }
  catch (e)
  { return false; }
  return true;
};
const isScopeAndRootAvailable = checkSelector(':scope') &&
checkSelector(':root');

And since we already know if scope and root are available then try-catch check
could be omitted since selector should be valid in all browsers.

[Manish Jethani, 2018/01/23 16:42:11]

I agree this could probably be made to work on Edge, but maybe that should be a
separate patch.

[hub, 2018/01/23 16:51:11]

Filed https://issues.adblockplus.org/ticket/6304

+    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)
         {
           if (selector == null)
-          {
             yield null;
-            continue;
-          }
-          if (relativeSelectorRegexp.test(selector))

[Manish Jethani, 2018/01/23 16:42:11]

So this and the try..catch could be made generic and put into a function:

  let scopeSupported = null;

  function tryQueryScopeSelectorAll(element, selector)
  {
    let result = [];
    try
    {
      result = element.querySelectorAll(selector);
      scopeSupported = true;
    }
    catch (e)
    {
      scopeSupported = false;
    }
    return result;
  }

  function querySelectorAll(element, selector)
  {
    if (relativeSelectorRegexp.test(selector))
    {
      selector = ":scope" + selector;
      if (scopeSupported)
        return element.querySelectorAll(selector);
      if (scopeSupported == null)
        return tryQueryScopeSelectorAll(element, selector);
      return [];
    }
    return element.querySelectorAll(selector);
  }

Then we can call querySelectorAll from the rest of the code and get an array
(possibly empty) of elements, and it'll only do the try..catch once.

I don't think we should always prefix with ":scope" because we still need to
support Edge for selectors that don't need it.

[hub, 2018/01/25 21:58:34]

This should allow us to fix issue #6304 more easily later. And avoid the try {}
catch {} penalty.

-            selector = ":scope" + selector;
-          try
-          {
-            if (element.querySelector(selector))
-              yield element;
-          }
-          catch (e)
-          {
-            // :scope isn't supported on Edge, ignore error caused by it.
-          }
+          else if (scopedQuerySelector(element, selector))
+            yield element;
         }
         yield null;
       }
     }
-    catch (e)
-    {
-      // :scope isn't supported on Edge, ignore error caused by it.
-    }
   }
 };
 
 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;
-    if (relativeSelectorRegexp.test(actualPrefix))
-      actualPrefix = ":scope" + actualPrefix;
-    try
-    {
-      let elements = subtree.querySelectorAll(actualPrefix);
-
+
+    let elements = scopedQuerySelectorAll(subtree, actualPrefix);
+    if (elements)
+    {
       for (let element of elements)
       {
         if (element.textContent.includes(this._text))
           yield element;
         else
           yield null;
       }
-    }
-    catch (e)
-    {
-      // :scope isn't supported on Edge, ignore error caused by it.
     }
   }
 };
 
 function PropsSelector(propertyExpression)
 {
   let regexpString;
   if (propertyExpression.length >= 2 && propertyExpression[0] == "/" &&
       propertyExpression[propertyExpression.length - 1] == "/")
   {
@@ skipping 343 matching lines @@
           characterData: true,
           subtree: true
         }
       );
       this.document.addEventListener("load", this.onLoad.bind(this), true);
     }
   }
 };
 
 exports.ElemHideEmulation = ElemHideEmulation;