Issue 5422 - Properly build props selector

chrome/content/elemHideEmulation.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 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 31 matching lines @@
     else if (sep == "")
     {
       if (chr == '"' || chr == "'")
         sep = chr;
       else if (chr == "(")  // don't split between parentheses
         level++;            // e.g. :matches(div,span)
       else if (chr == ")")
         level = Math.max(0, level - 1);
       else if (chr == "," && level == 0)
       {
-        selectors.push(selector.substring(start, i));
+        selectors.push(selector.substring(start, i).trim());
         start = i + 1;
       }
     }
   }
 
-  selectors.push(selector.substring(start));
+  selectors.push(selector.substring(start).trim());
   return selectors;
 }
 
 /** 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)
 {
   let {children} = node.parentNode;
@@ skipping 197 matching lines @@
     regexpString = filterToRegExp(propertyExpression);
 
   this._regexp = new RegExp(regexpString, "i");
 }
 
 PropsSelector.prototype = {
   preferHideWithSelector: true,
 
   *findPropsSelectors(styles, prefix, regexp)
   {
+    let actualPrefix = (prefix && !incompletePrefixRegexp.test(prefix)) ?
+        prefix + " " : prefix;

[Wladimir Palant, 2017/08/16 08:37:02]

No, this is not what we want. Consider
.abp-testsuite-fail:-abp-properties(border: 4px solid #B30B0B) for example. This
should resolve into .abp-testsuite-fail.abp-testsuite-fail and hide the element.
Your changes will insert a space into this selector which will no longer match
anything. If a space is needed, it should be part of the filter.

[hub, 2017/08/16 19:50:32]

Acknowledged.

     for (let style of styles)
       if (regexp.test(style.style))
         for (let subSelector of style.subSelectors)
-          yield prefix + subSelector;
+        {
+          if (subSelector == "*")
+            subSelector = "";

[Wladimir Palant, 2017/08/16 08:37:02]

What if we have a filter like `:-abp-properties(foo)` and the only matching
subselector is "*"? This will produce an empty selector. Also, please consider a
filter like `foo+:-abp-properties(foo)`, we'll produce an invalid selector here.
Besides, this doesn't address the case where subSelector is something like
"*.foo"

It seems that we need to check whether subSelector starts with an asterisk. If
that's the case and we have a prefix that isn't incomplete, we should strip the
asterisk from subSelector. All the other changes here seem unnecessary.

[hub, 2017/08/16 19:50:32]

Done.

+          yield actualPrefix + subSelector;
+        }
   },
 
   *getSelectors(prefix, subtree, styles)
   {
     for (let selector of this.findPropsSelectors(styles, prefix, this._regexp))
       yield [selector, subtree];
   }
 };
 
 function ElemHideEmulation(window, getFiltersFunc, addSelectorsFunc,
@@ skipping 166 matching lines @@
 
       if (this.patterns.length > 0)
       {
         let {document} = this.window;
         this.addSelectors(document.styleSheets);
         document.addEventListener("load", this.onLoad.bind(this), true);
       }
     });
   }
 };