485514 - drag & drop attachments truncates name part with '&'

Status: RESOLVED FIXED

(MailNews Core :: Attachments, defect)

Description

[Phil Lacy]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090312 Shredder/3.0b3pre
1.make arbitrary file with &, e.g. 'New text & file.txt'
2.draft letter and save and view in 3 pane.
3.tool tip and save message saves full name.
4.drag to o/s saves file as 'New text '

related bug 407577. fixes should try to fix both bugs.
bug 401542 may be related

Comment 1

[Phil Lacy]

Attachment: email test

open with shredder and drag attachments to local directory. Names are truncated at & and %nn is escaped instead of rendering unescaped.

Comment 2

[Phil Lacy]

Attachment: attachment display code fix

saving attachments work ok, only dragging. The front end code is msgHdrViewOverlay.js and is getting data from this backend.

Comment 4

[David :Bienvenu]

Phil, I thought you were going to comment about opening/saving/dragging, since it seemed to me that changing the libmime code would affect more than just dragging.

Comment 5

[Phil Lacy]

Sorry, meant to answer it in IRC.  The double clicking, save as, and opening the attachment also truncates the name part.  But this patch fixes that problem also.

Comment 6

[David :Bienvenu]

double clicking, opening, and save as all seem to do the right thing w/o the patch for me, on vista. I sent a file called "New text & file.txt" as an attachment.  Drag and drop did truncate the file. I'll try with the patch.

Comment 7

[David :Bienvenu]

with this patch, when I do the drag drop, I'm getting 0 byte files - maybe I was just lucky before, but w/o the patch, I was actually getting the right data in the file.

Comment 8

[Phil Lacy]

Mine works ok. 
Rechecking all with file name 'New %26 test.txt' and 'New & test.txt'

Right click attachment or menu: file->attachments both 'save as' works
Right click or file->attachment menu item 'open' brings up dialog box
 then both 'save' and open file is incorrect.

I'll rebuild and check on the 0 byte thing but it worked before.

Comment 9

[Phil Lacy]

Mine works ok. 
Rechecking all with file name 'New %26 test.txt' and 'New & test.txt'

Right click attachment or menu: file->attachments both 'save as' works
Right click or file->attachment menu item 'open' brings up dialog box
 then both 'save' and open file is incorrect. First one saves as "New & test.txt' second one just saves as 'New'

dragging file does not result in 0 byte file for me.

Comment 10

[Phil Lacy]

http://mxr.mozilla.org/comm-central/source/mailnews/local/src/nsMailboxService.cpp#354
This location appends a second "&filename=" which although is escaped by the js code, it is not used by the 'open with' code (or maybe the dialog box).  The first &filename= is used and is unescaped.  I don't know why we have 2 &filename=.

Can I correct that here?

As far as the 'save as', I'll review that and make another comment but it appears if it goes direct to the filepicker we are loading the escaped displayname (second $filename=) as the default 'save as' name which makes it correct.

Comment 11

[Phil Lacy]

http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMessenger.cpp#851
save as b/e code uses the display name that is escaped and it seems to be further 'sanitized'. Nevertheless the url still has an unencoded filename fixed in this patch.

Comment 12

[Phil Lacy]

A file from IMAP account has correct file names in all aspects. But the files received from OExp agent have incorrect data.  I'll file a separate bug on that. 

My tests are simple drafts in the local folders inbox.  David, can you confirm those situations?

I haven't tried POP yet.

Comment 13

[Phil Lacy]

nsIImapUrl works
nsMsgMailNewsUrl doesn't

http://mxr.mozilla.org/comm-central/source/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp#359

Comment 14

[Phil Lacy]

I think it's the nsMailboxUrl doesn't work.

Comment 15

[Phil Lacy]

in IMAP the nsImapService sets the filename from the encoded one passed in from the js code at least in handling attachments in msgHdrViewOverlay.js.

http://mxr.mozilla.org/comm-central/source/mailnews/imap/src/nsImapService.cpp#404

The nsMailboxService.cpp sets the filename here
http://mxr.mozilla.org/comm-central/source/mailnews/base/util/nsMsgMailNewsUrl.cpp#384
The above setting correctly copies up to the '&' because there shouldn't be one in the name. It get the bad name from the 'spec' which is corrected by this patch.

Comment 16

[David :Bienvenu]

Comment on attachment 371611 [details] [diff] [review]
attachment display code fix

sorry for the delay - this does seem like the right thing to do. Now that vista drag drop works, I can test this out a bit. I'm going to land a slightly tweaked version of this patch, where we check if (NS_SUCCEEDED) instead of failed, and switch the statements around.

Comment 17

[David :Bienvenu]

fix checked in.