1882013 - Thunderbird keeps adding mails to the Drafts folder for imap servers that don't support UIDPLUS

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[forum.news]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:123.0) Gecko/20100101 Firefox/123.0

Steps to reproduce:

Since the TB 115 release (or maybe even before that) we are facing the problem that Thunderbird keeps adding mails to the Drafts folder but it does not delete them anymore after sending the mail. With older releases, Thunderbird saved the mail every n minutes but after sending the mail, the "one" draft got deleted again. That was OK. Now Thunderbird saves a "new" mail every n minutes so they get more and more and after the final send, nothing gets deleted anymore. It almost feels like Thunderbird cannot identify the draft mail anymore so it keeps creating new ones while composing a mail. Not sure if this is related to our mail server or if this is a general issue but it is a 100% reproducible here.

FWIW: I believe this started with the new way of dealing with encrypted mails (which got added at some point to the Thunderbird core rather than adding it with the add-on "Enigmail"). Maybe TB cannot identify the draft mails anymore because they get stored in an encrypted way? Just a thought...

Comment 1

[gene smith]

Thanks for entering the report.
I've also been working with reporter on Bug 1856536 using the test account on his server that he provided.
Reporter's imap server (Mercury by Pegasus Mail) doesn't support the imap extension UIDPLUS. Without UIDPLUS when a draft is saved (actually imap APPEND'd) into the Drafts folder, there is no UID (message's unique identifier) returned in the APPEND response so nothing is saved so that messages can't be marked deleted on the next save. To handle this, the imap code tries to search the Drafts folder using the unique (*) "Message-ID" generated by TB when the message is created which it is supposed to find the message by UID, which is returned and saved. However, something has changed ahead of the imap code and the imap code can't get the Message-ID to do the search so the old message is never marked deleted on the next draft save and the old messages versions stay visible and accumulate in the Drafts folder.

This worked OK with TB 78 and 91 releases with the reporter's test account (with jsmodule switched off) but is broken at the last release version of 102. I did a "bisect" (using mozregression tool) and found exactly where it breaks with daily pushes, Aug 13, 2021, the first push that day: https://hg.mozilla.org/comm-central/pushloghtml?startdate=8-12-21&enddate=8-13-21.

I suspected this was not just a simple one line or typo error but caused by a major transition in functionality, i.e. from c++ to JS implementation in the "compose" and SMTP send code.

I have a interim fix but I need some help getting data from c++ to JS and then to c++. What I have works but one part of it is a major kluge.

I checked all my other imap accounts that I have set up on a lot of "well known" servers and they all support UIDPLUS capability, which explains why I never saw this (and I don't write a lot of emails). Possibly this has been reported before by someone using >=102?

*) Another issue is that all the saved drafts for a given message have the same message ID -- not unique. This may not be a problem if they were properly marked deleted before saving the next messages. But even at that, they should all be unique as they were prior to 102. My interim fix produces a unique message ID on each saved draft version.

Comment 2

[gene smith]

Attachment: WIP: Bug 1882013 - Fix non-UIDPLUS imap so previous draft versions are deleted. r=mkmelin

Comment 3

[gene smith]

Here's a try version with the patch from comment 2 plus the proposed fix for Bug 1856536:
https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/Y1mziIhCRn-KsT9KAZbBLw/runs/1/artifacts/public/build/install/sea/target.installer.exe

This applies both patches on top of the newest beta 124.0b2.
This should fix both problems. However, the patch at comment 2, at this time, is still "Work in Progress" (WIP).

Link to the full try build: https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=dd390c9cabffda652cb2942ff21eb569ec1e9478&selectedTaskRun=Y1mziIhCRn-KsT9KAZbBLw.1

Comment 4

[[e-mail]]

I need some help on this.

The implementation of getMessageId() should be:

getMessageId(messageID) {
  messageID.value = this._compFields.messageId;
}

to match void getMessageId(out ACString messageID);. Or change the IDL to ACString getMessageId(); and use:

getMessageId() {
  return this._compFields.messageId;
}

Comment 5

[gene smith]

Attachment: Bug 1882013 - Fix non-UIDPLUS imap so previous draft versions are deleted. r=mkmelin

Comment 6

[forum.news]

Thanks for providing the "test version". It seems to work as expected. Saving a mail as draft several times does not keep adding mails to the drafts folder anymore. Only one mail is shown (and overwritten) and after sending the mail, the one draft gets deleted.

Looking forward to the fix. To which release will this be added?

Konrad

Comment 7

[Wayne Mery (:wsmwk)]

Looking forward to the fix. To which release will this be added?

Maybe 115.9.0 in a few weeks.

Comment 8

[forum.news]

Amazing. Cant wait for this fix ;-)

Comment 9

[[e-mail]]

Please add || deliverMode == nsIMsgCompDeliverMode::SaveAsTemplate so templates are also superseded when being edited and saved.

Comment 10

[gene smith]

From comment 9:

... so templates are also superseded when being edited and saved.

