Issue 5386 - Ignore messages from content scripts in devtools panel

ext/content.js

 "use strict";
 
-chrome.runtime.onMessage.addListener((message, sender, sendResponse) =>
+// Firefox 55 erroneously sends messages from the content script to the
+// devtools panel:
+// https://bugzilla.mozilla.org/show_bug.cgi?id=1383310
+// As a workaround, listen for messages only if this isn't the devtools panel.
+if (!chrome.devtools)
 {
-  return ext.onMessage._dispatch(message, {}, sendResponse).indexOf(true) != -1;
-});
+  // Listen for messages from the background page.

[Manish Jethani, 2017/07/25 12:52:28]

I've added this extra comment here to clarify what we're really listening for
here, since it wasn't obvious when I first looked at the code.

+  chrome.runtime.onMessage.addListener((message, sender, sendResponse) =>
+  {
+    return ext.onMessage._dispatch(message, {}, sendResponse).indexOf(true) !=
+           -1;
+  });

[Wladimir Palant, 2017/08/16 08:56:44]

I might be missing something here, but this way the devtools panel won't listen
to any messages whatsoever, right? Effectively, it's the same as not loading
this file in devtools-panel.html at all. But the devtools panel is actually
listening to messages from the background page like "add-record", it merely
doesn't want to receive messages from content scripts.

[Manish Jethani, 2017/08/16 09:18:54]

Yeah, so the devtools panel opens a persistent connection to the background page
(rather than listening for one-off messages) [1], and messages like "add-record"
come over that persistent connection. It's the only way for the devtools panel
to really communicate with the background page.

You can see that ext.onMessage is getting reassigned in ext/devtools.js

If it wasn't for this bug in Firefox, the runtime.onMessage handler would never
get called.

[1]:
https://hg.adblockplus.org/adblockpluschrome/file/552ac1922795/ext/devtools.j...

[Wladimir Palant, 2017/08/16 09:28:33]

I see. So why is devtools-panel.html loading ext/content.js in the first place?
It seems that not loading it will fix this issue as well.

[Manish Jethani, 2017/08/16 09:42:43]

I went down that route [1] but decided to back off because it would need more
changes, and it didn't seem worth the effort at the time. Mainly the test
environment for the devtools panel also needs ext/content.js, but the one in
adblockplusui instead [2], which initializes the background page in an iframe
and so on and sets up the messaging.

Ideally it would be fixed that way, along with any other inconsistencies found.
If you think it's worth going down that route, I can give it a shot.

[1]: https://codereview.adblockplus.org/29494581/
[2]: https://hg.adblockplus.org/adblockplusui/file/461ee88245c7/ext/content.js

[Wladimir Palant, 2017/08/16 10:05:19]

You are right, doesn't seem to be worth it right now.

+}
 
 (function()
 {
   let port = null;
 
   ext.onExtensionUnloaded = {
     addListener(listener)
     {
       if (!port)
         port = chrome.runtime.connect();
@@ skipping 10 matching lines @@
 
         if (!port.onDisconnect.hasListeners())
         {
           port.disconnect();
           port = null;
         }
       }
     }
   };
 }());