470746 - IMAP Draft messages are marked as read (and thus won't be noticed)

Status: VERIFIED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Ben]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.4) Gecko/2008112522 Firefox/3.0.4
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b3pre) Gecko/20081204 Thunderbird/3.0b1

In an IMAP account, when saving a draft (through autosave?), it does not show in the draft folder until you click on the folder. It should however automatically update the contents of Drafts when saving a message there.

This is important because when it doesn't see the draft that has been saved by autosave, it is not deleted.

thanks

Reproducible: Always

Steps to Reproduce:
1. Compose new message ... take your time so it does an autosave
2. Send the message before IMAP explicitely checks (like every 10 min)
3. Go into Drafts folder --> there's your undeleted message

Thunderbird didn't know that it was there.
If you click on drafts before you send the message, and then you see it loads the message into the folder -- if you send the message then, then it gets deleted, but otherwise not.

Comment 1

[Magnus Melin [:mkmelin]]

Well on IMAP the drafts are marked as read, but on POP they are unread... I suppose that's the bug here. 

(I don't see the problem with drafts not getting deleted.)

Comment 2

[Ben]

Autosaves should get deleted though.
If I spend an hour writing an email (for example), then I have 6 autosave drafts in my draft folder, which eventually show up unread.
That's not ideal behavior.
Autosave drafts should get deleted once the message is sent, and on top of that, autosave drafts should overwrite each other and not just add to the list.
And I suppose autosaves should be marked as read, so as not to distract.

I don't know, I'm still using Thunderbird 3.0b1

Comment 3

[WADA:World Anti-bad-Duping Agency]

(In reply to comment #1)
> Well on IMAP the drafts are marked as read, but on POP they are unread...

Following is IMAP log of "Save As Draft", with Gmail IMAP & Tb trunk. 
> [1524] 2068[75bed98]: 75b5f90:imap.gmail.com:S-Drafts:SendData: 9 append "Drafts" (\Seen \Draft) {529}
Tb explicitly requests \Seen upon APPEND to "Drafts".
"Adding of \Seen" was done in the past by user's request? (for example, stop annoying new mail notification for each saved draft mail. If draft is saved in Inbox, it'll easily/frequently occur.)

Comment 4

[Nathan Tuggy (:tuggyne)]

Apparently this is a regression from bug 198087, fixed clear back in 2004. I'm not currently able to track down when the regression happened (hg+CVS+inexperience_with_Mozilla_tools = fun), but looking at that bug, it would appear the fix went into http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/mailnews/imap/src/nsImapMailFolder.cpp&rev=1.638#4394 -- however, there are no obvious signs of it in e.g. http://hg.mozilla.org/comm-central/file/tip/mailnews/imap/src/nsImapMailFolder.cpp

I can't find any bugs that deliberately reversed this, so I'd guess it happened somewhat by accident.

(In reply to comment #3)
> "Adding of \Seen" was done in the past by user's request? (for example, stop
> annoying new mail notification for each saved draft mail. If draft is saved in
> Inbox, it'll easily/frequently occur.)
No, in point of fact it was taken out in the past by users' request -- TB does not notify on new unread messages in the Draft folder anyway, and if you're saving drafts in the Inbox, well... "Doctor, it hurts when I do this!"

IMO, this bug is chiefly about the utility of remembering that you have unsent drafts in your Drafts folder.

Comment 5

[Mark Banner (:standard8)]

(In reply to comment #4)
> I can't find any bugs that deliberately reversed this, so I'd guess it happened
> somewhat by accident.

It could have been bug 301084, but I'm not 100% convinced without going back and retesting everything.

Comment 6

[Mark Banner (:standard8)]

Attachment: Possible fix

This seems to fix the bug, however it appears IMAP servers won't update the unread count until you select the folder (or you already have the folder selected).

I'd have thought that because this is a message copy, it should be updating the count on getting the "new" message into the found. Unless for some weird reason we'll only do that on checking for new mail, and hence would need the check for new mail preference set on the folder.

I think David should have a few better ideas, so requesting feedback from him.

Comment 7

[David :Bienvenu]

Comment on attachment 435566 [details] [diff] [review]
Possible fix

this looks reasonable, yeah.

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 435566 [details] [diff] [review]
Possible fix

Getting ui-review. Bryan this will make it so that drafts saved into imap folders will be unread - just the same as for Local Folders.

I suspect this patch needs a couple of tweaks and a unit test, but wanted to get your review before proceeding.

I suspect this may expose some bugs as well (e.g. multiple copies that I've seen mentioned occasionally), but that should be a good thing.

Comment 9

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 435566 [details] [diff] [review]
Possible fix

sounds good to me

Comment 10

[Mark Banner (:standard8)]

Attachment: Fix with unit test

This fixes it and adds a couple of unit tests. I was having issues with the imap test hanging on shutdown, but I think that is down to bug 606052 as I can't reproduce now.

Comment 11

[David :Bienvenu]

Comment on attachment 485680 [details] [diff] [review]
Fix with unit test

Don't think you want this copied comment:

+/*
+ * This file tests recognizing a message as junk due to
+ *  SpamAssassin headers, and marking that as good
+ *  without having the message return to the junk folder,
+ *  as discussed in bug 540385.
+ *
+ * adapted from test_filterNeedsBody.js
+ */

don't think this is used:

+// Globals
+const gMessage = "SpamAssassinYes"; // message file used as the test message
+

might as well remove the commented out line:
+  {
+    dl('msgsMoveCopyCompleted to folder ' + aDestFolder.name);
+//    async_driver();


do you really want the +++ b/mailnews/test/resources/mailShutdown.js changes?

Comment 12

[Mark Banner (:standard8)]

(In reply to comment #11)
> do you really want the +++ b/mailnews/test/resources/mailShutdown.js changes?

I was debating keeping them because of the issues we've had with tests on shutdown, but I think I'll leave those out for now.

Comment 13

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/0930e772903b

Comment 14

[Nikolay Shopik]

Just noticed this change by writing email, looks good.

Comment 15

[Nikolay Shopik]

Mark, any chance this could be backported to 3.1 tree, w/o breaking stuff?

Comment 16

[Mark Banner (:standard8)]

(In reply to comment #15)
> Mark, any chance this could be backported to 3.1 tree, w/o breaking stuff?

This won't be backported to 3.1.x because it is effectively a change of user experience, and whilst a regression, I don't really want to change it on a stable branch. I'm also suspecting it may highlight some existing bugs in saving drafts.

Comment 17

[Tammo van Lessen]

Although I like the idea of this fix, it introduces a regression with other clients, in this case with GMail (for Android), which is probably caused by a chain of other bugs. Would it be possible to make this feature configurable?

Here is what happens:
When I write an email reply with auto save on, it creates several drafts which are now all unread. This causes GMail for Android to include _all_ of them into the conversation the reply belongs to, also as unread mails. To make things worse, when the mail is sent, the drafts are moved (if you are lucky) to trash but remain marked as unread. So at the end of the day you end up with a conversation that unnecessarily contains a bunch of drafts of different states from the drafts and/or the trash folder.

Restarting TB and compacting the drafts folder helps sometimes to clean up the mess.

I'd just like to bring this "regresssion" to attention, if I shall file a new bug for this I'm happy to do this.

Comment 18

[Ludovic Hirlimann [:Usul]]

Tammo please file new bug(s).

Comment 19

[Tammo van Lessen]

Done. See Bug 673400.

Comment 21

[Tammo van Lessen]

Mark Banner and David Bienvenu, may I ask you have a look at Bug 673400 and the patch I attached to it? It aims at making the result of the fix of this bug configurable because marking drafts unread turns out to be very, very annoying when using Thunderbid in conjunction with GMail and Android devices. I attached an untested patch (due to lack of dev environment) but I hope it may serve as a starting point for a Mozialla TB dev. Thanks in advance.