1674932 - messageDisplayScripts on TB 78.4: content script initially fails to run in main 3-pane window

Status: RESOLVED FIXED

(Thunderbird :: Add-Ons: Extensions API, defect)

Description

Bug

The messageDisplayScripts display API was introduced in Thunderbird 82 and then backported to ESR 78.4, see bug 1504475.

On Thunderbird 82, the API works fine.

On Thunderbird 78.4, registering a content script with messageDisplayScripts.register() does not always behave as expected. Let's consider the minimal reproducer add-on below.

Reproducer

manifest.json

{
  "manifest_version": 2,
  "applications": {
    "gecko": {
      "id": "[e-mail]",
      "strict_min_version": "78.4.0"
    }
  },
  "name": "test",
  "version": "1.0.0",
  "permissions": [
    "messagesModify"
  ],
  "background": {
    "scripts": [
      "background.js"
    ]
  }
}

background.js

browser.messageDisplayScripts.register({
    js: [{
        file: "content.js",
    }]
});

content.js

console.info("Test content script executed");

Expected behaviour

This should log the string to the console each time a message is displayed in Thunderbird.

Actual behaviour

After this add-on has been installed on Thunderbird 78.4, we can observe that:

• The string is not logged to the console when displaying messages in the main 3-pane window. No error is printed.
• When opening messages in a new window, by right-clicking either on the email account (left panel) or on a message in the list, then the add-on works in the new window and the string is logged when messages are displayed in this new window. For the old window, observations may differ: I observed that the add-on is still not working, but that displaying messages would now log an error (uncaught exception: Object) instead of remaining silent. Another user reported that the content script would also work in the first window after opening a new one.
• When changing the layout (classic/wide/vertical) of the main window, then the add-on works (and keeps working if switching back to the initial layout). But this manipulation needs to be done each time Thunderbird is restarted.
• Under certain conditions (Not sure which ones, this has been observed with a more complex add-on), the add-on produces a slightly more verbose error message: uncaught exception: Object 2 ext-extensionScripts.js:161:12.

This was tested on fresh profiles, on Linux and MacOS.

Remarks

To me it looks like something in the main 3-pane window is not initialised as it should be, and this would cause the content script injection to break. Changing the window layout apparently fixes that.

The issue is not present on Thunderbird 82, so there is a good chance this bug has been fixed already, but it may be desirable to identify the fix and to backport it to ESR 78.4+ for the messageDisplayScripts to work as expected.

This issue was reported on the GitHub repository for an add-on, here and there.

Comment 1

[juraj.masiar]

This has been reported also to my Darko addon.
Same problem, when using Layout / Vertical view, the messageDisplayScripts API won't work.
But yes, it works in current Beta and Alpha, so it would be nice to know if it's coming in 78.6 version.
Thanks!

Comment 2

[Geoff Lankow (:darktrojan)]

Attachment: 1674932-extension-browser-inserted-ESR78-1.diff

I almost forgot this patch existed.

This extension-browser-inserted event is happening too early and breaking a later call in tabmail.openFirstTab. It's only happening on 78 because of other patches we've only partially uplifted. In later versions the code I'm changing doesn't exist.

[Approval Request Comment]
Regression caused by (bug #): Not sure, some combination of earlier uplifts.
User impact if declined: Extension scripts don't run as expected.
Testing completed (on c-c, etc.): No, this is for 78 only. Here's a Try run of it not that that really helps other than prove it doesn't break the tests.
Risk to taking this patch (and alternatives if risky): This could only break extensions, but it's intended to fix them so I've checked it doesn't.

Comment 3

[Magnus Melin [:mkmelin]]

Comment on attachment 9191772 [details] [diff] [review]
1674932-extension-browser-inserted-ESR78-1.diff

Review of attachment 9191772 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/base/content/msgMail3PaneWindow.js
@@ +363,5 @@
>      // re-insert the messagePaneBoxWrapper back into the document.  But the dtor
>      // doesn't fire when the element is removed from the document.  Manually
>      // call destroy here to avoid a nasty leak.
> +    let messagePane = document.getElementById("messagepane");
> +    messagePane.destroy();

Seems a bit odd to destroy, and then reference it later.

Comment 4

[Wayne Mery (:wsmwk)]

Comment on attachment 9191772 [details] [diff] [review]
1674932-extension-browser-inserted-ESR78-1.diff

[Triage Comment]
Approved for esr78

Comment 5

[Rob Lemley [:rjl]]

Thunderbird 78.6.0:
https://hg.mozilla.org/releases/comm-esr78/rev/a9caeeb0d3bd

Comment 6

[zephyr]

I would like to reopen this issue as it seems the patch does not work. I am working on add-on using the messageDisplayScripts display API.

I am able to reproduce the issue with Thunderbird 78.10.0 on Linux, Mac OS and Windows.
On the beta and alpha channel there is no issue.

As a user point it out, it seems to be race condition with the startup cache. At the first startup no issue but on the subsequent starts it is not working.

It would be nice if to know a patch is possible in the next version 78.X, or if I should just wait for the version 91 to release

Thanks!

Comment 7

[Mark Banner (:standard8)]

(In reply to zephyr from comment #6)

I would like to reopen this issue as it seems the patch does not work. I am working on add-on using the messageDisplayScripts display API.

Please file a new bug rather than re-opening. Re-opening already landed bugs makes it difficult to track.

Comment 8

[juraj.masiar]

@zephyr please add here link to the new bug. I would like to follow it as some of my users were complaining about this as well. Thank you!

Comment 9

[zephyr]

(In reply to juraj.masiar from comment #8)

@zephyr please add here link to the new bug. I would like to follow it as some of my users were complaining about this as well. Thank you!

For anyone willing to follow the new bug 1707573