Thanks! I added that to the patch and re-tested and now templates work the same as drafts.

Going ahead and requesting check-in without further review.

Comment 11

[gene smith]

Well, with templates, what I see is the original version only gets deleted on the first save after editing the template. Subsequent saves from the compose window produces a new template message without deleting the previously saved message from that same compose window.
So you only delete the original version on the first save when editing a template.
I see this with and without the change recommended with comment 9.
I also see the exact same behavior with 91.
Also, the template saves are not affected by the UIDPLUS issue and don't seem to be handled as a special case by the low level imap protocol code like draft saves are.
Anyhow, this may be the subject of another bug or (most likely) I just don't know exactly how template saves are supposed to work. It's something I've never had a need for or looked into in detail.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/5cf94f85ccf0
Fix non-UIDPLUS imap so previous draft versions are deleted. r=mkmelin

Comment 13

[Geoff Lankow (:darktrojan)]

FYI mailnews/imap/test/unit/test_saveImapDraft.js is failing after this patch landed.

Comment 14

[Magnus Melin [:mkmelin]]

Going to back out to green up the tree.

Comment 15

[Magnus Melin [:mkmelin]]

Backout: https://hg.mozilla.org/comm-central/rev/e354a06c2a3b3283aeb262ff62ab3dd2c2c09234

Comment 16

[gene smith]

Attachment: Bug 1882013 - Fix non-UIDPLUS imap. Follow up fix for test failure. r=mkmelin

Imapd.sys.mjs (fakeserver) SEARCH response was sending an additional
imap tag causing test_saveImapDraft.js to fail.

Depends on D202892

Comment 17

[gene smith]

Running a full try here: https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=2d273edcbc7d9b6275ac72261dbc0441a02d31ff

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/6e9d4725a084
Fix non-UIDPLUS imap so previous draft versions are deleted. r=mkmelin
https://hg.mozilla.org/comm-central/rev/6f571271ac91
Fix non-UIDPLUS imap. Follow up fix for test failure. r=mkmelin

Comment 19

[Wayne Mery (:wsmwk)]

Comment on attachment 9387963 [details]
Bug 1882013 - Fix non-UIDPLUS imap so previous draft versions are deleted. r=mkmelin

[Triage Comment]
approved for beta

taking because this is wanted for esr.

Comment 20

[Wayne Mery (:wsmwk)]

Comment on attachment 9388848 [details]
Bug 1882013 - Fix non-UIDPLUS imap. Follow up fix for test failure. r=mkmelin

[Triage Comment]
approved for beta

Comment 21

[Rob Lemley [:rjl]]

Thunderbird 124.0b5:
https://hg.mozilla.org/releases/comm-beta/rev/7dbafa69b58d
https://hg.mozilla.org/releases/comm-beta/rev/0b1dcefcb541

Comment 22

[forum.news]

Hi, does this last comment mean that this will be added to TB 124.x? I was hoping that this fix (and the one from #1856536) will be available for us in one of the next updates for TB 115.

Thanks
Konrad

Comment 23

[Wayne Mery (:wsmwk)]

(In reply to forum.news from comment #22)

Hi, does this last comment mean that this will be added to TB 124.x? I was hoping that this fix (and the one from #1856536) will be available for us in one of the next updates for TB 115.

It is in 124 beta.
Eventually in 115.

Comment 24

[forum.news]

Good to hear - thanks!

Comment 25

[Wayne Mery (:wsmwk)]

Drafts still behaving well in beta?

Comment 26

[forum.news]

So far I have never installed or tried a beta version. Can I run it in parallel to my release version? When will this fix be released officially?

Thanks
Konrad

Comment 27

[gene smith]

(In reply to forum.news from comment #26)

So far I have never installed or tried a beta version. Can I run it in parallel to my release version?

Yes you can. It installs the beta in its own directory in Program Files and it has its own special startup icon. It will run with your existing profile.
The only possible issue is that when you go back and run the 115 release after running the beta on the same profile, you might get a message that you must create a new profile. This can be avoided by changing the startup path for TB in the release shortcut to include the command line parameter --allow-downgrade. But you ran the "try" version in comment 3 without this issue it seems, so I doubt it will be a problem with the beta either.

Here the link to the latest beta:
https://archive.mozilla.org/pub/thunderbird/releases/125.0b3/

When will this fix be released officially?

I'm don't know the date, but to get the fix for this bug approved for the official release, it helps to have a verification from the bug reporter.
Thanks!

Comment 28

[forum.news]

The provided beta works perfectly! Drafts behave as expected and testing the fix for bug 1856536 also was successfully. All good ;-)

Thanks
Konrad

Comment 29

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Managed to reproduce this issue using a build from 2024-02-24 (125.0a1 buildID: 20240224091819)
Confirming this issue as verified fixed using 124.0b5(20240311164815) on Windows 11 and macOS 14.
CC'ing myself and Ramona in order to verify the fix in ESR when available.

Comment 30

[Wayne Mery (:wsmwk)]

Thanks Vlad.