58866 - Files are not opened, if the path or file name contains some high-ascii or double-byte characters

Status: VERIFIED INVALID

(Core :: Internationalization, defect, P2)

Description

[Max Sanzharov]

From Bugzilla Helper:
User-Agent: Mozilla/4.76 [en] (WinNT; U)
BuildID:    2000103120

file can not be opened from Windows Explorer, if the path contains, for example, 
trademark character

Reproducible: Always
Steps to Reproduce:
1. set up Mozilla to handle jpeg images (Preferences: Advanced\Desktop 
integration)
2. Quit Mozilla
3. open Windows Explorer, make "temp" subfolder in the root of drive c: (if it 
does not exist)
4. make "™"(trademark character, alt+0153) subfolder in "c:\temp"
5. copy any jpeg file to this subfolder("c:\temp\™")
6. Double-click on this file

Actual Results:  browser displays "c:\temp" directory

Expected Results:  browser displays jpeg image

if the path contains cyrillic letters, actual results are the same (in the 
russian version of Windows NT, SP6a)

Comment 1

[Kathleen :Brade]

This might be Windows-specific.  I don't see a problem opening a gif file in the
browser when the gif file is in a folder named ? (option-2).  I am using
Macintosh ns6 build (2000103114).

Comment 2

[nhottanscp]

Cc IQA people, can anybody reproduce?

Comment 3

[marina]

I am not able to open it by double-clicking but i can do it through browser:
File|Open file (path), it opens in the browser page fine (using Win NT 40)

Comment 4

[Max Sanzharov]

Browser can not open that file, if it is executed by another application (not 
only by Explorer). This is a problem that I wanted to report. Try to start that 
file from Windows Commander or from any shell, that supports Unicode filenames, 
and you will get the same results. Copyright sign produces expected results. But 
the trademark sign, cyrillic letters and some other characters in the path 
produce actual results. It is tested on the Russian WinNT 4.0 Workstation and on 
the English WinNT 4.0 Server.

Comment 5

[nhottanscp]

I can reproduce the trade mark case on Windows 2000 en-US.

Comment 6

[nhottanscp]

Reassign to yokoyama, cc to law.

Comment 7

[Teruko Kobayashi]

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

Comment 8

[Teruko Kobayashi]

Adding nsbeta1 and intl keywords.

Comment 9

[Teruko Kobayashi]

Changed QA contact to [e-mail].

Comment 10

[Roy Yokoyama]

Updating the target milestone.

Comment 11

[Roy Yokoyama]

This looks like problem with not converting to UCS2
before passing to JS.
nsCmdLineService::GetURLToLoad(char ** aResult)
{ return GetCmdLineValue("-url", aResult); }

We may need to change from :
interface nsICmdLineService : nsISupports
{
  string Initialize(in long argc);
  string getCmdLineValue(in string argc);
  readonly attribute string URLToLoad;
  readonly attribute string programName;
  readonly attribute long argc;
  [noscript] readonly attribute charArray argv;
};

to

interface nsICmdLineService : nsISupports
{
  wstring Initialize(in long argc);
  wstring getCmdLineValue(in string argc);
  readonly attribute wstring URLToLoad;
  readonly attribute wstring programName;
  readonly attribute long argc;
  [noscript] readonly attribute charArray argv;
};

Debug tools/trace:
-setting #define STRICT_CHECK_OF_UNICODE in js/src/xpconvert.cpp
 Display assertion for ILLEGAL_CHAR_RANGE [at line 301]
-NS_IMETHODIMP nsDocShell::LoadURI(const PRUnichar* aURI, PRUint32 aLoadFlags)
 already contains invalid PRUnichar aURI.

Comment 12

[Roy Yokoyama]

ylong: Can you test this on Mac and Linux?   
I see different source file names such like nsCommandLineServcie.cpp and
nsCommandLineServiceMac.cpp.   

Comment 13

[Yuying Long]

