Issue 2395 - Added content script for CSS property filters

chrome/content/cssProperties.js

-CSSPropertyFilters = {
+function CSSPropertyFilters(window, addSelectorsFunc) {
+  this.window = window;
+  this.addSelectorsFunc = addSelectorsFunc;
+  this.load(this.apply.bind(this));
+}
+
+CSSPropertyFilters.prototype = {
   stringifyStyle: function(style)
   {
     var styles = [];
     for (var i = 0; i < style.length; i++)
     {
       var property = style.item(i);
       var value    = style.getPropertyValue(property);
       var priority = style.getPropertyPriority(property);
       styles.push(property + ": " + value + (priority ? " !" + priority : "") + ";");
     }
     styles.sort();
     return styles.join(" ");
   },
 
   findSelectors: function(stylesheet, selectors)
   {
     var rules = stylesheet.cssRules;
     if (!rules)
       return;
 
     for (var i = 0; i < rules.length; i++)
     {
       var rule = rules[i];
-      if (rule.type != CSSRule.STYLE_RULE)
+      if (rule.type != this.window.CSSRule.STYLE_RULE)
         continue;
 
       var style = this.stringifyStyle(rule.style);
       for (var j = 0; j < this.patterns.length; j++)
       {
         var pattern = this.patterns[j];
         var regexp = pattern.regexp;
 
         if (typeof regexp == "string")
           regexp = pattern.regexp = new RegExp(regexp);

[Wladimir Palant, 2015/10/23 18:36:37]

What about invalid regexps? new RegExp() can throw.

[Sebastian Noack, 2015/10/23 20:49:31]

The regular expression is generated by our code. See 
CSSPropertyFilter.regexpString in lib/filterClasses.js

 
         if (regexp.test(style))
           selectors.push(pattern.prefix + rule.selectorText + pattern.suffix);
       }
     }
   },
 
   addSelectors: function(stylesheets)
   {
     var selectors = [];
     for (var i = 0; i < stylesheets.length; i++)
       this.findSelectors(stylesheets[i], selectors);
-    addElemHideSelectors(selectors);
+    this.addSelectorsFunc(selectors);
   },
 
   onLoad: function(event)
   {
     var stylesheet = event.target.sheet;
     if (stylesheet)
-      CSSPropertyFilters.addSelectors([stylesheet]);
+      this.addSelectors([stylesheet]);
   },
 
   load: function(callback)
   {
     ext.backgroundPage.sendMessage(
       {
         type: "filters.get",
         what: "cssproperties"
       },
       function(patterns)
       {
         this.patterns = patterns;
         callback();
       }.bind(this)
     );
   },
 
   apply: function()
   {
-    if (patterns.length > 0)
+    if (this.patterns.length > 0)
     {
+      var document = this.window.document;
       this.addSelectors(document.styleSheets);
-      document.addEventListener("load", this.onLoad, true);

[Wladimir Palant, 2015/10/23 18:36:36]

Are you sure that the "load" event is being triggered for stylesheets while the
page is still loading?

[Sebastian Noack, 2015/10/23 20:49:31]

Seems so. Tested using https://pastebin.mozilla.org/8850267 on Chrome 46 and
Firefox 38.

+      document.addEventListener("load", this.onLoad.bind(this), true);
     }
-  },
-
-  init: function()
-  {
-    this.load(this.apply.bind(this));
   }
 };
-
-CSSPropertyFilters.init();

[Wladimir Palant, 2015/10/23 18:36:37]

As I said earlier, CSSPropertyFilters should ideally be a class. So this should
be:

  if (typeof window == "object")
    new CSSPropertyFilters(window);

In non-window context (Firefox) the class will be instantiated by outside code.

[Sebastian Noack, 2015/10/23 20:49:31]

I've added the check. Do we have to use a constructor? It's not that passing
around instances seems to make sense. But maybe I miss something.

[Wladimir Palant, 2015/10/23 21:04:40]

In Firefox there will be more than one window/document handled in the same
scope. So - yes, having one instance per window should be the simplest way.

[Sebastian Noack, 2015/10/23 21:23:22]

Gotya.