Issue 5090 - Use user stylesheets for element hiding if possible

lib/css.js

Index: lib/css.js
diff --git a/lib/css.js b/lib/css.js
new file mode 100644
index 0000000000000000000000000000000000000000..d266f833c8e6236f4f9718ff5af9084551dc0231
--- /dev/null
+++ b/lib/css.js
@@ -0,0 +1,60 @@
+/*
+ * This file is part of Adblock Plus <https://adblockplus.org/>,
+ * Copyright (C) 2006-2017 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 css */

[Sebastian Noack, 2017/04/29 22:11:03]

I feel "css" being a too generic name for what is done here. However, "elemHide"
is already taken. But we could still call it "elemHideHelper" which would be
consistent with "notificationHelper". Other suggestions are welcome.

[Manish Jethani, 2017/05/01 23:18:05]

Done.

+
+"use strict";
+
+const info = require("info");
+
+/**
+ * Whether user stylesheets are supported on this platform.
+ *
+ * @type {boolean}
+ * @static
+ */
+exports.userStylesheetsSupported = info.platform == "gecko" &&

[Sebastian Noack, 2017/04/29 22:11:04]

Again, please avoid statically binding logic to the environment, use feature
detection instead. Initially, in the issue tracker we specified it this way, any
reason you changed it? (This really should have been justified with a comment in
the issue when you changed it.)

[Manish Jethani, 2017/05/01 23:18:05]

I had used feature detection originally, but then Dave pointed out that we would
have to check the platform details anyway for Chrome, because they're not going
to support cssOrigin. I suppose we could still use feature detection, I'm not
sure it's worth it. It would not even be possible if Chrome starts to accept
cssOrigin but ignore its value, always inserting the CSS as an author stylesheet
until they decide to switch the default. I feel like checking for the platform
here is the safer option.

Yeah, I should have left a comment explaining the change. I'll bear this in mind
for next time.

[Wladimir Palant, 2017/05/17 11:50:16]

Given that the values provided by the info module are extracted from the user
agent and are hence inherently unreliable - yes, I think that it is still worth
it. And even if Chrome uses a different name, we could detect that one similarly
then.

+                                   info.platformVersion.split(".")[0] >= 53;
+
+/**
+ * Hides elements on the page using the browser.tabs.insertCSS API.
+ *
+ * @param {string} tabId       The ID of the tab in which to hide elements
+ * @param {string} frameId     The ID of the frame in which to hide elements
+ * @param {string[]} selectors The list of selectors for the elements to hide
+ * @param {Function} callback  The function to be called upon completion
+ * @static
+ */
+exports.hideElements = (tabId, frameId, selectors, callback) =>
+{
+  let code = selectors.length > 0 ?
+             selectors.join(", ") + "{display: none !important;}" :
+             "";

[Sebastian Noack, 2017/04/29 22:11:03]

Is this function under any circumstances called with an empty array for the
selectors? If so, we shouldn't call insertCSS() either, not just fall back to an
empty string.

[Manish Jethani, 2017/05/01 23:18:05]

Done.

+
+  chrome.tabs.insertCSS(tabId,
+    {
+      code,
+      cssOrigin: "user",
+      frameId,
+      matchAboutBlank: true
+    },
+    () =>
+    {
+      callback(chrome.runtime.lastError);
+    }
+  );
+};