I don't know why I can't see the comments I added on yesterday(submit by 04-30 
build).  Anyway, here is the result on 04-30 trunk build:
Linux:  When you run: ./netscape file:<path and file name>, then won't open the 
file, and non-ascii path name was garbled in URL field.
Mac: I don't know if there is a way that I can open a file from command line.
Windows: Can not open the file from 
Start menu | Run |  <N6.5 path>\netscp6 <path and file name>
 or double click on the file name.

However, the file was file fine from File | Open File and then click the 
location except the non-ascii path name was display as code like: %D6%D0%CE%C4

Comment 14

[Jaime Rodriguez, Jr.]

Is this for all extended chars, or just TM???

Comment 15

[Roy Yokoyama]

All the extended chars are affected as well as CJK chars.

Comment 16

[Jaime Rodriguez, Jr.]

Roy - What's the user impact of this bug?

Comment 17

[Roy Yokoyama]

Move to 0.9.2 per ftang.

Comment 18

[Roy Yokoyama]

Attachment: Preventing call to AssignWithConversion(fileurl) with corrupts non-ascii file url

Comment 19

[Frank Tang]

so... yokoyama, could you put down some common Japanese usage which will hit 
this problem. Basically, we need some user cases here.

Here is what I can think about:
User using CJK Win95/98/ME create html document in local drive and put into the 
"My Document" folder. In CJK Win95/90/ME the "My Docuemnt" is translate into
Japanese (or Korean/Chinese). User use explore to find it. and then dobule click 
in the exloper. User expect mozilla/Netscape will view them. However, the actual 
result is mozilla/netscape do not open it but open some parent directory.

This is what I can "think about". Can you confirm that is one of the common case 
will happen to Japanese users? If yes, then we should really consider this as 
moz0.9.2

However, when we test the patch, we need to test the following condiction
1. in mozilla, go to the directlry say "file:///d|" in mozilla, double click the 
view in the tree cell to open the file. Try the directory name contains non 
ASCII, try the case the file name contains non ASCII. Try the case both the 
directory and file name contains non ASCII
2. in Explorer, doulble click the file name to view in Netscape.
3. In Mozilla, directly type the URL URL bar and hit return to view it.
4. Use 4.x to view the file, then copy and paste the URL and paste into mozilla 
and try to view it by click return in URL bar.
5. Use IE to view the file, then copy and paste the URL and paste into mozilla 
and try to view it by click return in URL bar.

This particulare bug is talking about case 2. And I believe case 2 is not 
working in 6.0 RTM. However, we also want to make sure this fix does not break 
other condiction, if they are working in 6.0 RTM. For example, I am sure case 1 
is working 

Comment 20

[Frank Tang]

ok, I try case 4 in trunk and netscape 6.0 rtm . Neither work. Hope this patch 
can ALSO fix that problem. Since it is already broken, we should check in the 
current patch even it does not fix case 4. 
But we should still try other cases. 

Comment 21

[Katsuhiko Momoi]

I think this bug presents a big problem. 
Often installing a program creates a desktop 
file (with a shortcut). It could be a README file
or the HELP manual. If a program creates a shortcut
to a file under a non-ASCII named folder, that shortcut
would be useless and would inconvenience users. 
I can think of some programs which bundle Mozilla/Netscape
not working as intended because of this bug.

Comment 22

[Roy Yokoyama]

(ftang using yokoyama's account)
Why this bug is important
For Japanese user:
User move a html file (could be pure english name say aa.html) to the desktop 
on Japanese Win2K/98/95/NT. User double click that aa.html
expect to see aa.html display in mozilla. actual result- mozilla display a 
directory listing .
The reason is the name of "Desktop" in non English OS is localized- For 
example, Roy's desktop folder name is 
"C:\Documents and Settings\yokoyama.JUKEBOX\デスクトップ"

It is a very high visible bug if user cannot double click english file name on 
desktop and view it in mozilla.  (in Japanese or other non English system)

Comment 23

[Roy Yokoyama]

nhotta: can you try my patch on Mac?
bstell: can you try my patch on Linux?
Thanks in advance. :)

Comment 24

[Frank Tang]

mscott- I talk to pdt and selmer and lchiang both suggest you help to review 
this code.

