1702957 - Recipients not restored when re-opening/editing a draft message [Add-on API: set/getComposeDetails]

Status: RESOLVED FIXED

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

Description

[Achim Seufert]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:87.0) Gecko/20100101 Firefox/87.0

Steps to reproduce:

As reported by a SignatureSwitch-user:
https://github.com/4ch1m/mozext/issues/43

• open a new mail
• enter recipient (, subject, text)
• save as draft
• re-edit the draft

Actual results:

The previously entered recipient from the draft doesn't get restored.

Expected results:

The previously entered recipient from the draft should get restored.

This is probably yet another side-effect of the recently introduced pillification (see bug 1696765).

Comment 2

[Thomas D. (:thomas8)]

Alfred, first of all, big thank you for looking into bugs!
In cases like this sometimes better to forward-duplicate. This bug was filed only 2 days after bug 1702852, but this one has much clearer steps and starts out with the right hints, namely the involvement of SignatureSwitch Add-on. Moreover, it was filed by the addon-author himself, claiming that it's a bug in Thunderbird. We'll look into that in the next comments.

Comment 4

[Thomas D. (:thomas8)]

Hi Achim,

sorry for nagging, but using your STR on 89.0b3, this bug...

• ... does not occur without your SignatureSwitch Add-on
• ... does occur with SignatureSwitch Add-on

So it looks like bug caused by add-on, no?
I'm failing to understand then why you seem to suggest that this is a bug in Thunderbird?
Is Thunderbird serving you wrong data? Any details why your addon would touch regular recipient headers?
Maybe I'm overlooking something.

(In reply to Achim Seufert from duplicate Bug 1702852 Comment 7)

(In reply to Thomas D. (:thomas8) from comment #6)

We have seen somewhat similar problems with SignatureSwitch add-on in Bug 1696765.
Achim (addon-author), could you comment?

Sure.

As John already stated:

Yes, requested by add-on developers getComposeDetails should contain partially entered values, which needed an enforced pillification. Hm. Achim is not doing something illegal. How do we solve this? Lets move this discussion over to bug 1681008.

Achim, could you reference the comment with John's quote? Context will be helpful.

I understand the need for getComposeDetails to return partially entered / non-pillified recipient values in some way, but afasics this bug does not involve those, so I don't see how John's comment wrt partial values could apply here. Furthermore, Bug 1696765 seems to have been mitigated somehow, so even with the addon installed, it does no longer happen for me.

The add-on is using pure WebExtension/MailExtension APIs... no experiments or other "hacky code" (imho).

The sourcecode can be found here:
https://github.com/4ch1m/mozext/tree/master/signatureswitch-me

I haven't yet found the time to investigate which parts of the base code are broken or lead to the described behaviour.

Comment 5

[Thomas D. (:thomas8)]

^^ John, fyi: comment 4.

Comment 6

[John Bieling (:TbSync)]

Found it.

Achim has triggered "something" which I was coincidently looking at this morning by myself.

Thanks for finding this, sorry for it being there in the first place!

To quick-fix this in the add-on, add a small delay here:

function addWindowCreateListener() {
    browser.windows.onCreated.addListener(window => {
        if (window.type === WINDOW_TYPE_MESSAGE_COMPOSE) {
            browser.tabs.query({windowId: window.id}).then(async tabs => {
                let tabId = tabs[0].id;

                let storage = await browser.storage.local.get();
                await new Promise(resolve => window.setTimeout(resolve, 500));  // <-- wait here
                let details = await browser.compose.getComposeDetails(tabId);

getComposeDetails is called to early (the window is ready after onCreated has fired, but not the composer). I keep digging.

Comment 7

[John Bieling (:TbSync)]

I have a fix. Will create bug and patch later today.

Edit: Fix needs some more time. It is complicated.

Comment 8

[Achim Seufert]

(In reply to John Bieling (:TbSync) from comment #6)

Found it.

Nice. [thumbsup]

Achim has triggered "something" which I was coincidently looking at this morning by myself.

Thanks for finding this, sorry for it being there in the first place!

Seems that I'm getting infamous or at least I'm about to start buildung a certain reputation with all those bug-titles claiming that SignatureSwitch breaks Thunderbird. ;-)

To quick-fix this in the add-on, add a small delay here:

function addWindowCreateListener() {
    browser.windows.onCreated.addListener(window => {
        if (window.type === WINDOW_TYPE_MESSAGE_COMPOSE) {
            browser.tabs.query({windowId: window.id}).then(async tabs => {
                let tabId = tabs[0].id;

                let storage = await browser.storage.local.get();
                await new Promise(resolve => window.setTimeout(resolve, 500));  // <-- wait here
                let details = await browser.compose.getComposeDetails(tabId);

Thanks for taking the time to provide a workaround.
However, this doesn't reliably solve the issues on my end.
(Even with this fix in place, it works only 8 out of 10 times. Maybe 500ms is not enough in some cases?!)

What's even more confusing:
It always works then opening a draft directly after starting Thunderbird.
In other words: If it is the very first time a composer-window is opened; then it will work.
Only after subsequent openings of a compose-window/draft it is broken.
Hmmm.

Comment 9

[John Bieling (:TbSync)]

Attachment: Bug 1702957 - Wait for compose window being ready before using set/getComposeDetails. r=mkmelin

Comment 10

[John Bieling (:TbSync)]

Comment on attachment 9221393 [details]
Bug 1702957 - Wait for compose window being ready before using set/getComposeDetails. r=mkmelin

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined:
API clears recipient field of re-opened drafts, which is a serious/annoying bug.

Testing completed (on c-c, etc.):
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f4f6e7f75116908053e3237b7ee72e3fde3a65c5

Risk to taking this patch (and alternatives if risky):
Low.

Comment 11

[Wayne Mery (:wsmwk)]

Comment on attachment 9221393 [details]
Bug 1702957 - Wait for compose window being ready before using set/getComposeDetails. r=mkmelin

[Triage Comment]
Approved for beta

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/cc70e22232d7
Wait for compose window being ready before using set/getComposeDetails. r=mkmelin

Comment 13

[Rob Lemley [:rjl]]

Thunderbird 89.0b4:
https://hg.mozilla.org/releases/comm-beta/rev/d6e1f2eea747

Comment 14

[John Bieling (:TbSync)]

The current Daily includes the patch. Can you confirm it is fixed? Beta89.0b4 should be fixed as well.

Comment 15

[Thomas D. (:thomas8)]

(In reply to Achim Seufert from comment #8)

(In reply to John Bieling (:TbSync) from comment #6)

Achim has triggered "something" which I was coincidently looking at this morning by myself.
Thanks for finding this, sorry for it being there in the first place!
Seems that I'm getting infamous or at least I'm about to start building a certain reputation with all those bug-titles claiming that SignatureSwitch breaks Thunderbird. ;-)

Sorry Achim, my mistake, no offense meant - I have removed the erroneous blame from the summary of this bug 1702957 and duplicate bug 1702852 - that's it, right? I've found this set of bugs pretty confusing, but with your help and following my invitation of John (our add-on expert) in comment 5, at least we have been able to figure this out, thank you! I've learned something new here about bugs involving add-ons, that even though the bug only appears with add-ons, it might still be a Thunderbird around the API...

So FTR: SignatureSwitch is a highly popular add-on and it does NOT break Thunderbird! :-)