364285 - appending "-n" suffix to downloaded files with duplicate names breaks unicode filename support

Status: RESOLVED FIXED

(Toolkit :: Downloads API, defect, P4)

Description

[utf16]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20061217 Minefield/3.0a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20061217 Minefield/3.0a1

1. Downloaded files that contain Unicode characters in their names can be correctly opened only once by an external application.

2. Files with Unicode characters in their paths can be opened only by a default application (other programs will announce that "File not found").

Reproducible: Always

Steps to Reproduce:
Problem nr. 1:

1. Click on the given link.
2. Choose your default application to open the file. (The application must support Unicode, OpenOffice is not the best choice. If it is your default program for DOC files, temporarily set Notepad, Wordpad or Word to open them).
3. Any Unicode-friendly application should have opened the file with correct Cyrillic letters in its name.
4. Repeat first two steps.

Problem nr. 2:

1. Save the above given file as it appears in Cyrillic.
2. Locate it somehow via Firefox (by dragging and dropping, by pressing "Ctrl+O" or by locating it through a built-in explorer).
3. In the dialog box choose to open it with any other, but not your default program (Notepad, Wordpad, Word or any other that supports Unicode).
Actual Results:  
Problem nr. 1:

No matter which program you choose, the file will open as "____________________.doc".

Problem nr. 2:

External application that is not set to default for this filetype will not be able to open the file.

Expected Results:  
Prob. 1 and 2:

The file should be opened as "Извещ № 81-транспорт.doc".

Firefox v2.0 cannot save files with Unicode characters if downloaded via dialog box, this is fixed in v3.0α.

Comment 1

[Volkmar Kostka]

FYI: Original discussion is here:
http://forums.mozillazine.org/viewtopic.php?t=498123

Comment 2

[obval]

Same problem occurs with Apache mod_rewrite applied on non ASCII characters on localhost (not checked for client side)

Comment 3

[utf16]

I think I understand what's wrong here. I've just tried using Thunderbird, and it gives the exat same results with files that contain Unicode characters. But there is a slight detail I never paid attention to: when I open the same file for the second time, its name is slightly different - it has a numer in its end like "-1.doc". The more times you open the file, the higher is the number.

There must be a supplementary function that renames the files appending the necessary number when it encounters a file with the same title in a temporary folder. And this function is clearly not widestring compatible.

Comment 4

[Stephen Donner [:stephend] Not actively reading bugmail]

Confirming with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a9pre) Gecko/2007092804 Minefield/3.0a9pre

When I open http://ivadm.ivanovo.ru/portal/request.nsf/38c42d7b098cf39fc3257105003998ed/70cade6ae52a6d89c32572170027b38d/$FILE/%D0%98%D0%B7%D0%B2%D0%B5%D1%89%20%E2%84%96%2081-%D1%82%D1%80%D0%B0%D0%BD%D1%81%D0%BF%D0%BE%D1%80%D1%82.doc using Notepad on Windows Vista, I see the disparity between the first time opened and consecutively opened files.

(See attached screenshot.)

Comment 5

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Screenshot, JP2 format

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

I've morphed the summary to reflect comment 3; please re-morph if I misunderstood. 

Comment 7

[utf16]

It should be "to downloaded files". The world "files" is missing.

Comment 8

[Simon Montagu :smontagu]

There are actually two separate bugs here.
1) As comment 3 says, nsLocalFile::CreateUnique uses the lossy GetNativeLeafName
2) In the "open the file with non-default application" codepath, nsProcess::Run calls CreateProcess with ASCII filename instead of CreateProcessW with Unicode filename

Comment 9

[Simon Montagu :smontagu]

Attachment: Patch for issue #1

This fixes the issue with CreateUnique, which is seen when opening a file for the second time with the default application. It also fixes two or three other issues that I noticed while debugging:

The current code to truncate an over-long filename only kicks in when the sequence number is added, i.e. if the original name is not too long, but adding the sequence number would push it over the edge. If the original name is too long, trying to open the file fails silently. Neil pointed out to me on IRC that it also checks the length of the leaf name rather than the whole path name, which is Just Wrong.

I also check when truncating the filename that the truncation takes place at character boundaries in UTF-8 or UTF-16, rather than codepoint boundaries.

Comment 10

[Simon Montagu :smontagu]

Attachment: Patch for issue #2

This fixes the issue with nsProcess::Run, which is seen when opening a file with a non-default app, even for the first time.

Comment 11

[utf16]

Will that handle the same bug in Thunderbird?

Comment 12

[Simon Montagu :smontagu]

Thunderbird has a bug in attaching files with unicode names, which I'll have to fix before I can test opening attachments.

Comment 13

[Simon Montagu :smontagu]

Attachment: Correct patch for issue #1

Sorry, the previous attachment is an early version of the patch.

Comment 14

[Simon Montagu :smontagu]

The Thunderbird bug with attaching files with unicode names is bug 234681

Comment 15

[Simon Montagu :smontagu]

Of course, nobody says I have to use Thunderbird to send the attachment ;-)

I sent myself an attachment with another client, and I can confirm that the patch here fixes the bug in opening the attachment from Thunderbird.

Comment 16

[utf16]

That's just great! I can't wait the patch being applied to the next release.

By the way, the Thunderbird bug with attaching files with unicode names is also https://bugzilla.mozilla.org/show_bug.cgi?id=332110

Comment 17

[Benjamin Smedberg]

Comment on attachment 294584 [details] [diff] [review]
Correct patch for issue #1

r+, except that I can't let this land without a unit test

Comment 18

[Benjamin Smedberg]

Comment on attachment 294479 [details] [diff] [review]
Patch for issue #2

This isn't correct... LaunchWithIProcess isn't expecting UTF8, it's expecting the native charset.

To do this correctly, you really would need nsIProcess.runw or to make nsIProcess accept UTF8 instead of native.

Comment 19

[Simon Montagu :smontagu]

Attachment: Patch for issue #1 with unit test

Comment 20

[Simon Montagu :smontagu]

Comment on attachment 294479 [details] [diff] [review]
Patch for issue #2

(In reply to comment #18)
> (From update of attachment 294479 [details] [diff] [review])
> This isn't correct... LaunchWithIProcess isn't expecting UTF8, it's expecting
> the native charset.
> 
> To do this correctly, you really would need nsIProcess.runw or to make
> nsIProcess accept UTF8 instead of native.
> 

I've split that out into bug 411511.

Comment 21

[Simon Montagu :smontagu]

Attachment 295625 [details] [diff] checked in. Just a reminder that this only fixes the case of opening files with the default application. Bug 411511 will fix the case of opening with a non-default application.