332108 - can attach but cannot send a file with non-ASCII name even in the file system encoding

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect)

Description

[Mike Cowperthwaite]

When attempting to attach a file with a Unicode name (e.g. something Asian) to 
a message, using the file-open dialog (e.g. File|Attach), the name is copied incorrectly and the message cannot be sent.

1) Start a new message composition
2) Attach a file with a Unicode name by clicking the Attach toolbutton.
3) Send the file

Actual results:
2) filename in panel shows ____ instead of proper name
3) Error:
  Sending of message failed.
  Unable to open the temporary file [[<path>\_____.ext]]. ...

Expected results
2) filename shown as seen in the Explorer or File Open windows.
3) Transmission without a problem.

I first reported this (bug 285073 comment 5) in 2005 with Gecko 1.8b2 versions of TB and Mozilla.  Still present in TB 1.5, 2a1-0328 and (importantly) 
3a1-0327, which has the patch for bug 162361 in place.
I am running an English installation of Windows 2000.  I see the same results whether the compose window has the (default) ISO-8859-1 charset or the UTF-8 or UTF-16LE charset.

Comment 1

[Mike Cowperthwaite]

This may be closely related to bug 234681.

See also bug 332110.

Comment 2

[Jungshik Shin]

Mike, the latest nightly (trunk) doesn't have this problem. I had no problem attaching a file with a Hindi name on Windows 2000 (with the default code page set to Korean CP 949) until I tried to send it. When I tried to send it, I get an error message about not being able to open a temp file. The gibberish filename that TB cannot open for sure resulted from a UTF-8 string *misinterpreted* as CP949. This indicates that somewhere in mailnews nsI(Local)File or nsFileURL (the latter is a lot more likely) is incorrectly converted to nsFileSpec assuming that nsFileURL has a path encoded in the system codepage (rather than UTF-8 which is the case now thanks to bug 278161 having been fixed). As a result, even a file whose name can be covered by the current system codepage (i.e. French/Swedish  on English Windows, Japanese on Japanese Windows) cannot be attached. 

Given this, I'm taking the liberty of morphing this bug to what I've just described. 

Comment 3

[Mike Cowperthwaite]

OK, I see that difference too, with the 0329 nightly.