Comment 25

[kill this account]

it compiles and runs on Linux

Comment 26

[Scott MacGregor]

Please get an r= before asking me to do the super review. I'll take a look at it
once you've gotten an initial review. Thanks.

Comment 27

[Frank Tang]

r=ftang. But I also want to make sure nhott review it.
mscott- we are expecting you to do a r= level review, not just sr= review. This 
is a sensitive area. 

Comment 28

[nhottanscp]

That code is weird, it's not executed for Macintosh the function always returns
"NS_ERROR_FAILURE". And this bug doesn't happen on Macintosh. Anyway, the patch
will not affect Macintosh. I will review the patch.

Comment 29

[nhottanscp]

-    nsAutoString uriSpecOut(aStringURI);
+    nsCAutoString uriSpecOut;

Why did the old code intialize the output string, was that necessary?

Comment 30

[Roy Yokoyama]

nhotta: the old code was not necessary.

I needed to change from nsAutoString to nsCAutoString
due to the ConvertStringURIToFileCharset(nsString& aIn, nsCString& aOut)
parameter declaration plus nsCAutoString constructor doesn't take (PRUniChar*).

Comment 31

[nhottanscp]

r=nhotta

Comment 32

[Scott MacGregor]

r=mscott

Comment 33

[Roy Yokoyama]

Attachment: Adding few extra lines of code to fix 86948 (by having PossiblyByteExpandedFileName())

Comment 34

[Roy Yokoyama]

Brief on new patch:
Not match changed except in the case where sometimes we receive
a proper PRUnichar (bug 86948); so we need to distiguish between
the padded locale string and the proper unicode string 
(thus adding PossiblyByteExpandedFileName()). 

Currently, 4 of 5 test cases specified by ftang (2001-06-20 00:11)
doesn't work.  If we apply the new patch, it fixes all the cases.
Previous patch (06/19/01 19:11) only fixed the case 2. ;)

nhotta, ftang, mscott: Please review again.  Thanks

Comment 35

[Frank Tang]

1. I think you should use
+    if ((uniChar[i] >= 0x0080) && (uniChar[i] <= 0x00FF))  {
instead of 
+    if ((uniChar[i] >= 0x80) && (uniChar[i] <= 0xFF))  {

Not sure what 0xFF be extend to when it compare with uniChar[i]. Will it extend 
to 0x00FF or 0xFFFF ? I think make it 0x00FF is safer. 

2. before 
+        if (PossiblyByteExpandedFileName(aIn)) {
please add the following comments:
+        // this is not the real fix but a temporary fix
+        // in order to really fix the problem, we need to change the 
+        // nsICmdLineService interface to use wstring to pass paramenters 
+        // instead of string since path name and other argument could be
+        // in non ascii. Since it is too risky to make interface change right
+        // now, we decide not to do so now.
+        // Therefore, the aIn we receive here maybe already in damage form
+        // (e.g. treat every bytes as ISO-8859-1 and cast up to PRUnichar
+        //  while the real data could be in file system charset )
+        // we choice the following logic which will work for most of the case.
+        // Case will still failed only if it meet ALL the following condiction:
+        //    1. running on CJK, Russian, or Greek system, and 
+        //    2. user type it from URL bar
+        //    3. the file name contains character in the range of 
+        //       U+00A1-U+00FF but encode as different code point in file
+        //       system charset (e.g. ACP on window)- this is very rare case
+        // We should remove this logic and convert to File system charset here
+        // once we change nsICmdLineService to use wstring and ensure
+        // all the Unicode data come in is correctly converted. 

Also file a new bug against yourself to work on the nsICmdLineService changes. 
and include the bug number in the comments above. 

otherwise r=ftang. 

Comment 36

[Frank Tang]

mscott- sorry, can you sr the latest patch ?
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=39433
It is slightly differnt from the last one to address 86948

Comment 37

[Roy Yokoyama]

Attachment: Updated patch as per ftang's recommendation

Comment 38

[Roy Yokoyama]

mscott- can you sr the latest patch ?
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=39517

Comment 39

[Scott MacGregor]

sr=mscott

Comment 40

[chris hofmann]

a=chofmann

Comment 41

[Roy Yokoyama]

checked-in

Comment 42

[Yuying Long]

Mark as verified since now we can open non-ascii file name.

Comment 43

[Rui Xu]

Can still repro with trademark character on Win2K SP2 (build 2001-09-19 
branch), even if it is as a file name. Reopen it.
However, cannot repro with cyrillic letters.

Comment 44

[Frank Tang]

mark it as m0.9.6

Comment 45

[Rui Xu]

It also happened with some double-byte characters.

Change bug title from "files are not opened, if the path contains some 
non-ascii characters" to "Files are not opened, if the path or file name 
contains some high-ascii or double-byte characters".

Comment 46

[Roy Yokoyama]

ruixu: can you show me the steps to reproduce this?
Here is what I've tried on my W2K-CS:
1) File/Open File from menu and opened a chinese html file in a chinese folder
2) type d:\chinese folder\chinese file.html in URL bar
3) type d:\japanese folder\chinese file.html in URL bar.
All of above worked well with 2001/10/15 0.9.4 tree.
 

