code review of procedural cosmetic filters + better validate :style option (#2278)

src/js/background.js

@@ -108,8 +108,8 @@ return {
 
     // read-only
     systemSettings: {
-        compiledMagic: 'xhjvmgkamffc',
+        compiledMagic: 'zelhzxrhkfjr',
-        selfieMagic: 'xhjvmgkamffc'
+        selfieMagic: 'zelhzxrhkfjr'
     },
 
     restoreBackupSettings: {

src/js/contentscript.js

@@ -417,8 +417,9 @@ var PSelector = function(o) {
     this.raw = o.raw;
     this.selector = o.selector;
     this.tasks = [];
-    var tasks = o.tasks, task, ctor;
+    var tasks = o.tasks;
-    for ( var i = 0; i < tasks.length; i++ ) {
+    if ( !tasks ) { return; }
+    for ( var i = 0, task, ctor; i < tasks.length; i++ ) {
         task = tasks[i];
         ctor = this.operatorToTaskMap.get(task[0]);
         this.tasks.push(new ctor(task));
@@ -455,26 +456,6 @@ PSelector.prototype.test = function(input) {
     return false;
 };
 
-var PSelectors = function() {
-    this.entries = [];
-};
-PSelectors.prototype.add = function(o) {
-    this.entries.push(new PSelector(o));
-};
-PSelectors.prototype.forEachNode = function(callback) {
-    var pfilters = this.entries,
-        i = pfilters.length,
-        pfilter, nodes, j;
-    while ( i-- ) {
-        pfilter = pfilters[i];
-        nodes = pfilter.exec();
-        j = nodes.length;
-        while ( j-- ) {
-            callback(nodes[j], pfilter);
-        }
-    }
-};
-
 /******************************************************************************/
 
 var domFilterer = {
@@ -498,8 +479,6 @@ var domFilterer = {
             this.entries.push(selector);
             this.selector = undefined;
         },
-        forEachNodeOfSelector: function(/*callback, root, extra*/) {
-        },
         forEachNode: function(callback, root, extra) {
             if ( this.selector === undefined ) {
                 this.selector = this.entries.join(extra + ',') + extra;
@@ -532,7 +511,29 @@ var domFilterer = {
             }
         }
     },
-    proceduralSelectors: new PSelectors(),           // Hiding filters: procedural
+    styleSelectors: {                           // Style filters
+        entries: [],
+        add: function(o) {
+            this.entries.push(o);
+        }
+    },
+    proceduralSelectors: {                      // Hiding filters: procedural
+        entries: [],
+        add: function(o) {
+            this.entries.push(new PSelector(o));
+        },
+        forEachNode: function(callback) {
+            var pfilters = this.entries, i = pfilters.length, pfilter, nodes, j;
+            while ( i-- ) {
+                pfilter = pfilters[i];
+                nodes = pfilter.exec();
+                j = nodes.length;
+                while ( j-- ) {
+                    callback(nodes[j], pfilter);
+                }
+            }
+        }
+    },
 
     addExceptions: function(aa) {
         for ( var i = 0, n = aa.length; i < n; i++ ) {
@@ -556,11 +557,13 @@ var domFilterer = {
         }
         var o = JSON.parse(selector);
         if ( o.style ) {
-            this.newStyleRuleBuffer.push(o.parts.join(' '));
+            this.newStyleRuleBuffer.push(o.style.join(' '));
+            this.styleSelectors.add(o);
             return;
         }
-        if ( o.procedural ) {
+        if ( o.tasks ) {
             this.proceduralSelectors.add(o);
+            return;
         }
     },
 

src/js/cosmetic-filtering.js

@@ -242,7 +242,7 @@ FilterBucket.fromSelfie = function() {
 /******************************************************************************/
 
 var FilterParser = function() {
-    this.prefix =  this.suffix = this.style = '';
+    this.prefix =  this.suffix = '';
     this.unhide = 0;
     this.hostnames = [];
     this.invalid = false;
@@ -254,7 +254,7 @@ var FilterParser = function() {
 
 FilterParser.prototype.reset = function() {
     this.raw = '';
-    this.prefix = this.suffix = this.style = '';
+    this.prefix = this.suffix = '';
     this.unhide = 0;
     this.hostnames.length = 0;
     this.invalid = false;
@@ -628,7 +628,6 @@ var FilterContainer = function() {
     this.netSelectorCacheCountMax = netSelectorCacheHighWaterMark;
     this.selectorCacheTimer = null;
     this.reHasUnicode = /[^\x00-\x7F]/;
-    this.reClassOrIdSelector = /^[#.][\w-]+$/;
     this.rePlainSelector = /^[#.][\w\\-]+/;
     this.rePlainSelectorEscaped = /^[#.](?:\\[0-9A-Fa-f]+ |\\.|\w|-)+/;
     this.rePlainSelectorEx = /^[^#.\[(]+([#.][\w-]+)/;
@@ -735,7 +734,16 @@ FilterContainer.prototype.freeze = function() {
 FilterContainer.prototype.compileSelector = (function() {
     var reStyleSelector = /^(.+?):style\((.+?)\)$/,
         reStyleBad = /url\([^)]+\)/,
-        reScriptSelector = /^script:(contains|inject)\((.+)\)$/;
+        reScriptSelector = /^script:(contains|inject)\((.+)\)$/,
+        div = document.createElement('div');
+
+    var isValidStyleProperty = function(cssText) {
+        if ( reStyleBad.test(cssText) ) { return false; }
+        div.style.cssText = cssText;
+        if ( div.style.cssText === '' ) { return false; }
+        div.style.cssText = '';
+        return true;
+    };
 
     return function(raw) {
         if ( isValidCSSSelector(raw) && raw.indexOf('[-abp-properties=') === -1 ) {
@@ -747,11 +755,10 @@ FilterContainer.prototype.compileSelector = (function() {
 
         // `:style` selector?
         if ( (matches = reStyleSelector.exec(raw)) !== null ) {
-            if ( isValidCSSSelector(matches[1]) && reStyleBad.test(matches[2]) === false ) {
+            if ( isValidCSSSelector(matches[1]) && isValidStyleProperty(matches[2]) ) {
                 return JSON.stringify({
-                    style: true,
                     raw: raw,
-                    parts: [ matches[1], '{' + matches[2] + '}' ]
+                    style: [ matches[1], '{' + matches[2] + '}' ]
                 });
             }
             return;
@@ -784,7 +791,7 @@ FilterContainer.prototype.compileSelector = (function() {
 /******************************************************************************/
 
 FilterContainer.prototype.compileProceduralSelector = (function() {
-    var reParserEx = /(:(?:has|has-text|if|if-not|matches-css|matches-css-after|matches-css-before|xpath))\(.+\)$/,
+    var reOperatorParser = /(:(?:has|has-text|if|if-not|matches-css|matches-css-after|matches-css-before|xpath))\(.+\)$/,
         reFirstParentheses = /^\(*/,
         reLastParentheses = /\)*$/,
         reEscapeRegex = /[.*+?^${}()|[\]\\]/g;
@@ -849,11 +856,9 @@ FilterContainer.prototype.compileProceduralSelector = (function() {
     ]);
 
     var compile = function(raw) {
-        var matches = reParserEx.exec(raw);
+        var matches = reOperatorParser.exec(raw);
         if ( matches === null ) {
-            if ( isValidCSSSelector(raw) ) {
+            if ( isValidCSSSelector(raw) ) { return { selector: raw }; }
-                return { selector: raw, tasks: [] };
-            }
             return;
         }
         var tasks = [],
@@ -864,7 +869,7 @@ FilterContainer.prototype.compileProceduralSelector = (function() {
             depth = 0, opening, closing;
         if ( firstOperand !== '' && isValidCSSSelector(firstOperand) === false ) { return; }
         for (;;) {
-            matches = reParserEx.exec(selector);
+            matches = reOperatorParser.exec(selector);
             if ( matches !== null ) {
                 nextOperand = selector.slice(0, matches.index);
                 nextOperator = matches[1];
@@ -903,7 +908,6 @@ FilterContainer.prototype.compileProceduralSelector = (function() {
         lastProceduralSelector = raw;
         var compiled = compile(raw);
         if ( compiled !== undefined ) {
-            compiled.procedural = true;
             compiled.raw = raw;
             compiled = JSON.stringify(compiled);
         }
@@ -965,15 +969,6 @@ FilterContainer.prototype.compile = function(s, out) {
         return true;
     }
 
-    // For hostname- or entity-based filters, class- or id-based selectors are
-    // still the most common, and can easily be tested using a plain regex.
-    if (
-        this.reClassOrIdSelector.test(parsed.suffix) === false &&
-        this.compileSelector(parsed.suffix) === undefined
-    ) {
-        return true;
-    }
-
     // https://github.com/chrisaljoudi/uBlock/issues/151
     // Negated hostname means the filter applies to all non-negated hostnames
     // of same filter OR globally if there is no non-negated hostnames.

src/js/reverselookup-worker.js

@@ -139,9 +139,9 @@ var fromCosmeticFilter = function(details) {
     // compiled form of a filter.
     var filterEx = '(' +
                    reEscape(filter) +
-                   '|[^\\v]+' +
+                   '|\{[^\\v]*' +
                    reEscape(JSON.stringify({ raw: filter }).slice(1,-1)) +
-                   '[^\\v]+)';
+                   '[^\\v]*\})';
 
     // Second step: find hostname-based versions.
     // Reference: FilterContainer.compileHostnameSelector().

src/js/scriptlets/cosmetic-logger.js

@@ -51,6 +51,17 @@ vAPI.domFilterer.simpleHideSelectors.entries.forEach(evaluateSelector);
 // Complex CSS selector-based cosmetic filters.
 vAPI.domFilterer.complexHideSelectors.entries.forEach(evaluateSelector);
 
+// Style cosmetic filters.
+vAPI.domFilterer.styleSelectors.entries.forEach(function(filter) {
+    if (
+        loggedSelectors.hasOwnProperty(filter.raw) === false &&
+        document.querySelector(filter.style[0]) !== null
+    ) {
+        loggedSelectors[filter.raw] = true;
+        matchedSelectors.push(filter.raw);
+    }
+});
+
 // Procedural cosmetic filters.
 vAPI.domFilterer.proceduralSelectors.entries.forEach(function(pfilter) {
     if (

src/js/scriptlets/element-picker.js

@@ -770,9 +770,9 @@ var filterToDOMInterface = (function() {
         }
         var elems;
         if ( o.style ) {
-            elems = document.querySelectorAll(o.parts[0]);
+            elems = document.querySelectorAll(o.style[0]);
-            lastAction = o.parts.join(' ');
+            lastAction = o.style.join(' ');
-        } else if ( o.procedural ) {
+        } else if ( o.tasks ) {
             elems = vAPI.domFilterer.createProceduralFilter(o).exec();
         }
         if ( !elems ) { return; }