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

NEW
Unassigned

Status

MailNews Core
Networking: IMAP
--
minor
8 years ago
2 years ago

People

(Reporter: neil@parkwaycc.co.uk, Unassigned)

Tracking

({regression})

Trunk
regression

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
+++ 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.
(Reporter)

Comment 1

8 years ago
Created attachment 422723 [details] [diff] [review]
Possible patch

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

Comment 2

7 years ago
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).

Updated

7 years ago
Severity: major → minor

Comment 3

5 years ago
(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?
Flags: needinfo?(irving)
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.
Flags: needinfo?(irving)

Comment 5

2 years ago
Do you still see this?
I tried a couple times and was unable to reproduce.
Flags: needinfo?(neil)

Comment 6

2 years ago
Absolutely still there. Maybe we should just land the patch as testing is way challenging as we can't test imap in mozmill.
Flags: needinfo?(neil)
You need to log in before you can comment on or make changes to this bug.