1925747 - Calling compose.onBeforeSend might prevent sending

Status: VERIFIED FIXED

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

Description

[Günter Gersdorf]

Attachment: Minimal add-on

Steps to reproduce:

A customer of one of my add-ons (CopySend2Current) who uses Fedora 40 Wayland reported that the add-on prevents sending a message. Selecting File->Send Now or clicking the send button does nothing.

I found out that i could reproduce this behavior on Windows if i have the filter rules editor dialog open before opening the compose window. To investigate this further i wrote a minimal add-on (see attachment) which does nothing more than calling compose.onBeforeSend in the background script:

messenger.compose.onBeforeSend.addListener(async (tab, details) => {
return { cancel: false };
});

And that was enough to trigger the problem on windows (but not on Fedora).
This problem started to happen on 128b3, 128b2 didn't show this problem.

Actual results:

Could not send message

Expected results:

Message should be send

Comment 1

[John Bieling]

You reported this against Thunderbird 128 Beta 3 (128b3), can you reproduce this on 128.3.2 ESR?

I am on 128.3.2 on WIndows and cannot reproduce the error. I load your add-on, open the Filter dialog (Tools -> Messagefilters) and then open a new compose window and send myself a message. It sends without issues.

What else do I need to do in order to trigger this on Windows?

Do you want to report this as two individual bugs? Your observations on Windows and the other one on Wayland?

Comment 2

[John Bieling]

I also could not reproduce this on Fedora 40 using Wayland and Thunderbird 128.2.0 ESR.

Comment 3

[John Bieling]

Oh wow, if you actually edit a filter, I can reproduce it on Windows and Wayland. That is a hell of a bug.

Comment 4

[Günter Gersdorf]

Thanks ;-)

Comment 5

[John Bieling]

What you observe is the caused by a race condition in the enigmail/openpgp startup code. This listener is registered too late sometimes and then does not call this.

You can observe this without involving any Webxtension API. Open a compose window and then execute this in a console:

Services.wm.getMostRecentWindow("msgcompose").composeEditorReady

It will evaluate to true. If you however open the Filter Dialog and edit or create a filter rule, then open a new compose window and then execute the above command, it will evaluate to undefined.

Edit: It seems one can trigger this with any modal window.

Comment 6

[Günter Gersdorf]

While investigating for this bug report i also found out that composeEditorReady was not set, but did not realized that this is a race condition.

Comment 7

[Kai Engert [:KaiE:]]

John, thank you very much for your analysis !

Was this problem introduced by bug 1710787 ?
https://hg.mozilla.org/comm-central/rev/e889720db408

Before that change, the flag was set to true by the NotifyComposeBodyReady() function in MsgComposeCommands.js
The above change moved that to the OpenPGP code.

Comment 8

[Kai Engert [:KaiE:]]

See the comment here:
https://searchfox.org/comm-central/source/mail/extensions/openpgp/content/ui/enigmailMsgComposeOverlay.js#2267

As I understand it, the compose-editor-ready event was postponed to a later time, because the Add-on code required a notification at a later time.
However, if that event is postponed too much, we get other problems (this bug).

Would it work to move the assignment (and posting of the notification) back to the original place, and rather introduce a new event for the Add-on code?

Comment 9

[John Bieling]

When the Enigmail code was pushed into our code base, the NotifyComposeBodyReady() function in MsgComposeCommands.js was no longer the final piece of executed code, and setting the flag to true there was no longer correct.

We need to have a valid done flag. The current code (independent of WebExtension API) does not allow that anymore. The issue was introduced by the Enigmail code.

Comment 10

[Kai Engert [:KaiE:]]

I see the following:

Enigmail.msg composeStartup() checks that gMsgCompose is already set.
Usually (no dialog open), when we arrive here, gMsgCompose was already set, and everything works.

In the scenario described here, the order of events is different.
gMsgCompose is not yet set, and all the init code here is skipped.

(This might have to do with the additional global event loop, which runs when a modal dialog is visible.)

Comment 11

[Kai Engert [:KaiE:]]

We need a solution that delays the call to the enig composeStartup() function until after the ComposeStartup() from MsgComposeCommands has completed.

Currently, the enig composeStartup() code is triggered on the Windows load event, but that cannot be used any longer.
(This approach was used when Enigmail was still an add-on, and had no other way to hook itself into the startup process.)

Maybe we can simply introduce a new event for triggering the enig composeStartup(), which is dispatched when ComposeStartup() from MsgComposeCommands is complete.

Comment 12

[Kai Engert [:KaiE:]]

Attachment: Bug 1925747 - Ensure OpenPGP init code in composer is delayed until main init is ready. r=mkmelin,john.bieling

Comment 13

[Kai Engert [:KaiE:]]

try builds seem to work?
c-c: https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=570cebfe86ed9608dc04b706ed197a14a1b9fb3d
128: https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=c4ae268a628a72e043053813d375384fa66575e4

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/fcbf82ab82ee
Ensure OpenPGP init code in composer is delayed until main init is ready. r=mkmelin,john.bieling

Comment 15

[Kai Engert [:KaiE:]]

Comment on attachment 9433958 [details]
Bug 1925747 - Ensure OpenPGP init code in composer is delayed until main init is ready. r=mkmelin,john.bieling

[Approval Request Comment]
Regression caused by (bug #): not sure what introduced it
User impact if declined: some add-ons not working
Testing completed (on c-c, etc.): manually by John
Risk to taking this patch (and alternatives if risky): I wonder if there's a theoretical risk that the new approach could skip the OpenPGP init code in edge case scenarios in the email composer window. But we believe it should work fine.

Comment 16

[Corey Bryant]

Comment on attachment 9433958 [details]
Bug 1925747 - Ensure OpenPGP init code in composer is delayed until main init is ready. r=mkmelin,john.bieling

[Triage Comment]
Approved for beta

Comment 17

[Corey Bryant]

Thunderbird 133.0b3:
https://hg.mozilla.org/releases/comm-beta/rev/66736f6f0ab1

Comment 18

[Corey Bryant]

Comment on attachment 9433958 [details]
Bug 1925747 - Ensure OpenPGP init code in composer is delayed until main init is ready. r=mkmelin,john.bieling

[Triage Comment]
Approved for esr128

Comment 19

[Daniel Darnell [:dandarnell]]

Thunderbird 128.4.3esr:
https://hg.mozilla.org/releases/comm-esr128/rev/405fc9da0026

Comment 20

[Ramona Gavrilescu]

Confirming this issue as verified fixed on 128.4.3esr(20241108210256) using Win 10.