Closed Bug 1521706 Opened 9 months ago Closed 9 months ago

nsIMsgFolder.deleteMessages should notify listener in all cases

Categories

(MailNews Core :: General, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 66.0

People

(Reporter: darktrojan, Assigned: darktrojan)

References

Details

Attachments

(2 files)

Some implementations of deleteMessage do nothing with the passed listener in some circumstances, e.g. IMAP when bypassing the trash folder. In order to make bug 1520338 easier, I'm looking into what does and doesn't use the listener, and then making everything use the listener.

That turned out to be easier than expected. I can't see any more implementations of the method.

Attachment #9038180 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9038180 [details] [diff] [review]
1521706-delete-messages-listener-1.diff

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

Looks reasonable. I notice we have a NotifyDeleteOrMoveMessagesCompleted comment but no implementation. Maybe we should remove that, or add a comment there
Attachment #9038180 - Flags: review?(mkmelin+mozilla) → review+

It seems to have been superseded by kDeleteOrMoveMsgCompleted. Same goes for NotifyFolderLoaded. I'll remove them.

Attachment #9038372 - Flags: review?(mkmelin+mozilla)
Attachment #9038372 - Flags: review?(mkmelin+mozilla) → review+

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/424dfd930d60
When a listener is passed to nsIMsgFolder.deleteMessages, notify it on completion; r=mkmelin
https://hg.mozilla.org/comm-central/rev/e8392f4d3862
Remove commented, unimplemented methods NotifyDeleteOrMoveMessagesCompleted, NotifyFolderLoaded; r=mkmelin

Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 66.0

This broke a bunch of tests in mailnews/db/gloda/test/unit. :(

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/8283e69c9dfa
Fix test function not expecting deleteMessages to notify listeners; rs=bustage-fix
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/954fd69c1e3c
When a listener is passed to nsIMsgFolder.deleteMessages, notify it on completion; r=mkmelin
https://hg.mozilla.org/comm-central/rev/5565f32e3a87
Remove commented, unimplemented methods NotifyDeleteOrMoveMessagesCompleted, NotifyFolderLoaded; r=mkmelin

Status: REOPENED → RESOLVED
Closed: 9 months ago9 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.