246051 - Hang when drag attachment to desktop when the file name has an accented char

Status: RESOLVED FIXED

(MailNews Core :: Attachments, defect)

Description

[Fernando Hartmann]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040608
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040608

When try to drag a file attached in a mail to disk that have a name with ascent,
for example: ó.txt   the following message appear:

"Unable to save attachment. Please check your file name and try again later."

Well, the message have a OK bottom, but they don't  work. the only way is to
close entire Mozilla with "Task Manager"


Reproducible: Always
Steps to Reproduce:
1. create a file like ó.txt
2. attach this file in new mail and send it to yourself
3. receive the mail,
4. try drag the attach to a desktop
Actual Results:  
The message "Unable to save attachment. Please check your file name and try
again later." appears and entire Mozzila freeze

Expected Results:  
Save the file in disk.

My Windows XP is in Portuguese Brazilian "pt_BR" language.

Comment 1

[Mike Cowperthwaite]

Reproduced with 
  Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a2) Gecko/20040604

Tried it with 1.6 Final, and could drag the same attachment using that version 
with no problem.

Comment 2

[(not reading, please use [e-mail] instead)]

two bad things are going on here:  the hang, and the inability to save the
attachment.

for the inability to save the attachment, I can see some problems while
debugging nsSaveMsgListener::OnStartRequest()

for a file like "foo.txt", I end up with this for m_fileSpec
"C:\DOCUME~1\SETH\LOCALS~1\foo.txt"
for file names like "ó.txt", I end up with "C:\DOCUME~1\SETH\LOCALS~1\", and
that causes me to fail to open the stream.

Comment 3

[(not reading, please use [e-mail] instead)]

ok, it looks like the problem starts in nsMessenger::SaveAttachmentToFolder()

when we call ConvertAndSanitizeFileName() with "ó.txt" this fails silently:

  nsCAutoString unescapedName(displayName);

  /* we need to convert the UTF-8 fileName to platform specific character set.
     The display name is in UTF-8 because it has been escaped from JS
  */ 
  NS_UnescapeURL(unescapedName);
  NS_ConvertUTF8toUCS2 ucs2Str(unescapedName);

for displayName of "ó.txt", ucs2Str will be "".

Comment 4

[(not reading, please use [e-mail] instead)]

I think we're just missing a call to encodeURIComponent() in the js.

Comment 5

[(not reading, please use [e-mail] instead)]

of course, a debug build asserts, but I didn't see it since I use
XPCOM_DEBUG_BREAK=warn

from my console:

###!!! ASSERTION: not a UTF8 string: 'Error', file ../../dist/include/string\nsU
TF8Utils.h, line 151
###!!! ASSERTION: Input wasn't UTF8 or incorrect length was calculated: 'Error',
 file c:/mozilla/trees/trunk/mozilla/xpcom/string/src/nsReadableUtils.cpp, line
262
Opening file LOCALS~1 failed

testing a patch now....

Comment 6

[(not reading, please use [e-mail] instead)]

Attachment: fix

Comment 7

[(not reading, please use [e-mail] instead)]

I've landed that fix to seamonkey trunk, tbird trunk, and aviary 1.0 branch.

for some background, see #225695 and #83803.

Comment 8

[(not reading, please use [e-mail] instead)]

note, if we fail some other way, we would still get this dialog that is hard to
dismiss.

note for me, tbird didn't hang, but I couldn't use the mouse to dismiss the dialog.

I had to use the keyboard.

I'll log a spin off bug about it.

Comment 9

[[e-mail]]

The other way to fix this would have been to make the parameters UTF8String,
AString or wstring so that Unicode can be correctly passed through to the C++.

Comment 10

[Hiro]

In Thunderbird, it is copied to the place on 1 class of the folder which Droped.

For example, if it Drop at a home(~/), it will be copied to a users(/Users).

Mac OS X 10.3.4
aviary 1.0 branch build.
version 0.7+ (20040718)

Comment 11

[Hiro]

additional comment:
When Japanese is contained in an attached file name, an error occurs in Drop.

Comment 12

[(not reading, please use [e-mail] instead)]

> The other way to fix this would have been to make the parameters UTF8String,
> AString or wstring so that Unicode can be correctly passed through to the C++.

neil, are you saying that would be a better fix or just another way of doing it?

Doing it that way would involve changing the following methods on the
nsIMessenger interface:

openAttachment()
saveAttachment()
saveAllAttachments()
saveAttachmentToFolder()

and the callers and the implementations.

I fixed it the way I did it because:

1) it followed the changes that jshin made.

1.9 	jshin%mailaps.org	2003-12-03 13:52	 	bug 225695 : follow-up to bug 44272.
tree-wide sweep to replace escape/unescape() with en|decodeURI(Component). part
2(mail/mailnews) of 3. r=neil, sr=bienvenu, a=asa

2) ...and it involved fewer changes (so it is lower risk)

Comment 13

[(not reading, please use [e-mail] instead)]

neil explained over aim what he meant:

"I know jshin wanted to try and make mailnews use unicode-safe strings
throughout, so at some point I assume he's going to fix up apis such as those
that take ASCII strings and thus currently need extra escapes/unescapes or other
workarounds for the issues."

Comment 14

[Mike Cowperthwaite]

*** Bug 252471 has been marked as a duplicate of this bug. ***

Comment 15

[Doron Shikmoni]

*** Bug 252630 has been marked as a duplicate of this bug. ***