Comment 47

[Rui Xu]

Yokoyama-san: 
Could you please try those folder names that are marked in the attachment 
58866 [details].jpg?

Steps:
File>Open File from menu and open a file in a folder with the folder name that 
is marked in the attachment 58866 [details].jpg.

Please let me know if you want to copy those test folders from my machine.
 

Comment 48

[Rui Xu]

Attachment: 58866.jpg

Comment 49

[Rui Xu]

On a double byte layout system, like SC Win2K, you might be able to pass 
some double byte test if the DBCS just support certain char. In this case, try 
another double byte layout system, like JA Win2K, or a non-double byte layout 
system, like English Win2K, you should find the problem.

Comment 50

[Katsuhiko Momoi]

Question: with the fix in this bug, do we now support **any**
characters in the path supported on the OS you're on?

So for example, Win 2000, Win XP, and even Win NT4 support
filenamse in Japanese under English NT4. Or Latin 1 accented
characters under Japanese Win 2000. 
I have 2 directories -- one named in Japanese and the other
named with a Latin 1 name (father in French, père) under Win 2000 with the
locale set to Japanese. Under these directories, I have an indential image file:

1. d:\JPN_Name\iamge.jpg
2. d:\père\image.jpg

If I type in the URL in 1 into the location bar, I don't
see the image. But if I type in the URL in 2, I do.

The above Windows platforms and MacOX X support file names
suported in Unicode. In essence, you can name files in any
characters that an input method can enter. Do we support all
such characters in the path URL regardless of the system locale?

Comment 51

[Roy Yokoyama]

Attachment: Call ::GetOpenFileNameW() instead of A() version.

Comment 52

[Roy Yokoyama]

momoi: I dug up little more on this.  

To answer your question of supporting all the characters that OS supports in
Mozilla is not an easy task at this moment.  I've tried (see attached) using 
:GetOpenFileNameW(&ofn); which should return an unicode Pathname/Filename; but
instead I got c:/???/test.html where ??? should be a Japanese foldername
in WinXP-En (system locale is En).  

I believe we need to have an Unicode version of Mozilla to solve this. 
(and lots other unicode related problems)

I am moving the milestone to 1.0

Comment 53

[timeless]

"Microsoft Layer for Unicode on Windows 95/98/Me Systems"
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/win32/unilayer_4wj7.asp

Comment 54

[nhottanscp]

nsbeta1-
 the original problem is fixed, remaining issue is for non default locale support

Comment 55

[Rui Xu]

This bug also happened with CHT chars on CHT WinNT/Win2K, and KOR chars on KOR 
WinNT/Win2K.

Comment 56

[Roy Yokoyama]

