Issue 242 - Use user style sheets on Chromium

lib/cssInjection.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 10 matching lines @@
 
 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) ||
-                                 (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).

+// 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 = info.platform == "gecko";
 
 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
   // calculate the sizes of the selectors and divide them up accordingly, but
@@ skipping 11 matching lines @@
     yield selectorGroup.join(", ") + " {display: none !important;}";
 }
 
 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)
+  {
+    userStyleSheetsSupported = false;
+  }
+
+  return userStyleSheetsSupported;
 }
 
 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
   });
 }
@@ skipping 10 matching lines @@
 
   let oldStyleSheet = frame.injectedStyleSheets.get(groupName);
 
   // 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;
 
   if (!checkWhitelisted(sender.page, sender.frame,
                         RegExpFilter.typeMap.DOCUMENT |
                         RegExpFilter.typeMap.ELEMHIDE))
   {
     let hostname = extractHostFromFrame(sender.frame);
     let specificOnly = checkWhitelisted(sender.page, sender.frame,
                                         RegExpFilter.typeMap.GENERICHIDE);
 
     selectors = ElemHide.getSelectorsForDomain(
       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);
+  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 emulated filters as described
-  // in issue #5864. Instead, we can just ask the content script to add styles
-  // for emulated filters inline.
+  // 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);
 });