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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 /** @module cssInjection */
 
 "use strict";
 
 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
   // 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
@@ skipping 23 matching lines @@
     code: styleSheet,
     cssOrigin: "user",
     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
   });
 }
 
 function updateFrameStyles(tabId, frameId, selectors, groupName)
 {
@@ skipping 50 matching lines @@
       emulatedPatterns.push({selector: filter.selector, text: filter.text});
   }
 
   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);
 });