Open Bug 1789565 Opened 2 years ago Updated 2 years ago

nsMessenger::SaveAttachment() calls nsIURLListener after first item saved, even if multiple items are queued up.

Categories

(MailNews Core :: Backend, defect)

defect

Tracking

(Not tracked)

People

(Reporter: benc, Unassigned)

References

Details

A little obscure, but I noticed that
nsMessenger::SaveAttachment() (the internal override, not the one that's exposed as part of the nsIMessenger interface) will call the supplied listener after the first item is saved, even if there are multiple items queued up (via an nsSaveAllAttachmentsState object).

I think it only currently affects nsMessenger::DetachAttachmentsWOPrompts(), as that's the only exposed interface that accepts multiple attachments and a listener, but it could be a bigger deal if listeners are added to other attachment functions, as per Bug 1788159.

I'll probably be adding a solution to this as part of Bug 1786237, but in this bug we should at least a unit test to check that saving and/or detaching multiple attachments triggers listeners as expected...

Just some notes after Bug 1786237:
SaveAttachment() still takes it's own listener (which triggers after that single attachment). In Bug 1786237 I added an overall listener to nsSaveAllAttachmentsState which triggers after all the attachments are complete.
I think there are still one or two places that still use the listener parameter on SaveAttachment(), but I think it should be ditched entirely in favour of using the overall listener on nsSaveAllAttachmentsState.

But I also think the whole mess should be refactored into something simpler. The lifetime management of nsSaveAllAttachmentsState is bonkers. It's currently passed between successive nsSaveMsgListener objects by successive SaveAttachment() calls. There's so much scope for subtle issues which would give memory leaks or let errors slip through without informing listeners... so. Bug 1788159.

You need to log in before you can comment on or make changes to this bug.