1528932 - Dragging attachments of message onto the desktop sometimes use the mailbox name as filename (e.g. when attached.pdf is only Mime part)

Status: RESOLVED FIXED

(MailNews Core :: General, defect, P5)

Description

[Jorg K (CEST = GMT+2)]

Drag "Foo.eml" from attachment 8850706 [details] from bug 1350080 or "google.com!xxxxxx.dk!1435536000!1435622399.zip" from attachment 8864261 [details] from bug 1361422 onto the desktop.

The filename used is the mailbox name, not the attachment name as shown in the attachment area.

Saving the message uses the correct name.

I remember looking at this at some stage, somehow the correct name isn't available at the time. Alfred, can you take a look.

Comment 2

[Thomas D. (:thomas8)]

Confirming on Win10, TB 78 and Daily 90.0a1 (2021-05-15) (64-bit), both with almost same symptoms.

STR

0. Save test.eml from attachment 9222167 [details] to your hard disk.
1. test.eml contains test.pdf as the only Mime part. That in itself does not disturb Thunderbird, and the msg seems to show correctly with blank body and pdf attachment.
2. Standalone message case: File > Open > Saved message > test.eml from attachment 9222167 [details]

• drag attached test.pdf to OS folder

3. Inbox message case: After step 2: Message > Copy to > Inbox

• drag attached test.pdf to OS folder

Actual

• Standalone message case: after drop, test.pdf gets saved as intact PDF with right base name, but wrong .eml extension (test.eml).
• Inbox message case: after drop, test.pdf gets saved as intact PDF with name of TB account folder + number INBOX94364 (no extension). On Daily, filename is Inbox (without number).

Expected

• save dropped attachment with correct file name, test.pdf

Comment 3

[KN4CK3R]

Attachment: drag-drop-attachment-name.patch

Comment 4

[Ben Campbell]

Comment on attachment 9222819 [details] [diff] [review]
drag-drop-attachment-name.patch

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

Looks good to me!

::: mailnews/mime/src/mimemoz2.cpp
@@ +80,5 @@
> +    const nsMsgAttachmentData* attachmentData, nsCString& url) {
> +  url.AppendLiteral("&filename=");
> +  nsAutoCString aResult;
> +  if (NS_SUCCEEDED(MsgEscapeString(attachmentData->m_realName,
> +                                   nsINetUtil::ESCAPE_XALPHAS, aResult)))

nit: while here, I'd probably take the opportunity to add braces around the if/else one-liner blocks
(https://firefox-source-docs.mozilla.org/code-quality/coding-style/coding_style_cpp.html#control-structures)

Comment 5

[KN4CK3R]

Should I add them only in my code or in the whole file?

Comment 6

[Thomas D. (:thomas8)]

(In reply to KN4CK3R from comment #5)

Should I add them only in my code or in the whole file?

Imho, only in your code, please. There are more than 62 occurrences of if one-liners in this file, too much for a peripheral change. Better to do that in a dedicated followup bug. Keeping separate things separate also makes it easier to track where things went wrong if they do.

Comment 7

[Ben Campbell]

What Thomas said :-)
Also, you don't need to request another review for that code - the general convention is that 'nit' comments are just for little things that don't really affect the functioning. Typos, formatting, minor comment clarifications, etc... So if you upload a revised patch, you can just set the "r+" flag on it directly.
(and thanks very much for the fix!)

Comment 8

[KN4CK3R]

Attachment: drag-drop-attachment-name.patch

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e5df9561f1cf
fixed drag&drop attachment name for when e.g. when attached.pdf is only Mime part. r=benc