Mozilla can't support char outside of its Native CodePage
unless we convert Moz to an Unicode app. ( calling W API won't help )
I believe those CHT chars on CHT WinNT/Win2K are not in the CHT code page.  

Comment 57

[Rui Xu]

I agree with Roy's comments. we really need Moz as an Unicode app.

Comment 58

[Roy Yokoyama]

timeless: thanks for the inof; but MSUL is for *Unicode app* 
running on Win9x.  Moz is not an Unicode app.  

Comment 59

[timeless]

This is extremely silly. Let's convert mozilla into a unicode app.

Comment 60

[Frank Tang]

move to future

Comment 61

[Frank Tang]

problem on SCH OS can be addressed by bug 126744 and problem with korean
characters on KO os can be addressed by bug 126752

Comment 62

[ji]

Renominating.

Comment 63

[Frank Tang]

ji- what is tthe reason let you renominating this bug ? please explain

Comment 64

[ji]

I explained to Frank via email since there are some info I won't reveal here.

Comment 65

[Frank Tang]

ji- I am asking technically why you need this ? Currently, only those characters
cannot be encoded as Locale charset have problem. Why do we need to fix those ?
Do you think people will use those characters as file name ?

Comment 66

[ji]

Is it because mozilla can't handle unicode path? Below are some possible scenarios:
1. On a Japanese system(WinNT/WinXP/W2K), open/save a file with a filename which
contains latin-1 chars.
2. On English XP/W2K + mulitiple langauge UI, when the user switches to Ja UI,
open/save a file with a filename which contains latin-1 chars.
Actually I meant to nominate the generic unicode file path handling problem on
navigator. There is a meta bug 101606 which includes everything: installer,
profile manager and browser.

Do we have a meta bug for browser only? I think fixing unicode path handling
problem on browser is important.

Comment 67

[Rui Xu]

One more scenario. 
In Hongkong, someone may use his Chinese name to create a folder and put some 
files in it, then use Netscape to open or save some files in this folder. So, 
this person may encounter this problem since special Chinese character are 
widely used in Chinese name. The similar case might happen on address, etc.

Comment 68

[Winnie Lam]

topembed+ since this is international related.

Comment 69

[Frank Tang]

with the limitation of the OS, the best case is to fix it for WinNT/XP/2K.
nsbeta1- it because the number of users on WinNT/XP/2K are still relative small
and the percentage of users which will use those file name are also relative small. 
remove topembed+ , please reconsider it as topembed-

Comment 70

[saari (gone)]

topembed-, embeddors shouldn't run into this

Comment 71

[Roy Yokoyama]

Attachment: MOZ_UNICODE patch for nsprpub

use MultiByteToWideChar(CP_UTF8....) for converting
filename from UTF-8 to UCS2 before calling system i/o W-APIs.

Comment 72

[Roy Yokoyama]

Attachment: MOZ_UNICODE patch for widget\nsFilePicker.xxxx

Use W-APIs for commondlg
NS_GETSAVEFILENAMEW() etc

Comment 73

[Roy Yokoyama]

Attachment: MOZ_UNICODE patch for xpcom\nsLocalFile

Modify FStoUCS2() and UCS2toFS() to use UTF-8.

Comment 74

[Roy Yokoyama]

Attachment: MOZ_UNICODE patch for widget

All the widget Windows messages are now in Unicode :)

Comment 75

[Wan-Teh Chang]

Comment on attachment 87715 [details] [diff] [review]
MOZ_UNICODE patch for nsprpub

This patch is basically fine but I saw some
problems that should be fixed.

1. In PR_WIN32_FIND_DATA_TO_WDATA, you have

>+  if (afda->cFileName[0] != '\0') {
>+    fileNameLen = MultiByteToWideChar(CP_UTF8, 0, afda->cFileName,
>+      strlen(afda->cFileName), afdw->cFileName, MAX_PATH);
>+    afdw->cFileName[fileNameLen] = 0;
>+  }

There are two problems with this code.	First,
afda->cFileName[0] may be uninitialized, for
example, when this function is called by
PR_FindFirstFile.  Second, afdw->cFileName is
not set if afda->cFileName[0] is '\0'.

PR_WIN32_FIND_DATA_TO_ADATA has the same problems.

2. Whenever a function fails, you go to UseAAPI
to retry the A version of the function.  I
think this is wrong.  If a function fails, we
should simply return with a failure status.

