1747441 - `compose.listAttachments` and `compose.addAttachment` should be reflexive

Status: RESOLVED FIXED

(Thunderbird :: Add-Ons: Extensions API, enhancement)

Description

[siefkenj]

Steps to reproduce:

The results from compose.listAttachments should be able to be fed into compose.addAttachment. Currently there is an error that an attachments needs a file property.

A potential backwards-compatible solution would be to look at an attachment object. If it has an id, then clone the existing attachment with that id. Otherwise, proceed as before.

However, I think it would be nice if the addAttachment API also accepted a File object directly. Would there be a downside to this?

Related to this issue, it would be nice if there were an addAttachments API so one could do compose.addAttachments(10, await compose.listAttachments(9)) to clone the attachments from tab 9 to tab 10.

Unrelated (and maybe discussed elsewhere...) is there a reason for the name listAttachments rather than getAttachments?

Comment 1

[John Bieling (:TbSync)]

Attachment: Bug 1747441 - Add support for referencing existing ComposeAttachments when adding attachments. r=mkmelin

Depends on D134648

Comment 2

[John Bieling (:TbSync)]

Unrelated (and maybe discussed elsewhere...) is there a reason for the name listAttachments rather than getAttachments?

I guess the reason was to align it with messages.list(), which gets a list of all messages in a folder, and with messages.listAttachements()

With this patch, something like this should work:

browser.composeAction.onClicked.addListener(async (tab) => {
    let details = await browser.compose.getComposeDetails(tab.id);
    details.attachments = await browser.compose.listAttachments(tab.id);
    await browser.compose.beginNew(details)
})

or

let attachments = await browser.compose.listAttachments(tab1.id);
attachments.forEach(attachment => await browser.compose.addAttachment(tab2.id, attachment));

I think adding an extra addAttachments() function just to save the forEach loop is not necessary.

Does the patch work for you? It will also improve the documentation, as currently the attachments property of the ComposeDetails type is just of type object but there is no explanation what kind of object it is: it is either a FileAttachment or a ComposeAttachment.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/6b5b06bd76f2
Add support for referencing existing ComposeAttachments when adding attachments. r=mkmelin