Bug 1788159 Comment 0 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

The nsIMessenger attachment functions seem a little convoluted.

Here are my proposals on what should change:

`saveAllAttachments()`
 - Don't prompt for filenames. Make the caller pass in destination path(s) up front
 - Single messageUri rather than array? (all attachments must be from same message, right?)
 - Rename to saveAttachments()
 - Make sure it works with multiple attachments
 - Add unit tests (there's a mochitest, but no xpcshell test)

`detachAllAttachments()`
 - Rename to detachAttachments().
 - Add a listener to trigger when complete.
 - Ditch saveFirst option (make the caller do it - easy if there's listener support!)
 - Single messageUri rather than array? (all attachments must be from same message, right?)
 - No UI prompting (not sure if it does or not right now).
 - Caller to pass in list of file urls for previously-detached attachments (to fill out X-Mozilla-External-Attachment-URL headers in the deleted attachements).
 - Make sure it works with multiple attachments
 - Add unit tests (there's a mochitest, but no xpcshell test)

`detachAttachmentsWOPrompt()`
 - Ditch entirely, use detachAttachments() instead.

`detachAttachment()`
 - Ditch entirely, use detachAttachments() instead.

`saveAttachment()`
 - Ditch entirely, use saveAttachments() instead.


See also:
  - Bug 1578801 -Provide an additional save path attribute in function call `nsMessenger::DetachAllAttachments()`
The nsIMessenger attachment functions seem a little convoluted.

Here are my proposals on what should change:

`saveAllAttachments()`
 - Don't prompt for filenames. Make the caller pass in destination path(s) up front
 - Single messageUri rather than array? (all attachments must be from same message, right?)
 - Rename to saveAttachments()
 - Make sure it works with multiple attachments
 - Add unit tests (there's a mochitest, but no xpcshell test)

`detachAllAttachments()`
 - Rename to detachAttachments().
 - Add a listener to trigger when complete.
 - Ditch saveFirst option (make the caller do it - easy if there's listener support!)
 - Single messageUri rather than array? (all attachments must be from same message, right?)
 - No UI prompting (not sure if it does or not right now).
 - Caller to pass in list of file urls for previously-detached attachments (to fill out X-Mozilla-External-Attachment-URL headers in the deleted attachements).
 - Make sure it works with multiple attachments
 - Add unit tests (there's a mochitest, but no xpcshell test)

`detachAttachmentsWOPrompt()`
 - Ditch entirely, use detachAttachments() instead.

`detachAttachment()`
 - Ditch entirely, use detachAttachments() instead.

`saveAttachment()`
 - Ditch entirely, use saveAttachments() instead.

Back to Bug 1788159 Comment 0