312287 - Cannot open local htm file if its path has non-ASCII character / Cannot open IDN from command line

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Arkady Sherman]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051010 SeaMonkey/1.1a
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051010 SeaMonkey/1.1a

Uable to open local file with path "C:\Documents and Settings\Геннадий
Евтюгин\Мои документы\article base\index.htm" from command shell.

Reproducible: Always

Steps to Reproduce:
1. Double clicking on he file "C:\Documents and Settings\Геннадий Евтюгин\Мои
документы\article base\index.htm" runs seamonkey, but doesn't open the file.
2. Second double clicking on he file (when seamonkey is already open) produces
the error: File Not Found
The file /c:/Documents and Settings/%C3%E5%ED%ED%E0%E4%E8%E9
%C5%E2%F2%FE%E3%E8%ED/%CC%EE%E8 %E4%EE%EA%F3%EC%E5%ED%F2%FB/article
base/index.htm cannot be found. Please check the location and try again.
and string in the URL bar:
file:///c:/Documents%20and%20Settings/%25C3%25E5%25ED%25ED%25E0%25E4%25E8%25E9%20%25C5%25E2%25F2%25FE%25E3%25E8%25ED/%25CC%25EE%25E8%20%25E4%25EE%25EA%25F3%25EC%25E5%25ED%25F2%25FB/article%20base/index.htm
3. If I open the file form menu File->Open file ... everything is alright with
string in the URL bar:
file:///C:/Documents%20and%20Settings/%C3%E5%ED%ED%E0%E4%E8%E9%20%C5%E2%F2%FE%E3%E8%ED/%CC%EE%E8%20%E4%EE%EA%F3%EC%E5%ED%F2%FB/article%20base/index.htm
Actual Results:  
Errors opening files form command shell

Expected Results:  
This shuold be corrected.

Comment 1

[Elmar Ludwig]

Dup of bug 162361 (via bug 294446).

*** This bug has been marked as a duplicate of 162361 ***

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

no, because according to comment 0, file|open works.

Comment 3

[Adam Guthrie]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051013
Firefox/1.6a1 ID:2005101303

Opening a file by double clicking gives: 
"Firefox can't find the file at /C:/downloads/????????????/index.html."

File > Open doesn't work either. It shows: 
"Firefox can't find the file at /C:/downloads/____________/index.html." and
doesn't open the file.

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

it all depends on what windows version you use. Mozilla only supports filenames
in the system charset. So on russian windows, file|open will be able to open
files with russian names. on english windows, russian filenames will not work,
no matter how you try to open it. bug 162361 is for this issue.

This bug is about the case that double-clicking a file doesn't work even if it
is in the system charset.

that said, this might be a duplicate of bug 311387?

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

