+++ This bug was initially created as a clone of Bug #1669828 +++

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Firefox/78.0

Steps to reproduce:

1. Add an attachment to a message
2. Upload the attachment to a Filelink provider ("Convert to ..." in the attachment's context menu).

=> The file is uploaded and the download URL is inserted into the message.

3. Change the attachment from an Addon using messenger.compose.updateAttachment().

Actual results:

a) The attachment is changed:

• Itappears to be local in the message again
• It has the name and contents supplied in compose.updateAttachment

b) The URL to the uploaded, original attachment is still in the message
c) The URL points to a file with the old name and contents
d) If you remove the attachment from the attachments list, the URL of the old file is removed from the message.
e) If you send the message, the changes by compose.updateAttachment are lost: the message is sent without the new attachment. Instead it contains a dummy, pointing to the uploaded (old) attachment.

The sender does not know that she sent the old attachment, not the updated one.

Expected results:

The uploaded attachment should change according to the changes by compose.updateAttachment: Rename and/or upload new file to the cloud.
This could be acheived by triggering the cloudFile.onFileDeleted and cloudFile.onFileUpload events.

or

The uploaded file and the URL should be removed and the attachment should be treated as a new, local attachment.
This would be the equivalent of selecting "Convert to normal attachment" from the attachment's context menu following by the changes, still resulting in a cloudFile.onFileDeleted event.

Correction:
d) If you remove the attachment from the attachments list, the URL of the old file is not removed from the message.

Urgh, when I was writing the attachments API it was supposed to handle cloud file attachments differently, but that must've got taken out somewhere along the line. :-(

Do I understand updateAttachment correctly: If I call removeAttachment/addAttachment the attachmentId changes, with updateAttachment it stays the same. Is this really the only difference for a user of the API?

Then a simple solution might be to deprecate compose.updateAttachment (before addons start using it ;-) and add compose.renameAttachment together with cloudFile.onFileRenamed().

This might be related to bug 1669897.

Johannes (reporter of this Bug 1669829, Bug 1669831, and Bug 1669897), thank you very much for alerting us on these issues and for your constructive proposals!

For Filelink attachments depending on the contents of data this should trigger some events:

• If only data.name is present, the proposed cloudFile.onFileRename event should be fired (see bug 1669897). But what to do if that returns an error?
• If data.file is present fire cloudFile.OnFileDeleted for the old file. Then do whatever is necessary to fire cloudFile.onFileUpload for the new file with either data.name (if present) or data.file.name as fileInfo.name. Then update the download url and text in the message. But
  • How to handle errors from the cloudFile.onFileUpload if compose.updateAttachment is called in an onBeforeSend listener?
  • What if the user edited the text surrounding the the url (in a plain text message) and Thunderbird does not recognize it to change it?

(In reply to Johannes Endres from comment #7)

• If only data.name is present, the proposed cloudFile.onFileRename event should be fired (see bug 1669897). But what to do if that returns an error?

And what if the Filelink provider does not register an cloudFile.onFileRename handler?

I am not so sure, updateAttachment should temper with cloud attachments. Currently, uploading files is a user initiated action. If we allow to trigger file uploads trough updateAttachment, then any other add-on could load files onto the server of the user. I have to think about that.

File uploads can not only fail due to renaming issues, but also due to space issues and we would need to deal with that.

Not sure either. My first thought is that uploading a new file is not worse than replacing a regular attachment.

What's the alternative? Should updateAttachment fail if called on a Filelink attachment?

Depends on D134648

Try run is looking good:
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=abd1cb3471f53840b4e9c861aeb30ed7ecf15285

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/981e6ad79c78
Fix compose.updateAttachment() to work with cloudFile attachments. r=mkmelin