One exception to this problem is if you drag an attachment from a displayed message to new one.  With three different trunk builds -- 0316, 0327, 0329 -- this works entirely as expected: the correct name is shown in the attachment panel, the message sends (and the filename is RFC2231-encoded -- the first time I've actually seen that occur on my system).  With 2a1-0328, there is an 
"Unable to save attachment" error popped up after the drop, but the file still appears correctly and sends correctly.

Comment 4

[Jungshik Shin]

http://lxr.mozilla.org/seamonkey/source/mailnews/compose/src/nsMsgSend.cpp#2284

2284         m_attachments[newLoc].mFileSpec = new nsFileSpec( nsFileURL(url.get()) );

url is a file url whose spec is escaped UTF-8 path, but nsFileSpec expects to get a file path in the native encoding (actually, it accepts whatever C string is thrown at it, but later it's interpreted as in the native encoding). This should fail on both Windows and Linux (with non-UTF-8 locale). However, by some magic, it works fine on Linux. Anyway, I can work around this in an ad-hoc manner, but perhaps a long term solution would be to replace |nsFileSpec* mFileSpec| in nsMsgAttachmentHandler with |nsCOMPtr<nsILocalFile> mLocalFile|.

Comment 5

[Patrick]

Dupe of/related to bug 329129?

Comment 6

[Jungshik Shin]

Attachment: ad-hoc patch

this fixes the problem on Windows. So shoult it on Linux with a non-UTF-8 locale, but I haven't tested it yet. (as mentioned in comment #4, somehow, the problem doesn't manifest itself on Linux). I can add more check (like |if IsUTF8(....)|
or NS_IsNativeUTF8()|...

Comment 7

[Jungshik Shin]

(In reply to comment #5)
> Dupe of/related to bug 329129?
 It cannot be a dupe. This bug is a regression due to the fix for bug 278191 (and possibly bug 162361) the patchese for which were not checked in until late-March, but bug 329129 was filed on March 2nd. 

Comment 8

[Mike Cowperthwaite]

(In reply to comment #7)
>  This bug is a regression due to the fix for bug 278191

That should be bug 278161, of course (per comment 2).

Comment 9

[Jungshik Shin]

Attachment: updated ad-hoc patch

This a short-term solution that we need to apply to 1.8 branch once bug 162361 and bug 278161 are fixed on the branch. 

Scott, do you consider this patch for the trunk on an interim basis or do you prefer to have 'the right' fix that would get us a step closer to the total elimination of nsFileSpec?

Comment 10

[Hermann Schwab]

I'm setting Bug 332751 a blocked, as it maybe a dupe. The reporter couldn't send 
if the name of the attachment was containing Scandinavian characters.

I tested with german Umlauts and found, that I couldn't save as draft or sent, if the name of the attachment contained umlauts AND the extension was .txt, see screenshot https://bugzilla.mozilla.org/attachment.cgi?id=217255

Comment 11

[Jungshik Shin]

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

Comment 12

[Jungshik Shin]

Comment on attachment 217136 [details] [diff] [review]
updated ad-hoc patch

Before landing this patch on the branch, I think it's better to bake it for a while on the trunk. After that, we'll have to fix this 'the right way' by getting rid of nsFileSpec. 

Both sending a mail and saving a draft work fine with this patch.

Comment 13

[Jungshik Shin]

thanks for r/sr. the patch was checked into the trunk. I filed a new bug (bug 334280) about filenames like ('Japanese' name on 'French' Windows). 

I'm also marking this as blocking bug 278161. Actually, the patch here and the patch for bug 278161 have to be landed together (in the branch) to avoid a regression dealt with here.
 

Comment 14

[Jungshik Shin]

Comment on attachment 217136 [details] [diff] [review]
updated ad-hoc patch

Scott, can you approve for the branch when you think that this has baked long enough? 

When I get your approval and approval of Darin for the patch for bug 278161, I'll land both patches together.

Comment 15

[Mike Cowperthwaite]

With TB 3a1-0418, I can verify that it's possible to attach and send a file 
with an ISO-8859-1, supra-ASCII name, in an en-US Windows 2000 install.  Jungshik, is there anything else I should be testing before marking this Verified?  Wait until the patch is in the 1.8 branch?

Names in Chinese, Japanese, Arabic are still problems, but Jungshik opened 
bug 334280 for that.

Comment 16

[Scott MacGregor]

Comment on attachment 217136 [details] [diff] [review]
updated ad-hoc patch

thanks for the trunk testing on this Mike.

I think this patch has baked long enough for us to take it on the branch.

Comment 17

[Jungshik Shin]

Scott, sorry I didn't notice your comment on March 30th and I'm just back. Patch just landed on the 1.8 branch along with the patch for bug 278161

Comment 18

[[e-mail]]

(In reply to comment #17)
>Patch just landed on the 1.8 branch along with the patch for bug 278161
Nit: it would have been nice if you had landed them in the correct order to avoid giving me another heart attack as you turned my tinderbox red again...

Comment 19

[Jungshik Shin]

(In reply to comment #18)

> Nit: it would have been nice if you had landed them in the correct order to
> avoid giving me another heart attack as you turned my tinderbox red again...

Sorry. I should have done that (I intended to land the patch for bug 278161 as soon as I landed this patch, but apparently the delay was too long to avoid turning seamonkey tinderbox red)
 

Comment 20

[Jungshik Shin]

(In reply to comment #15)

> Jungshik, is there anything else I should be testing before marking this
> Verified?  Wait until the patch is in the 1.8 branch?

Thank you for testing, Mike. I'm marking this as verified per your test result.
Your test of 1.8 branch build would be nice, but I don't think it's not a prerequisite for marking it as verified.

Comment 21

[Tony Chung [:tchung]]

changing keyword to verified1.8.1 per comment 20.