No, this is not a dup of bug 311387. This is another problem.
I tried with Japanese name path. I cannot open the file by double click.(When
Seamonkey doesn't launch.) Seamonkey opened a ancestor folder that is not having
non-ASCII char. I cannot reproduce with Firefox.

> File > Open doesn't work either. It shows: 
> "Firefox can't find the file at /C:/downloads/____________/index.html." and
> doesn't open the file.

This is your system location's charset doesn't support the character.
If Mozilla cannot convert Unicode to file system charset, it returns '_'.

Comment 6

[Arkady Sherman]

Probably that will help you to recognize the reason of the bug:
when error produced (after double clicking) the URL bar contains translated
string -  %25C3%25E5%25ED%25ED%25E0%25E4%25E8 ...
but if the file has been successfully opened through File->Open file the URL bar
contains - %C3%E5%ED%ED%E0%E4%E8
As you can see the difference is additional "25" before each translated symbol.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

This regression is made between 9/27 and 9/28.
I think cause by bug 58523.

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv1.0

This fixes the case of when Seamonkey is not started.
But "double escaped problem" that is the case of Seamonkey is already started
is not fixed by this patch.

Comment 9

[[e-mail]]

For that you'll need to figure out nsNativeAppSupportWin.cpp. OS/2?

You'll probably need to convert OpenBrowserWindow to work in Unicode.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Neil:

Sorry, I cannot understand your comment. What you mean?

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Another regression is made between 7/28 and 7/29.
Maybe, caused by bug 300485.

Comment 12

[[e-mail]]

Ah, maybe it would be easier to make GetURLToLoad return an AString instead?

Comment 13

[[e-mail]]

Apologies, but I meant that I think you will need to change the OpenWindow and
OpenBrowserWindow functions in nsNativeAppSupportWin.cpp to use AString.

Comment 14

[Christian :Biesinger (don't email me, ping me on IRC)]

I think we should switch more command-line handling to AString, and maybe also use 
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dllproc/base/getcommandline.asp
which also exists on 9x

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #12)
> Ah, maybe it would be easier to make GetURLToLoad return an AString instead?
I don't think so. Because if we make file:// URL, we need native code string.

(In reply to comment #13)
> Apologies, but I meant that I think you will need to change the OpenWindow and
> OpenBrowserWindow functions in nsNativeAppSupportWin.cpp to use AString.
Why? We can handle with nsACString. I think that it is enough until bug 278161
is fixed.

I will recreate new patch. Thanks,

Comment 16

[[e-mail]]

(In reply to comment #14)
>I think we should switch more command-line handling to AString
Sure, but we've still got to handle natively encoded DDE on 9x.

>(In reply to comment #13)
>>Apologies, but I meant that I think you will need to change the OpenWindow and
>>OpenBrowserWindow functions in nsNativeAppSupportWin.cpp to use AString.
>Why? We can handle with nsACString.
To get from the result of GetURLToLoad to something OpenBrowserWindow can use
you need to call NS_CopyNativeToUnicode which gives you an AString; one other
caller is a C literal which could become an NS_LITERAL_STRING and the other
caller already has an AString and currently converts it to a char* in UTF8.

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #16)
> >(In reply to comment #13)
> >>Apologies, but I meant that I think you will need to change the OpenWindow and
> >>OpenBrowserWindow functions in nsNativeAppSupportWin.cpp to use AString.
> >Why? We can handle with nsACString.
> To get from the result of GetURLToLoad to something OpenBrowserWindow can use
> you need to call NS_CopyNativeToUnicode which gives you an AString; one other
> caller is a C literal which could become an NS_LITERAL_STRING and the other
> caller already has an AString and currently converts it to a char* in UTF8.

But I don't change the caller of OpenBrowserWindow. Why I need to change?
Is it another refactoring issue?

Comment 18

[[e-mail]]

(In reply to comment #17)
>But I don't change the caller of OpenBrowserWindow. Why I need to change?
To fix the case of Seamonkey already started that is not fixed by your patch.

Comment 19

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv2.0

This fixes both problem.

Comment 20

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #18)
> (In reply to comment #17)
> >But I don't change the caller of OpenBrowserWindow. Why I need to change?
> To fix the case of Seamonkey already started that is not fixed by your patch.

I don't think so. See latest patch.

Currently, we don't use correct the nsIURI. If args is file path(not file://),
nsIURI has escaped file path(not file://). In |nsIBrowserDOMWindow::openURI|, it
is used for escaped spec only. So, if it is file path(not file://), the escape
process is run again.

Therefore, we need to convert file path to URI of file:// before calling
|nsIBrowserDOMWindow::openURI|.

Comment 21

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 199730 [details] [diff] [review]
Patch rv2.0

why not use NS_NewNativeLocalFile?

and you should probably use nsDependentCString instead of nsCAutoString here:
+	     nsCAutoString path( args );
+	     rv = file->InitWithNativePath( path );

Comment 22

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv2.1

You're right. Thank you for your advice!

Comment 23

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv2.2

Sorry, I have a mistake.

Comment 24

[[e-mail]]

(In reply to comment #20)
>Therefore, we need to convert file path to URI of file:// before calling
>|nsIBrowserDOMWindow::openURI|.
Isn't that outside the scope of this bug? This bug is about OpenWindow and
OpenBrowserWindow sometimes being passed native and sometimes UTF-8 encoding.
But if you want to make file paths work, then use nsIURIFixup instead.

Comment 25

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

> Isn't that outside the scope of this bug? This bug is about OpenWindow and
> OpenBrowserWindow sometimes being passed native and sometimes UTF-8 encoding.

I don't think so. This regression has two bugs. This bug is about those problems.
1. The assumed encoding is wrong in |DoCommandLines|.
2. Invalid URI is created by native file path in |OpenBrowserWindow|.

My patch only fixes these mistakes.

Comment 26

[[e-mail]]

(In reply to comment #25)
>I don't think so. This regression has two bugs. This bug is about those problems.
>1. The assumed encoding is wrong in |DoCommandLines|.
>2. Invalid URI is created by native file path in |OpenBrowserWindow|.
My point is that HandleRequest calls OpenBrowserWindow in two places, once using
the natively encoded request, and once using the Unicode home page.

Comment 27

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 199747 [details] [diff] [review]
Patch rv2.2

O.K. I understand. I'll recreate the patch.

Comment 28

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv3.0

This patch fixes "cannot open IDN from command line" problem too.
# I cannot find the bug in bugzilla.

Comment 29

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv3.1

Comment 30

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv3.2

Comment 31

[[e-mail]]

Comment on attachment 199812 [details] [diff] [review]
Patch rv3.2

I don't claim to be an expert in Unicode URIs, so my comments might easily be
completely wrong.

>+#include "nsILocalFile.h"
>+#include "nsNetUtil.h"
Last time I tried to include nsNetUtil it complained because it was trying to
pull in string support from xpcom.dll which isn't allowed in startup code.

>   nsCOMPtr<nsISupportsCString> sarg(do_CreateInstance(NS_SUPPORTS_CSTRING_CONTRACTID));
>-  if (sarg)
>-    sarg->SetData(nsDependentCString(args));
>+  if (sarg) {
>+    nsCAutoString args;
>+    if ( IsFilePath( aArgs ) || IsFileSchemeURL( aArgs ) ) {
>+      NS_CopyUnicodeToNative( aArgs, args );
>+    } else {
>+      CopyUTF16toUTF8( aArgs, args );
>+    }
>+    sarg->SetData(args);
>+  }
I'm not sure you have to do this, as I think the URI might get fixed up by the
browser UI. If this is the case then you can switch to an nsISupportsString
instead.

>+        if ( IsFilePath( aArgs ) ) {
>+            nsCOMPtr<nsILocalFile> file;
>+            rv = NS_NewLocalFile( aArgs,
>+                                  PR_FALSE,
>+                                  getter_AddRefs( file ) );
>+            if ( NS_FAILED(rv) || !file ) {
>+                break;
>+            }
>+            rv = NS_NewFileURI( getter_AddRefs( uri ), file );
>+        } else {
>+            nsCAutoString args;
>+            if ( IsFileSchemeURL( aArgs ) ) {
>+                NS_CopyUnicodeToNative( aArgs, args );
>+            } else {
>+                CopyUTF16toUTF8( aArgs, args );
>+            }
>+            rv = NS_NewURI( getter_AddRefs( uri ), args, nsnull );
>+        }
I wonder whether we should be using nsIURIFixup::CreateFixupURI here instead.

Comment 32

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch rv4.0

Comment 33

[[e-mail]]

Comment on attachment 199956 [details] [diff] [review]
Patch rv4.0

This patch looks nice :-)

Comment 34

[Jungshik Shin]

Comment on attachment 199956 [details] [diff] [review]
Patch rv4.0

r=jshin

Comment 35

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

checked-in to Trunk.

-> FIXED

Comment 36

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch for check-in

Comment 37

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Neil or Someone:

Will SeaMonkey 1.0 be released from 1.8 branch?
If so, we need to check-in to 1.8 branch too.

Comment 38

[[e-mail]]

I'm not sure, because bug 58523 did not go in on the branch.

Comment 39

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Umm... But we have fixed bug 300485 on 1.8 branch....

Comment 40

[[e-mail]]

I guess this and 301073 need to go in, but I'm still not sure about 58523.

Comment 41

[Stefan A. Möller]

> Will SeaMonkey 1.0 be released from 1.8 branch?
> If so, we need to check-in to 1.8 branch too.

At least this bug is still present in the latest 1.8 nightlies, which are named SeaMonkey 1.0b (Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8) Gecko/20051210 SeaMonkey/1.0b).

Comment 42

[[e-mail]]

The patch as provided can't go on the branch, because it depends on other patch(es), probably 58523, which still has some unsolved regression(s).

Comment 43

[Robert Kaiser]

This occurs with quite rare actions, and the branch situation is a bit unclear, as it seems (modified patch needed at least). As 1.0 is nearing fast, not holding back that release for that bug. If a branch patch comes up, would be nice to take it for 1.1 at least, possibly even 1.0.x

Comment 44

[Jungshik Shin]

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

Comment 45

[moz]

Attachment: Screenshot of the difference between addresses

Comment 46

[moz]

As I stated in https://bugzilla.mozilla.org/show_bug.cgi?id=334473  ,
let's notice [HINT, HINT :-)] this never happened (*) with Mozilla 1.7.X  , and always happened (**) in Seamonkey 1.0b, 1.0.0 and 1.0.1

(*) at least in all the computers I've seen with Mozilla 1.7.X in windows 2000 in Spanish
(**) at least in the 2 computers in windows 2000 in Spanish   where I've tried Seamonkey versions.

P.S. Maybe we can't say "resolved fixed" if we are releasing Seamonkeys with this error on it?

Comment 47

[[e-mail]]

We all know the regression occurred trying to improve other aspects of startup.
By fixed, we mean that we have a solution. We just haven't released it. We are also unlikely to release it for any 1.0.x release. If you want to see it in 1.1 rather than waiting for 1.5 it might help to request blocking-seamonkey1.1a?

Comment 48

[moz]

> If you want to see it in 1.1
Mmmm...  

it is a very important bug, to the point that 
- It annoys me every day when opening docs 
- I can not recomend Seamonkey to users and friends (though I use Seamonkey)
- I have to be careful when an Internet Explorer user is near so he does not see this

> it might help to request blocking-seamonkey1.1a?
I can not do it, so I have to ask you all so that non-English-speaking people can install Seamonkey and open files without problems.

Thank you for Seamonkey!

Comment 49

[Jungshik Shin]

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

Comment 50

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I cannot work on this for branch. If we need to work on branch, please take this bug for branch.

Comment 51

[Phoenix]

I've tested yesterday nightly build of seamonkey-1.5a, in some cases problem still exists. When I opeping file with «» and cyrillic symbols in name or in path, error "File Not Found" appear. In URL bar I see
file:///M:/Pers/text/Comps/!B@0=K%20IT/%C2%ABTest%C2%BB.htm
instead of
file:///M:/Pers/text/Comps/%D0%A1%D1%82%D1%80%D0%B0%D0%BD%D1%8B%20IT/%C2%ABTest%C2%BB.htm

Comment 52

[Robert Kaiser]

Would be nice to have for SeaMonkey 1.1 if anyone can find the time to port the patch over to it, but even though it may not be a nice thing to have, we can live with having that bug in another release, so not blocking one for that issue.