[adblockpluscore] Issue 4825 - Let ElemHideEmulation associate selector and filter when calling add…

chrome/content/elemHideEmulation.js

 var propertySelectorRegExp = /\[\-abp\-properties=(["'])([^"']+)\1\]/;
 
 function splitSelector(selector)
 {
   if (selector.indexOf(",") == -1)
     return [selector];
 
   var selectors = [];
   var start = 0;
   var level = 0;
@@ skipping 55 matching lines @@
     {
       return new URL(stylesheet.href).origin == this.window.location.origin;
     }
     catch (e)
     {
       // Invalid URL, assume that it is first-party.
       return true;
     }
   },
 
-  findSelectors: function(stylesheet, selectors, filters)
+  findSelectors: function(stylesheet, filters)
   {
     // Explicitly ignore third-party stylesheets to ensure consistent behavior
     // between Firefox and Chrome.
     if (!this.isSameOrigin(stylesheet))
       return;
 
-    var rules = stylesheet.cssRules;
+    let rules = stylesheet.cssRules;
     if (!rules)
       return;
 
-    for (var i = 0; i < rules.length; i++)
+    for (let rule of rules)
     {
-      var rule = rules[i];
       if (rule.type != rule.STYLE_RULE)
         continue;
 
-      var style = this.stringifyStyle(rule.style);
-      for (var j = 0; j < this.patterns.length; j++)
+      let style = this.stringifyStyle(rule.style);
+
+      for (let pattern of this.patterns)
       {
-        var pattern = this.patterns[j];
         if (pattern.regexp.test(style))
         {
-          var subSelectors = splitSelector(rule.selectorText);
-          for (var k = 0; k < subSelectors.length; k++)
-            selectors.push(pattern.prefix + subSelectors[k] + pattern.suffix);
-          filters[pattern.text] = true;
+          let selectors = filters.get(pattern.text);
+          if (!selectors)
+          {
+            selectors = [];
+            filters.set(pattern.text, selectors);
+          }
+
+          let subSelectors = splitSelector(rule.selectorText);

[Sebastian Noack, 2017/02/07 14:55:53]

The temporary variable is no longer necessary, as thanks to for..of loop syntax
the value only needs to be accessed one, so that the function call can be put
directly into the loop head.

[wspee, 2017/02/07 15:01:06]

Done.

+          for (let subSelector of subSelectors)
+            selectors.push(pattern.prefix + subSelector + pattern.suffix);
         }
       }
     }
   },
 
   addSelectors: function(stylesheets)
   {
-    var selectors = [];
-    var filters = {};
-    for (var i = 0; i < stylesheets.length; i++)
-      this.findSelectors(stylesheets[i], selectors, filters);
-    this.addSelectorsFunc(selectors, Object.keys(filters));
+    var filters = new Map();

[Sebastian Noack, 2017/02/07 14:55:53]

While you took the liberty to convert var-statements to let above, mind doing
the same here as well? (However, I wouldn't touch any functions that aren't
matter to this change though, in the same patch.)

[wspee, 2017/02/07 15:01:06]

Done.

+    for (let stylesheet of stylesheets)
+      this.findSelectors(stylesheet, filters);
+    this.addSelectorsFunc(filters);
   },
 
   onLoad: function(event)
   {
     var stylesheet = event.target.sheet;
     if (stylesheet)
       this.addSelectors([stylesheet]);
   },
 
   load: function(callback)
@@ skipping 32 matching lines @@
   apply: function()
   {
     if (this.patterns.length > 0)
     {
       var document = this.window.document;
       this.addSelectors(document.styleSheets);
       document.addEventListener("load", this.onLoad.bind(this), true);
     }
   }
 };