1760025 - Cannot get attached email files via compose API.

Status: RESOLVED FIXED

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

Description

[John Bieling (:TbSync)]

See https://thunderbird.topicbox.com/groups/addons/Teac40c9048e3ebcb/issues-with-listattachments-and-getfile

Comment 1

[Volker]

Please note that this happens with emails attached (.eml) and also with attachments from other emails attached. For example, drag&drop an attachment from an existing email to the compose window of a new email.

Comment 2

[John Bieling (:TbSync)]

The reason for this is: The compose API (and also the cloudFile API, same bug there) expect the attachment to be a local file.

Would it be ok to create temporary local files when such things are being attached? This would happen here:
https://searchfox.org/comm-central/rev/695e6bdee2214b84351445001b29e269b3888e39/mail/components/compose/content/MsgComposeCommands.js#6922-6934

Instead of treating the special cases everywhere we currently use getFileFromURLSpec (which is what fails) to get the attachment file, I would handle these cases only once.

This is not only happening for entire messages being attached, but also for example single attachments from another email being dragged into the composer. We would automatically fix a bunch of similar issues, by enforcing (temp) local files.

Comment 3

[Magnus Melin [:mkmelin]]

Would it be ok to create temporary local files when such things are being attached?

I think that should be ok yes.

Comment 4

[John Bieling (:TbSync)]

Attachment: Bug 1760025 - Use temporary files for attached messages and attachments from other messages. r=mkmelin

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/fbef0eae0aa6
Use temporary files for attached messages and attachments from other messages. r=mkmelin

Comment 6

[Rachel Martin]

It's unfortunate that this code got copied:

+      let extAppLauncher = Cc["@mozilla.org/mime;1"].getService(
+        Ci.nsPIExternalAppLauncher
+      );

Clearly the nsPIExternalAppLauncher has nothing to do with mime, instead it should be done like here:
https://searchfox.org/comm-central/rev/d76107d796479ddf6c6c33057e63de932bb2e716/mail/extensions/openpgp/content/modules/persistentCrypto.jsm#151-152

You will ask why it works in the first place. Well, for some mysterious reason it seems to work by accident, since @mozilla.org/uriloader/external-helper-app-service;1 and @mozilla.org/mime;1 for some unexplicable reason have the same contract ID:
https://searchfox.org/mozilla-central/rev/8766f623040634d3d6c17150a6ec60e5fd30602e/docshell/build/components.conf#165-169

Comment 7

[John Bieling (:TbSync)]

Attachment: Bug 1760025 - Fix wrong usage of nsPIExternalAppLauncher. r=#thunderbird-reviewers

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/f1a4746ab4f8
Fix wrong usage of nsPIExternalAppLauncher. r=mkmelin

Comment 9

[Rachel Martin]

And these two?
https://searchfox.org/comm-central/search?q=let+extAppLauncher&path=MsgComposeCommands.js&case=false&regexp=false

Comment 10

[John Bieling (:TbSync)]

I returned to my latest public head when searching for that usage and forgot to do a pull, so I did not have my own patch included anymore. Thanks for noticing.

Comment 11

[John Bieling (:TbSync)]

Attachment: Bug 1760025 - Fix more wrong usage of nsPIExternalAppLauncher. r=mkmelin

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/53fd4281c32d
Fix more wrong usage of nsPIExternalAppLauncher. r=mkmelin