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

VERIFIED FIXED

Status

MailNews Core
Composition
--
major
VERIFIED FIXED
12 years ago
10 years ago

People

(Reporter: Mike Cowperthwaite, Assigned: Jungshik Shin)

Tracking

({intl, regression, verified1.8.1})

Trunk
x86
Windows 2000
intl, regression, verified1.8.1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

1.75 KB, patch
Scott MacGregor
: review+
Bienvenu
: superreview+
Scott MacGregor
: approval-branch-1.8.1+
Details | Diff | Splinter Review
(Reporter)

Description

12 years ago
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.
(Reporter)

Comment 1

12 years ago
This may be closely related to bug 234681.

See also bug 332110.
(Assignee)

Comment 2

12 years ago
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. 
Assignee: nobody → jshin1987
Keywords: regression
Summary: Attaching Unicode-named file (via File Open dialog) fails: substitutes '_' characters in name, won't send → can't send a mail with a file whose name is non-ASCII
(Reporter)

Comment 3

12 years ago
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.
(Assignee)

Comment 4

12 years ago
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|.
Status: NEW → ASSIGNED
Summary: can't send a mail with a file whose name is non-ASCII → can attach but cannot send a file with non-ASCII name

Comment 5

12 years ago
Dupe of/related to bug 329129?
(Assignee)

Comment 6

12 years ago
Created attachment 216942 [details] [diff] [review]
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()|...
(Assignee)

Comment 7

12 years ago
(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. 
(Reporter)

Comment 8

12 years ago
(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).
(Assignee)

Comment 9

12 years ago
Created attachment 217136 [details] [diff] [review]
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?
Attachment #216942 - Attachment is obsolete: true

Comment 10

12 years ago
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
Blocks: 332751
(Assignee)

Comment 11

12 years ago
*** Bug 332751 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

12 years ago
No longer blocks: 332751
(Assignee)

Comment 12

12 years ago
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.
Attachment #217136 - Flags: superreview?(bienvenu)
Attachment #217136 - Flags: review?(mscott)

Updated

12 years ago
Attachment #217136 - Flags: superreview?(bienvenu) → superreview+

Updated

12 years ago
Attachment #217136 - Flags: review?(mscott) → review+
(Assignee)

Comment 13

12 years ago
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.
 
Blocks: 278161
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
No longer depends on: 33451
Resolution: --- → FIXED
Summary: can attach but cannot send a file with non-ASCII name → can attach but cannot send a file with non-ASCII name even in the file system encoding
(Assignee)

Comment 14

12 years ago
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.
Attachment #217136 - Flags: approval-branch-1.8.1?(mscott)
(Reporter)

Comment 15

12 years ago
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

12 years ago
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.
Attachment #217136 - Flags: approval-branch-1.8.1?(mscott) → approval-branch-1.8.1+
(Assignee)

Comment 17

12 years ago
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
Keywords: fixed1.8.1

Comment 18

12 years ago
(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...
(Assignee)

Comment 19

12 years ago
(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)
 

(Assignee)

Comment 20

12 years ago
(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.
Status: RESOLVED → VERIFIED

Comment 21

11 years ago
changing keyword to verified1.8.1 per comment 20.
Keywords: fixed1.8.1 → verified1.8.1
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.