[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, filters)
+  findSelectors: function(stylesheet, selectors, 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);
-          var selectors = [];
-
-          for (var k = 0; k < subSelectors.length; k++)
-            selectors.push(pattern.prefix + subSelectors[k] + pattern.suffix);
-
-          if (!filters.has(pattern.text))
-            filters.set(pattern.text, []);
-          Array.prototype.push.apply(filters.get(pattern.text), selectors);

[Sebastian Noack, 2017/02/07 12:35:59]

Creating a temporary array, just to merge it with another array, seems
unnecessary, here. How about following approach?

  let selectors = filters.get(pattern.text);
  if (!selectors)
  {
    selectors = [];
    filters.set(pattern.text, selectors);
  }

  for (let subSelector of subSelectors)
    selectors.push(pattern.prefix + subSelector + pattern.suffix);

Note that while on it, I also replaced the for(;;) loop with a for..of loop, for
simplicity and compliance with our coding practices for modern code. You might
want to do the same below.

And by the way, if you should ever need to pass variable arguments to a
function, in modern code, be aware of the rest operator; i.e. `o.f(...vargs)` is
equivalent to `f.apply(o, vargs)`, but much more readable.

[wspee, 2017/02/07 14:15:08]

Done.

+          let subSelectors = splitSelector(rule.selectorText);
+          for (let subSelector of subSelectors)
+          {
+            selectors.push(pattern.prefix + subSelector + pattern.suffix);
+            filters.push(pattern.text);
+          }
         }
       }
     }
   },
 
   addSelectors: function(stylesheets)
   {
-    var filters = new Map();
+    var selectors = [];
+    var filters = [];
     for (var i = 0; i < stylesheets.length; i++)
-      this.findSelectors(stylesheets[i], filters);
-    this.addSelectorsFunc(filters);
+      this.findSelectors(stylesheets[i], selectors, filters);
+    this.addSelectorsFunc(selectors, 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);
     }
   }
 };