Issue 242 - Use user style sheets on Chromium

lib/cssInjection.js

Index: lib/cssInjection.js
===================================================================
--- a/lib/cssInjection.js
+++ b/lib/cssInjection.js
@@ -21,19 +21,25 @@
 
 const {RegExpFilter} = require("filterClasses");
 const {ElemHide} = require("elemHide");
 const {ElemHideEmulation} = require("elemHideEmulation");
 const {checkWhitelisted} = require("whitelisting");
 const {extractHostFromFrame} = require("url");
 const {port} = require("messaging");
 const devtools = require("devtools");
+const info = require("info");
 
-const userStyleSheetsSupported = "extensionTypes" in browser &&
-                                 "CSSOrigin" in browser.extensionTypes;
+const userStyleSheetsSupported = ("extensionTypes" in browser &&
+                                  "CSSOrigin" in browser.extensionTypes) ||
+                                 (info.platform == "chromium" &&
+                                  parseInt(info.platformVersion, 10) >= 66);

[Sebastian Noack, 2018/01/31 11:23:37]

Perhaps, we should consider going back to the try-and-error approach to detect
user stylesheets, rather than relying on the user agent string.

[Manish Jethani, 2018/01/31 12:36:27]

What if Edge adds support for cssOrigin but behaves differently than Chrome and
Firefox, or throws a different error (we were checking for the error string)?

Anyway since Edge is still off a separate branch (right?) it's probably OK to go
back to that approach now.

+const styleSheetRemovalSupported = "removeCSS" in browser.tabs &&
+                                   info.platform != "chromium";

[kzar, 2018/01/31 11:11:32]

IMO we only know removeCSS works on Gecko and so we are disabling it for the
other platforms. I think it would be better to check that the platform is Gecko
here, rather than that it's not Chromium.

[Sebastian Noack, 2018/01/31 11:23:37]

Why not just checking for presence of "removeCSS"? It doesn't exist on Chrome,
and once they add it, it surely would be great to use it.

[kzar, 2018/01/31 11:25:29]

Well as we discussed in IRC don't we agree it would be better to first have QA
test the feature works OK on Chromium before switching it on for our users?

[Sebastian Noack, 2018/01/31 11:54:44]

Well, we were talking about cssOrigin, which is now available in Canary. Whether
we should generally prefer browser detection over feature detection, is a
different question, and seems quite radical to me just because it backfired
once. On the other hand, how do we make sure not to forget to turn such features
on, and even if we do, there will be some delay until we get a release out. Also
FWIW removing the CSS seems to less likely break something than inserting it.

[kzar, 2018/01/31 11:57:56]

Fair enough, if you both agree to do it that way I won't block it.

[Manish Jethani, 2018/01/31 12:36:27]

Makes sense.

Done.

[Manish Jethani, 2018/01/31 12:36:27]

With tabs.removeCSS specifically we're thinking of having a different API on
Chrome, and we haven't decided how it'll work exactly.

See this comment and the next:

https://bugs.chromium.org/p/chromium/issues/detail?id=608854#c26

So even if it's there it'll probably work differently.

[Manish Jethani, 2018/01/31 12:36:27]

See my other comment. In the case of tabs.removeCSS we _know_ it will break ABP
once Canary has it (if we go with feature detection).

+
 const selectorGroupSize = 1024;
 
 function* splitSelectors(selectors)
 {
   // Chromium's Blink engine supports only up to 8,192 simple selectors, and
   // even fewer compound selectors, in a rule. The exact number of selectors
   // that would work depends on their sizes (e.g. "#foo .bar" has a size of 2).
   // Since we don't know the sizes of the selectors here, we simply split them
@@ -67,16 +73,23 @@
     frameId,
     matchAboutBlank: true,
     runAt: "document_start"
   });
 }
 
 function removeStyleSheet(tabId, frameId, styleSheet)
 {
+  // Chromium's support for tabs.removeCSS is still a work in progress and the

[kzar, 2018/01/31 11:11:32]

IMO this comment should go by the styleSheetRemovalSupported definition.

[Manish Jethani, 2018/01/31 12:36:27]

Done.

+  // API is likely to be different from Firefox's; for now we just don't use it
+  // at all, even if it's available.
+  // See https://crbug.com/608854
+  if (!styleSheetRemovalSupported)
+    return;
+
   browser.tabs.removeCSS(tabId, {
     code: styleSheet,
     cssOrigin: "user",
     frameId,
     matchAboutBlank: true
   });
 }
 
@@ -137,16 +150,23 @@
 
   if (!inline)
     updateFrameStyles(sender.page.id, sender.frame.id, selectors);
 
   let response = {trace, inline, emulatedPatterns};
   if (trace || inline)
     response.selectors = selectors;
 
+  // If we can't remove user style sheets using tabs.removeCSS, we'll only keep
+  // adding them, which could cause problems with emulated filters as described
+  // in issue #5864. Instead, we can just ask the content script to add styles
+  // for emulated filters inline.
+  if (!styleSheetRemovalSupported)
+    response.inlineEmulated = true;
+
   return response;
 });
 
 port.on("elemhide.injectSelectors", (message, sender) =>
 {
   updateFrameStyles(sender.page.id, sender.frame.id, message.selectors,
                     message.groupName);
 });