Closed Bug 1528932 Opened 4 years ago Closed 1 year ago

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

Categories

(MailNews Core :: General, defect, P5)

Tracking

(thunderbird_esr78 wontfix)

RESOLVED FIXED
91 Branch
Tracking Status
thunderbird_esr78 --- wontfix

People

(Reporter: jorgk-bmo, Assigned: admin)

References

Details

Attachments

(1 file, 1 obsolete file)

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.

Duplicate of this bug: 1711481

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

STR

  1. Save test.eml from attachment 9222167 [details] to your hard disk.
  2. 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.
  3. Standalone message case: File > Open > Saved message > test.eml from attachment 9222167 [details]
  • drag attached test.pdf to OS folder
  1. 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
Summary: Dragging attachments of message onto the desktop sometimes use the mailbox name as filename → Dragging attachments of message onto the desktop sometimes use the mailbox name as filename (e.g. when attached.pdf is only Mime part)
Priority: -- → P2
Severity: normal → S3
Priority: P2 → P5
Attached patch drag-drop-attachment-name.patch (obsolete) — Splinter Review
Attachment #9222819 - Flags: review?(mkmelin+mozilla)
Attachment #9222819 - Flags: review?(mkmelin+mozilla) → review?(benc)
Assignee: nobody → admin
Status: NEW → ASSIGNED
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)
Attachment #9222819 - Flags: review?(benc) → review+

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

(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.

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!)

Attachment #9222819 - Attachment is obsolete: true
Attachment #9224361 - Flags: review+

Pushed by mkmelin@iki.fi:
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

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 91 Branch
Duplicate of this bug: 1411185
You need to log in before you can comment on or make changes to this bug.