3. As I explained in email, we can't change PR_OpenFile
and PR_GetFileInfo because we need to maintain backward
compatibility.	We will need to add new UTF-8 variants
of these functions, say PR_OpenFileUTF8 and
PR_GetFileInfoUTF8, that take UTF-8 pathnames.

Comment 76

[Wan-Teh Chang]

Comment on attachment 87715 [details] [diff] [review]
MOZ_UNICODE patch for nsprpub

It turns out that the PR_WIN32_FIND_DATA_TO_WDATA
function is not necessary and should be deleted.
The second argument to FindFirstFileW and FindNextFileW
is an output argument, so it is not necessary to
initialize the WIN32_FIND_DATAW structure (fdw)
before passing it to FindFirstFileW or FindNextFileW.

In PR_WIN32_FIND_DATA_TO_ADATA, the if statement at
the end should have an else clause:

>+  if (afdw->cFileName[0] != '\0') {
>+    fileNameLen = WideCharToMultiByte(CP_UTF8, 0, afdw->cFileName,
>+      wcslen(afdw->cFileName), afda->cFileName, MAX_PATH, NULL, NULL);
>+    afda->cFileName[fileNameLen] = 0;
>+  } else {
>+    afda->cFileName[0] = '\0';
>+  }

It may be a good idea to also convert cAlternateFileName
for completeness although NSPR doesn't use that field.

Comment 77

[Roy Yokoyama]

Attachment: Taking Wan-Teh's advise for NSPR

Wan-Teh: can you quickly take a look and see what you think.

Comment 78

[Roy Yokoyama]

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

Comment 79

[Roy Yokoyama]

Attachment: revised nsprpub io

New UTF-8 nsprpub/io API

Wan-Teh: can you review? Thanks

Comment 80

[Roy Yokoyama]

Attachment: revised. Call PR_xxxxx_UTF8() instead of FS

dougt: can you review this patch?

Comment 81

[Doug Turner (:dougt)]

Comment on attachment 88957 [details] [diff] [review]
revised.  Call PR_xxxxx_UTF8() instead of FS

assuming that nspr adds UTF support, I suppose that this patch is okay.  Where
and why is MOZ_UNICODE defined?  

Where is the Makefile.in patch?  Yes, the windows build can use make.

Comment 82

[Roy Yokoyama]

dougt: thanks for a responsive review. 

>Where and why is MOZ_UNICODE defined?  
Where- MOZ_UNICODE will be defined as a environment variable
Why  - so that we continue to support the currect Win32 build until
       this UNICODE build is ready.  ('ready' means ready for public. 
       We need to add MS Unicode Layer support for Win9x OSs and more
       vigorous testing on file i/o for non-locale filenames.)  
       Other related bugs, see 9449 also.  I am also pursuing to set
       up the Unicode nightly build from tinderbox.
>Where is the Makefile.in patch?  Yes, the windows build can use make.  
       First focus is for the Windows platforms only.  I really don't
       want to bite a big fish here.  I'd like to make smaller steps 
       before we move onto the other platforms.  

Above info is enough for getting /r from you, doug?

Comment 83

[Doug Turner (:dougt)]

There are currently two ways to build Mozilla on win32. You can build using
either nmake or GNU make.  I would like to see a patch for Makefile.in.

Comment 84

[Roy Yokoyama]

>There are currently two ways to build Mozilla on win32
Really. Man, I am learning new stuff everyday. :)
and yes, I found the build process for GNU make for Win32.  
Let me read up on it and I'll create a new patch. Thanks, Doug

Comment 85

[Doug Turner (:dougt)]

Everytime I run make -f client.mk on my windows box, I hope that someday Seawood
will get the courage and time to kill off the nmake builds.  :-)

Comment 86

[Roy Yokoyama]

Attachment: adding MOZ_UNICODE to Makefile.in

It took me few hours to get the GNU make to work.
dougt: can you review?

Comment 87

[Doug Turner (:dougt)]

Comment on attachment 89319 [details] [diff] [review]
adding MOZ_UNICODE to Makefile.in

