Issue 5090 - Use user stylesheets for element hiding if possible

lib/elemHideHelper.js

Index: lib/elemHideHelper.js
diff --git a/background.js b/lib/elemHideHelper.js
similarity index 54%
copy from background.js
copy to lib/elemHideHelper.js
index cb7a861be117f6a390b74ffa1370bb9de50ba33f..1e3c712a5663994281335537e0834ca188581f92 100644
--- a/background.js
+++ b/lib/elemHideHelper.js
@@ -15,6 +15,8 @@
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.

[Sebastian Noack, 2017/05/24 08:31:41]

This file is new. So how does it come that there are lines in this patch that
are not added, but already seem to exist? It looks like this patch has been
created on top of a previous iteration of this review. Please always upload
combined patches with all changes squashed. Otherwise, it's quite difficult to
review all changes together.

[Manish Jethani, 2017/05/24 17:13:34]

This is probably upload.py messing up. I'm trying hg review now.

[Sebastian Noack, 2017/05/24 18:14:05]

Well, that is what usually happens if you commit your changes for a new patch on
top, but don't specify a revision range to upload.py that includes all changes
related to this review. Are you sure, that's not what happened?

[Manish Jethani, 2017/05/25 02:52:25]

That may have been the case, I get your point. hg review has been working well
for me so far.

  */
 
+/** @module elemHideHelper */
+
 "use strict";
 
 const {RegExpFilter} = require("filterClasses");
@@ -24,6 +26,36 @@ const {extractHostFromFrame} = require("url");
 const {port} = require("messaging");
 const devtools = require("devtools");
 
+let userStylesheetsSupported = true;
+
+function hideElements(tabId, frameId, selectors, callback)

[Sebastian Noack, 2017/05/24 08:31:41]

Perhaps we should make this function return a promise rather than taking a
callback, in particular considering that the calling code wraps it into a
proomise anyway?

[Manish Jethani, 2017/05/24 17:13:34]

Done.

+{
+  let code = selectors.join(", ") + "{display: none !important;}";
+
+  try
+  {
+    chrome.tabs.insertCSS(tabId,
+      {
+        code,
+        cssOrigin: "user",
+        frameId,
+        matchAboutBlank: true
+      },
+      () =>
+      {
+        callback(chrome.runtime.lastError);
+      }
+    );
+  }
+  catch (error)
+  {
+    if (/\bError processing cssOrigin\b/.test(error.message) != -1)
+      userStylesheetsSupported = false;
+
+    callback(error);

[Manish Jethani, 2017/05/18 02:25:23]

Doing this for any errors instead of just the one for unsupported cssOrigin. We
could also throw here if it's not an error we're interested in.

[Sebastian Noack, 2017/05/24 08:31:41]

Yeah, we definitely should make sure that any unexpected error will be noticed
during development and testing.

[Manish Jethani, 2017/05/24 17:13:34]

Done.

+  }
+}
+
 port.on("get-selectors", (msg, sender) =>
 {
   let selectors;
@@ -45,22 +77,41 @@ port.on("get-selectors", (msg, sender) =>
     selectors = [];
   }
 
-  return {selectors, trace};
+  if (!userStylesheetsSupported)
+    return {selectors, trace, inject: true};
+
+  if (selectors.length == 0)
+  {
+    if (trace)
+      return {selectors, trace, inject: false};
+
+    return {trace, inject: false};
+  }
+
+  return new Promise(resolve =>
+  {
+    hideElements(sender.page.id, sender.frame.id, selectors, error =>
+    {
+      let response = {trace, inject: !!error};
+
+      if (trace || error)
+        response.selectors = selectors;
+
+      resolve(response);
+    });
+  });
 });
 
-port.on("forward", (msg, sender) =>
+port.on("hide-elements", (msg, sender) =>
 {
-  let targetPage;
-  if (msg.targetPageId)
-    targetPage = ext.getPage(msg.targetPageId);
-  else
-    targetPage = sender.page;
+  if (!msg.selectors || msg.selectors.length == 0)
+    return {success: true};
 
-  if (targetPage)
+  return new Promise(resolve =>
   {
-    msg.payload.sender = sender.page.id;
-    if (msg.expectsResponse)
-      return new Promise(targetPage.sendMessage.bind(targetPage, msg.payload));
-    targetPage.sendMessage(msg.payload);
-  }
+    hideElements(sender.page.id, sender.frame.id, msg.selectors, error =>
+    {
+      resolve({success: !error});
+    });
+  });
 });