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 176 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)
+    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 iter = evaluate(this._innerSelectors, 0, "", element, styles);
-      for (let selector of iter)
+      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

+      for (let element of elements)
       {
-        if (selector == null)
+        let iter = evaluate(this._innerSelectors, 0, "", element, styles);
+        for (let selector of iter)
         {
-          yield null;
-          continue;
+          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.
+          }
         }
-        if (relativeSelectorRegexp.test(selector))
-          selector = ":scope" + selector;
-        try
-        {
-          if (element.querySelector(selector))
-            yield element;
-        }
-        catch (e)
-        {
-          // :scope isn't supported on Edge, ignore error caused by it.
-        }
+        yield null;
       }
-      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;
-    let elements = subtree.querySelectorAll(actualPrefix);
+    if (relativeSelectorRegexp.test(actualPrefix))
+      actualPrefix = ":scope" + actualPrefix;
+    try
+    {
+      let elements = subtree.querySelectorAll(actualPrefix);
 
-    for (let element of elements)
+      for (let element of elements)
+      {
+        if (element.textContent.includes(this._text))
+          yield element;
+        else
+          yield null;
+      }
+    }
+    catch (e)
     {
-      if (element.textContent.includes(this._text))
-        yield element;
-      else
-        yield null;
+      // :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;