r=dougt

Comment 88

[Roy Yokoyama]

seawood: can you super review?

Comment 89

[hacker formerly known as [e-mail]]

Comment on attachment 89319 [details] [diff] [review]
adding MOZ_UNICODE to Makefile.in

sr=cls

Comment 90

[Wan-Teh Chang]

Comment on attachment 88956 [details] [diff] [review]
revised nsprpub io

Roy,

Sorry that it took me so long to review this patch.
Thank you for incorporating my previous comments.
This patch is basically good.  My comments below
are on coding style, sharing common code, and
issues not obvious to someone new to NSPR.

1. PR_Open, PR_MkDir, and PR_Stat are deprecated,
so we don't need to add the UTF8 versions of these
three functions.

2. NSPR's naming convention for public functions is
PR_FooBarBaz, so PR_OpenFile_UTF8 should be changed
to PR_OpenFileUTF8, and so on.

3. There are some C++ // comments that should be
changed to C /* */ comments.

4. You need to add the implementations for non-Windows
platforms.  This patch will only compile on Windows.

The non-Windows implementations should simply call
PR_SetError(PR_NOT_IMPLEMENTED_ERROR, 0) and fail.

5. We should try to share code between PR_Foo and
PR_FooUTF8.  The only difference between the two is
that PR_Foo calls _PR_MD_FOO and PR_FooUTF8 calls
_PR_MD_FOO_UTF8.

We should also try to share code between _PR_MD_FOO
and _PR_MD_FOO_UTF8.  Their code is mostly the same.

6. The internal functions in w95io.c, such as
PR_WIN32_FIND_UCS2_DATA_TO_UTF8_DATA, should be
declared 'static'.  We usually use the _pr_ prefix
for static functions.  The PR_ prefix is reserved
for public functions.

7. Please follow the prevalent indentation and braces
style in the files you are modifying.

Comment 91

[Roy Yokoyama]

Attachment: adding Wan-Teh's suggestions

Wan-Teh,

I think I did all the platforms; but I don't have machines to test
my patch.  (patch is 3000 lines of code)  I really hate to ask for
review; but can you?

Can someone also help me compile my patch on other platforms?  
Windows is ok; but I need help on Mac/OS2/Unix/Linux/beos. 

Thanks

Comment 92

[timeless]

what about qnx ;-?

Comment 93

[Roy Yokoyama]

qnx too :)

Comment 94

[Roy Yokoyama]

Attachment: nsLocalFile.mResolvedPath and nsLocalFile.mWorkingPath to have UTF8; not FS

Change of nsLocalFile.mResolvedPath and nsLocalFile.mWorkingPath 
to be stored in UTF-8; not in FS.
Moreover, make nsLocalFile::xxxxNativeyyyy() to return FS; 
and nsLocalFile::xxxxyyyy() to return UCS2

Comment 95

[Sergei Dolgov]

I wonder if i can/should create at last nsLocalFileBeOS...it may be heavily
simplified in comparison with nsLocalFileUnix (which handles BeOS case now).
It may have sence, because there, in BeOS, FS==UTF-8. Forever and ever, without
variants.

Timeless, Wan-Teh, what's your opinion ?

Comment 96

[Frank Tang]

nsbeta1+ for m1.2final

Comment 97

[Roy Yokoyama]

Attachment: Add UCS2 APIs in NSPR

APIs (below) are defined to support for UCS2 filenames.
PR_OpenFileUCS2(), PR_OpenDirUCS2(), PR_ReadDirUCS2(), PR_CloseDirUCS2

Wan-Teh: can you review?

Comment 98

[Roy Yokoyama]

Attachment: Paths are stored in UTF8 of XPCOM/IO

I'll make UCS2 as well for the performance/runtime bloat comparison.

Comment 99

[Roy Yokoyama]

