Issue 242 - Use user style sheets on Chromium

lib/cssInjection.js

Index: lib/cssInjection.js
===================================================================
--- a/lib/cssInjection.js
+++ b/lib/cssInjection.js
@@ -21,21 +21,29 @@
 
 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;
+// Chromium's support for tabs.removeCSS is still a work in progress and the
+// 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
+const styleSheetRemovalSupported = "removeCSS" in browser.tabs &&
+                                   info.platform == "gecko";

[Sebastian Noack, 2018/01/31 14:30:05]

tabs.removeCSS() was added in Firefox 49. The oldest version we still support is
Firefox 51. So the check for info.platform should be sufficient.

[Manish Jethani, 2018/02/01 08:05:07]

Done.

+
 const selectorGroupSize = 1024;
 
+let userStyleSheetsSupported = true;
+
 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
   // into groups of 1,024, based on the reasonable assumption that the average
   // selector won't have a size greater than 8. The alternative would be to
@@ -56,27 +64,42 @@
 
 function createStyleSheet(selectors)
 {
   return Array.from(createRules(selectors)).join("\n");
 }
 
 function addStyleSheet(tabId, frameId, styleSheet)
 {
-  browser.tabs.insertCSS(tabId, {
-    code: styleSheet,
-    cssOrigin: "user",
-    frameId,
-    matchAboutBlank: true,
-    runAt: "document_start"
-  });
+  try
+  {
+    browser.tabs.insertCSS(tabId, {
+      code: styleSheet,
+      cssOrigin: "user",
+      frameId,
+      matchAboutBlank: true,
+      runAt: "document_start"
+    });
+  }
+  catch (error)
+  {
+    if (/\bError processing cssOrigin\b/.test(error.message) == -1)

[Sebastian Noack, 2018/01/31 14:30:05]

I wonder how safe the assumption is that this will match any error indicating
unsupported cssOrigin on all versions of Chromium and Firefox. On the other
hand, without this check we would just fall back to content script injected
author style sheets, as well in the theoretical scenario where insertCSS() might
fail for another reason. That doesn't seem too bad to me.

[Manish Jethani, 2018/02/01 08:05:07]

OK, I think we should just look for /\bcssOrigin\b/ here. We already know it's
an error, then if we know the error message contains the word "cssOrigin", and
since we've passed the value "user" to that option, we can conclude that user
style sheets are not supported. This is more general than the previous check.

Done.

[Sebastian Noack, 2018/02/01 10:32:50]

I still think, we should just remove that check all together. But it's not too
important. Let's see what Dave says.

[Manish Jethani, 2018/02/01 10:40:38]

Yeah I'm cool with just removing the check too. If it fails, it fails, then the
reason for failure doesn't matter. We lose the actual error but that's OK.

+      throw error;
+
+    userStyleSheetsSupported = false;
+  }
+
+  return userStyleSheetsSupported;
 }
 
 function removeStyleSheet(tabId, frameId, styleSheet)
 {
+  if (!styleSheetRemovalSupported)
+    return;
+
   browser.tabs.removeCSS(tabId, {
     code: styleSheet,
     cssOrigin: "user",
     frameId,
     matchAboutBlank: true
   });
 }
 
@@ -94,26 +117,27 @@
 
   // Ideally we would compare the old and new style sheets and skip this code
   // if they're the same, but the old style sheet can be a leftover from a
   // previous instance of the frame. We must add the new style sheet
   // regardless.
 
   // Add the new style sheet first to keep previously hidden elements from
   // reappearing momentarily.
-  if (styleSheet)
-    addStyleSheet(tabId, frameId, styleSheet);
+  if (styleSheet && !addStyleSheet(tabId, frameId, styleSheet))
+    return false;
 
   // Sometimes the old and new style sheets can be exactly the same. In such a
   // case, do not remove the "old" style sheet, because it is in fact the new
   // style sheet now.
   if (oldStyleSheet && oldStyleSheet != styleSheet)
     removeStyleSheet(tabId, frameId, oldStyleSheet);
 
   frame.injectedStyleSheets.set(groupName, styleSheet);
+  return true;
 }
 
 port.on("elemhide.getSelectors", (message, sender) =>
 {
   let selectors = [];
   let emulatedPatterns = [];
   let trace = devtools && devtools.hasPanel(sender.page);
   let inline = !userStyleSheetsSupported;
@@ -130,23 +154,33 @@
       hostname,
       specificOnly ? ElemHide.SPECIFIC_ONLY : ElemHide.ALL_MATCHING
     );
 
     for (let filter of ElemHideEmulation.getRulesForDomain(hostname))
       emulatedPatterns.push({selector: filter.selector, text: filter.text});
   }
 
-  if (!inline)
-    updateFrameStyles(sender.page.id, sender.frame.id, selectors, "standard");
+  if (!inline && !updateFrameStyles(sender.page.id, sender.frame.id,
+                                    selectors, "standard"))
+  {
+    inline = true;
+  }
 
   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 emulation filters as
+  // described in issue #5864. Instead, we can just ask the content script to
+  // add styles for emulation 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);
 });