1395252 - Thunderbird 57 - Unable to add attachments to email

Status: VERIFIED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Bob Clary [:bc] (inactive)]

I recently realized that Daily was no longer adding attachments. Steps to reproduce are to start a new email, then attempt to add an attachment. Nothing is added but I found the following in the console:

NS_ERROR_MALFORMED_URI: Component returned failure code: 0x804b000a (NS_ERROR_MALFORMED_URI) [nsIFilePicker.file] MsgComposeCommands.js:4080
AttachFile/< chrome://messenger/content/messengercompose/MsgComposeCommands.js:4080:63

I'm not sure how recent this regression is. It is more than just a few days, but I haven't narrowed down the regression range. I'll try to do that when I have them time unless someone wants to do it before then.

Comment 1

[Alice0775 White]

Regression window:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=5a44a25a7d82ed1954b6eb994eab2a8155ac1e34&tochange=8802479e93e761eed1ee4497b92e22f6bb0f043b
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=80ff3f300e05f38f96c385b03d1973a966a2bd35&tochange=6062341662fcee683434ee71539bfa10964c022e

Suspect: Bug 1341211

Comment 2

[Jorg K (CEST = GMT+2)]

Thanks for reporting. In bug 1341211 we sadly had to turn the whole system upside down due to some changes in Mozilla Central.

I can reproduce the problem that no attachment is added, but not the error in the console. In a debug window I see:
[428] WARNING: RememberLastUsedDirectory failed to init file path.: file c:/mozilla-source/comm-central/mozilla/widget/windows/nsFilePicker.cpp, line 789

Anyway, we'll get this fixed pronto. In the meantime, please drag files onto the addressing widget to attach them.

Aceman, looks like this slipped through the review cracks.

Comment 3

[Jorg K (CEST = GMT+2)]

Attachment: 1395252-filepicker.patch

I've checked, we use nsIFilePicker.modeOpenMultiple three times in C-C and this one is wrong.

Comment 4

[:aceman]

Comment on attachment 8902912 [details] [diff] [review]
1395252-filepicker.patch

Review of attachment 8902912 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, this seems to help.

::: mail/components/compose/content/MsgComposeCommands.js
@@ +4081,2 @@
>        return;
>      }

I notice this block doesn't need the {}.

@@ +4083,5 @@
>      let file;
>      let attachments = [];
>  
>      for (file of fixIterator(fp.files, Components.interfaces.nsIFile))
>        attachments.push(FileToAttachment(file));

While a for() block always needs {} in our style.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/a8c9aa8f996c
Bug 1341211 follow-up: Use nsIFilePicker.files when using nsIFilePicker.modeOpenMultiple. r=aceman DONTBUILD

Comment 6

[Jorg K (CEST = GMT+2)]

(In reply to :aceman from comment #4)
> While a for() block always needs {} in our style.
There are many for() blocks in that file and I'm not on a clean-up tour here. I took away the braces since I touched that line anyway.

Bob, this will be fixed in tomorrow's Daily.

Comment 8

[Bob Clary [:bc] (inactive)]

Works like a charm! Back on Daily now. Thanks!

Comment 9

[:aceman]

Thanks for confirmation.

Comment 10

[Jorg K (CEST = GMT+2)]

(In reply to Bob Clary [:bc:] from comment #8)
> Works like a charm! Back on Daily now. Thanks!
Thanks for riding the sometimes bumpy Daily channel. There was no Daily on 31st August, I cancelled it due to other problems. Today's Daily is fine as far as I can tell.