541072 - IMAP mark-as-deleted model: undelete advances to next message

Status: NEW

(MailNews Core :: Networking: IMAP, defect)

Description

[[e-mail]]

+++ This bug was initially created as a clone of Bug #451877 +++

Using the IMAP with the mark as deleted model quite annoying since undeleting a message advances to the next message.

Happens both on 3.0 and trunk.

Comment 1

[[e-mail]]

Attachment: Possible patch

This might fix it, but I don't know how to write a test for it.

Comment 2

[David :Bienvenu]

test is difficult, if not impossible, because we can't do imap with mozmill, and mozmill doesn't work offline (though that's getting fixed, I believe).

Comment 3

[Wayne Mery (:wsmwk)]

(In reply to David :Bienvenu from comment #2)
> test is difficult, if not impossible, because we can't do imap with mozmill,
> and mozmill doesn't work offline (though that's getting fixed, I believe).

is this still true?  And if it is, can we accept patch without test?

Comment 4

[:Irving Reid (No longer working on Firefox)]

Comment on attachment 422723 [details] [diff] [review]
Possible patch

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

::: mailnews/imap/src/nsImapMailFolder.cpp
@@ +2335,5 @@
>            mDatabase->DeleteMessages(srcKeyArray.Length(), srcKeyArray.Elements(), nsnull);
>            EnableNotifications(allMessageCountNotifications, PR_TRUE, PR_TRUE /*dbBatching*/);
>          }
> +        if (deleteMsgs)
> +          NotifyFolderEvent(mDeleteOrMoveMsgCompletedAtom);

Would it be better to put an else clause on this like

else
  NotifyFolderEvent(mDeleteOrMoveMsgFailedAtom);

There isn't a perfectly suited notification atom, but at least this tells listeners that something has changed.

As far as the test goes, an xpcshell test that the correct notification gets sent, combined with a manual test (in TB and SM) that the display updates correctly, works for me.

Comment 5

[Wayne Mery (:wsmwk)]

Do you still see this?
I tried a couple times and was unable to reproduce.

Comment 6

[Magnus Melin [:mkmelin]]

Absolutely still there. Maybe we should just land the patch as testing is way challenging as we can't test imap in mozmill.

Comment 7

[Wayne Mery (:wsmwk)]

(In reply to Magnus Melin [:mkmelin] from comment #6)

Absolutely still there. Maybe we should just land the patch as testing is
way challenging as we can't test imap in mozmill.

Comment 8

[Wayne Mery (:wsmwk)]

Ben if you are able to land this patch, then I can highlight it for testing in the upcoming beta.

Comment 9

[Wayne Mery (:wsmwk)]

Ben, I don't remember - you use mark as deleted?