doug: I have spend few hours and realized the XPCOM patch will bloat to 
      over 1400 lines just to make the mWorkingPath and mResolvedPath 
      to nsString.  (I also considered to define new data members, namely 
      mResolvedUnicodePath, mResolvedUnicodePath). 
      I think it will be wise to keep the path in UTF8 format 
      for simplicity and maintenability of the code.

      Can you review the patch?  It is very similar to the one, exceptions of 
      PR_XXXX() changes, recieved blessing from you and and chris with 
      /r and /sr respectively a month ago.
      http://bugzilla.mozilla.org/attachment.cgi?id=93745&action=view

Comment 100

[Doug Turner (:dougt)]

Comment on attachment 93745 [details] [diff] [review]
Paths are stored in UTF8 of XPCOM/IO

Is this correct?

rv =
file->AppendNative(nsDependentCString(NS_ConvertUCS2toUTF8(entry->nameUCS2).get
()));

AppendNative expects a native filesystem char set which may not be utf8, right?
 Why not just use append() which expects ucs2.

Comment 101

[Roy Yokoyama]

Attachment: updating as per dougt's comment

dough: can you review again? Thanks

Comment 102

[Roy Yokoyama]

Attachment: remove bunch of UCS2 defines in primpl.h

wan-teh: while I was updating xpcom, I noticed that there were bunch of 
	 unnecessary #defines in primpl.h.  
	 please review? Thanks

Comment 103

[Doug Turner (:dougt)]

Comment on attachment 94099 [details] [diff] [review]
updating as per dougt's comment

+#ifdef MOZ_UNICODE
+	 *_retval =
PR_OpenFileUCS2(NS_ConvertUTF8toUCS2(mResolvedPath.get()).get(), flags, mode);
+#else

mResolvedPath.get() is suppose to be a native path, not a utf8 path.  right?

Comment 104

[Roy Yokoyama]

>mResolvedPath.get() is suppose to be a native path, not a utf8 path.  right?
No, we wanted to have mResolvedPath/mWorkingPath to be either UTF8 or UCS2.
However, I realized that converting mResolvedPath/mWorkingPath to UCS2, 
namely nsString, will add significant number of changes to XPCOM/IO.
Thus, we store mResolvedPath/mWorkingPath to be in UTF8. You can see
FStoUCS2() and UCS2toFS() doesnt' call MultibyteToWide. Instead we call
NS_ConvertUCS2toUTF8().

Comment 105

[Roy Yokoyama]

Comment on attachment 94099 [details] [diff] [review]
updating as per dougt's comment

doug: There are other places
      I need to change. please stay tuned for update.

Comment 106

[Roy Yokoyama]

Comment on attachment 94100 [details] [diff] [review]
remove bunch of UCS2 defines in primpl.h

Seperate bugs are created:
1) NSPR  UCS2 APIs 
  (162358)
2) XPCOM/IO changes
  (162361)

The same patch is
attached to 162358.
Marking this an obsolete.

Comment 107

[Roy Yokoyama]

Marking this an obsolete.

Comment 108

[Yuying Long]

Mark as verified per comment above, there are some other bugs filed for handle
the problem.

Comment 109

[Boris Zbarsky [:bzbarsky]]

So are the uri fixup hacks this bug added still necessary in light of the more
recent work?  They are blocking some other changes to URI fixup to make it saner...

Comment 110

[Roy Yokoyama]

>They are blocking some other changes to URI fixup
Please dont' wait for my changes.  It's still a long way to go before I 
can check them in.

Comment 111

[Boris Zbarsky [:bzbarsky]]

No, you misunderstand.  As far as I can tell, making the changes I want to URI
fixup to make it better is _impossible_ without getting this issue resolved. 
Doing so would regress this bug.  So when I say blocking I mean blocking.

So could you actually answer my question?  Now that the appshell does explicit
encoding conversion on the URI argument, is the hack in URI fixup still
necessary?  I would test myself, but I do not have an internationalized Windows
system (or any Windows system for that matter) on hand.

Comment 112

[Jungshik Shin]

I'm wondering why this bug was marked 'invalid'. I can't still open files with
characters outside the repertoire of the current system locale on Win2k. I set
the system locale to Greek and can't open files with Russian/Korean(put your
favoirte language here :-)) names.