Issue 4298 - Enforce 'display: none' for ElemHide

include.preload.js

Index: include.preload.js
diff --git a/include.preload.js b/include.preload.js
index bb6b97a01c6d5cc7bff5dc9ab8ca8285880f89bc..b144053bbf71a29711ba1c5f052032a17d7895e4 100644
--- a/include.preload.js
+++ b/include.preload.js
@@ -334,6 +334,26 @@ ElementHidingTracer.prototype = {
   }
 };
 
+function enforceStyleSheetRules(style)
+{
+  style.id = id;
+  injectJS(
+    function(id)
+    {
+      var style = document.getElementById(id) ||
+                  document.documentElement.shadowRoot.getElementById(id);
+      style.removeAttribute("id");
+
+      Object.defineProperty(style.sheet, "disabled",

[Sebastian Noack, 2016/08/04 20:44:23]

Where did the code go, that addressed deleteRule and removeRule?

[kzar, 2016/08/05 11:01:57]

This review is based upon the changes in a couple of other reviews. They're
listed in this review's description. That stuff is somewhere near line 400 here,
you'll have to expand the code to see it. Equally if it's easier these changes
are in GitHub too:
https://github.com/kzar/adblockpluschrome/commits/4298-enforce-display-none

I would be happy to combine this review with the review for 4191 since they're
very similar and things are getting confusing. Any objections?

+                            {value: false, enumerable: true});
+
+      for (var i = 0; i < style.sheet.rules.length; i += 1)
+        Object.defineProperty(style.sheet.rules[i].style, "display",

[Sebastian Noack, 2016/08/04 20:44:23]

How about overriding style.sheet.rules (and sheet.cssRules) so that it always
returns an empty array? That way we wouldn't have to monkey-patch every single
rule which might cause performance issues.

[kzar, 2016/08/05 11:01:57]

Cool idea, but I actually had to overwrite the getter on
CSSStyleSheet.prototype. 

(It also turns out my old implementation only happened to work for the website
in question by luck. Setting a rule's style to read only doesn't do anything
apparently, it's still writable!)

+                              {value: "none", enumerable: true});
+    }, id
+  );
+}
+
 function reinjectStyleSheetWhenRemoved(document, style)
 {
   if (!MutationObserver)
@@ -350,19 +370,9 @@ function reinjectStyleSheetWhenRemoved(document, style)
       if (style.sheet.rules.length == 0)
       {
         for (var i = 0; i < rules.length; i += 1)
-          style.sheet.addRule(rules[i].selectorText, "display: none !important;");
+          style.sheet.addRule(rules[i].selectorText);
 
-        style.id = id;
-        injectJS(
-          function(id)
-          {
-            var style = document.getElementById(id) ||
-                        document.documentElement.shadowRoot.getElementById(id);
-            style.removeAttribute("id");
-            Object.defineProperty(style.sheet, "disabled",
-                                  {value: false, enumerable: true});
-          }, id
-        );
+        enforceStyleSheetRules(style);
       }
     }
   });
@@ -595,8 +605,9 @@ function init(document)
     for (var i = 0; i < selectors.length; i += SELECTOR_GROUP_SIZE)
     {
       var selector = selectors.slice(i, i + SELECTOR_GROUP_SIZE).join(", ");
-      style.sheet.addRule(selector, "display: none !important;");
+      style.sheet.addRule(selector);
     }
+    enforceStyleSheetRules(style);
   };
 
   var updateStylesheet = function()