Last Comment Bug 162361 - Unicode file i/o in XPCOM/IO (cannot open files whose names contain characters outside the current locale: e.g. Japanese/Chinese on French Windows)
: Unicode file i/o in XPCOM/IO (cannot open files whose names contain character...
Status: RESOLVED FIXED
: fixed1.8.1, intl
Product: Core
Classification: Components
Component: Internationalization (show other bugs)
: Trunk
: x86 Windows XP
P2 normal with 19 votes (vote)
: mozilla1.8.1
Assigned To: Jungshik Shin
: Yuying Long
: Makoto Kato [:m_kato]
Mentors:
: 188383 226928 243558 253164 266718 279224 294914 296316 297304 306335 310394 315353 316168 368647 (view as bug list)
Depends on: mzlu 326168 331433 331453 332123
Blocks: 160236 169712 235385 58866 66041 88292 100344 108000 129736 166735 172337 174734 192154 193032 193358 194067 202366 211961 228437 234946 262922 273225 294914 326544 330668
  Show dependency treegraph
 
Reported: 2002-08-12 14:06 PDT by Roy Yokoyama
Modified: 2010-09-13 06:21 PDT (History)
69 users (show)
chofmann: blocking‑aviary1.0-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Supporting NSPR-UCS2. Storing mWorkingPath and mResolvedPath to be in UTF8 (45.22 KB, patch)
2002-09-04 18:14 PDT, Roy Yokoyama
no flags Details | Diff | Splinter Review
store mWorkingPath and mResolvedPath in UTF8 and call PR_fooUCS2() (2.69 KB, patch)
2002-11-01 15:55 PST, Roy Yokoyama
doug.turner: review+
kinmoz: superreview+
Details | Diff | Splinter Review
another patch (11.08 KB, patch)
2003-06-08 03:05 PDT, Jungshik Shin
no flags Details | Diff | Splinter Review
a new patch (33.83 KB, patch)
2003-06-09 04:54 PDT, Jungshik Shin
no flags Details | Diff | Splinter Review
another experimental patch (it's working) (43.07 KB, patch)
2003-06-13 16:50 PDT, Jungshik Shin
no flags Details | Diff | Splinter Review
MZLU (proof of concept) (14.69 KB, application/octet-stream)
2004-06-16 04:19 PDT, Brodie
no flags Details
MZLU v0.1 (20.43 KB, application/octet-stream)
2004-06-30 00:16 PDT, Brodie
no flags Details
patch (far far from review-ready) (57.34 KB, patch)
2006-01-11 07:00 PST, Jungshik Shin
no flags Details | Diff | Splinter Review
patch (stage 1) (82.73 KB, patch)
2006-01-13 09:11 PST, Jungshik Shin
no flags Details | Diff | Splinter Review
another checkpoint (nsLocalFileWin) (85.14 KB, patch)
2006-01-14 15:09 PST, Jungshik Shin
no flags Details | Diff | Splinter Review
yet another checkpoint (with support for Win 9x/ME) (142.46 KB, patch)
2006-01-19 20:13 PST, Jungshik Shin
no flags Details | Diff | Splinter Review
patch confirmed to work on Windows ME (155.45 KB, patch)
2006-01-22 20:24 PST, Jungshik Shin
no flags Details | Diff | Splinter Review
1.8.x branch patch (148.84 KB, patch)
2006-01-27 05:10 PST, Jungshik Shin
no flags Details | Diff | Splinter Review
patch that really works on Windows ME (161.57 KB, patch)
2006-02-05 22:31 PST, Jungshik Shin
no flags Details | Diff | Splinter Review
another update(getting closer) (154.51 KB, patch)
2006-02-12 09:23 PST, Jungshik Shin
no flags Details | Diff | Splinter Review
another update (should work on Win95) (162.17 KB, patch)
2006-02-17 15:07 PST, Jungshik Shin
no flags Details | Diff | Splinter Review
patch for 1.8.x branch (159.03 KB, patch)
2006-02-18 09:00 PST, Jungshik Shin
no flags Details | Diff | Splinter Review
patch for trunk (161.72 KB, patch)
2006-02-18 09:09 PST, Jungshik Shin
VYV03354: review-
Details | Diff | Splinter Review
a partial patch for bug 278161 necessary for testing my patch here (13.27 KB, patch)
2006-02-18 09:13 PST, Jungshik Shin
no flags Details | Diff | Splinter Review
trunk patch update addressing issues pointed out in comment #109 (163.95 KB, patch)
2006-02-18 21:35 PST, Jungshik Shin
no flags Details | Diff | Splinter Review
branch patch update (161.25 KB, patch)
2006-02-18 21:37 PST, Jungshik Shin
no flags Details | Diff | Splinter Review
patch updated (for a new patch for bug 326168) (150.14 KB, patch)
2006-03-09 18:50 PST, Jungshik Shin
no flags Details | Diff | Splinter Review
patch updated (for a new patch for bug 326168) (150.14 KB, patch)
2006-03-09 18:50 PST, Jungshik Shin
darin.moz: review-
Details | Diff | Splinter Review
review comments from darin on attachment 214628 (16.66 KB, text/plain)
2006-03-15 13:13 PST, Darin Fisher
no flags Details
trunk patch addressing Darin's review comment (149.46 KB, patch)
2006-03-19 18:40 PST, Jungshik Shin
no flags Details | Diff | Splinter Review
updated trunk patch to make it work on real Win 9x/ME (149.98 KB, patch)
2006-03-20 08:04 PST, Jungshik Shin
darin.moz: review+
Details | Diff | Splinter Review
patch for 1.8 branch (148.88 KB, patch)
2006-03-21 01:39 PST, Jungshik Shin
no flags Details | Diff | Splinter Review
updated trunk patch (NS_StartupWinAPIs is now gone) (148.30 KB, patch)
2006-03-21 15:08 PST, Jungshik Shin
benjamin: review+
darin.moz: superreview+
Details | Diff | Splinter Review
1.8 branch patch updated (no more NS_StartupWinAPis) (147.29 KB, patch)
2006-03-21 23:44 PST, Jungshik Shin
no flags Details | Diff | Splinter Review
Patch for mingw-header include/w32api/winver.h (1.52 KB, patch)
2006-03-22 09:19 PST, Hans-Andreas Engel
no flags Details | Diff | Splinter Review
fix a "typo" in attachment 215818 (878 bytes, patch)
2006-03-23 21:18 PST, Jungshik Shin
darin.moz: review+
darin.moz: superreview+
Details | Diff | Splinter Review
updated branch patch (with a 'typo' fixed, regression taken care of) (149.08 KB, patch)
2006-03-25 18:38 PST, Jungshik Shin
no flags Details | Diff | Splinter Review
fix mingw bustage v1 (3.02 KB, patch)
2006-03-28 09:35 PST, cls
no flags Details | Diff | Splinter Review
branch patch with follow-up patches combined (149.59 KB, patch)
2006-04-04 19:09 PDT, Jungshik Shin
darin.moz: approval‑branch‑1.8.1+
Details | Diff | Splinter Review
Fix my branch tinderbox (3.42 KB, patch)
2006-04-09 08:53 PDT, neil@parkwaycc.co.uk
jshin1987: review+
darin.moz: superreview+
darin.moz: approval‑branch‑1.8.1+
Details | Diff | Splinter Review
1st screenshot (58.27 KB, image/jpeg)
2006-04-10 15:29 PDT, Michael Osipov
no flags Details
2nd screenshot (55.70 KB, image/jpeg)
2006-04-10 15:30 PDT, Michael Osipov
no flags Details
3nd screenshot (38.85 KB, image/jpeg)
2006-04-10 15:31 PDT, Michael Osipov
no flags Details

Description User image Roy Yokoyama 2002-08-12 14:06:04 PDT
This is a split from 58866.

We need to store filenames in Unicode (either in
UTF8 or UCS2) and call a set of new NSPR_UCS2 interfaces.
Comment 1 User image Rui Xu 2002-08-12 14:24:50 PDT
code issue, QA to yokoyama@netscape.com for now, please reassign for QA.
Comment 2 User image Roy Yokoyama 2002-09-04 18:14:38 PDT
Created attachment 97885 [details] [diff] [review]
Supporting NSPR-UCS2.  Storing mWorkingPath and mResolvedPath to be in UTF8

Phew,  It's more than what I have initially thought; but I think
I covered the most of cases and ready for review.

dougt: can you review?
Here is the run down:
- store mWorkingPath and mResolvedPath in UTF8
- call new PR_fooUCS2() instead
- Get/SetNativeFoo() converts path by calling UTF8toFS and FStoUTF8
respectively
Comment 3 User image Frank Tang 2002-09-26 09:46:35 PDT
===================================================================
RCS file: /cvsroot/mozilla/xpcom/io/nsDirectoryService.cpp,v
+  #ifdef MOZ_UNICODE
...
+    if ( ::GetModuleFileNameW(0, buf, sizeof(buf)) ) {
..
+  #else
...
     if ( ::GetModuleFileName(0, buf, sizeof(buf)) ) {

Does GetModuleFileNameW exist on Win95/98/ME ? 
If NO, will this put into #ifdef cause load time error with the #ifdef turn on
under Win95/98/ME ? 
Does it function on Win95/98/ME ? 
If No, will this put into #ifdef always return false under Win95/98/ME ? 

Same question for
 _wstat in   void nsFileSpec::GetModDate, nsFileSpec::GetFileSize(),
nsFileSpec::IsFile(), nsFileSpec::IsDirectory() 
::ShellExecuteW in nsLocalFile::Launch()

Comment 4 User image Roy Yokoyama 2002-10-18 11:28:35 PDT
The strategy changed since I posted the patch on 09/04.
We intended to have a pure unicode application and use
MS Layer for Unicode for Win9x OS.  
However, we decided _not_to use MSLU. 

I'll post a new patch to accomodate the change of strategy.
Comment 5 User image Roy Yokoyama 2002-11-01 15:55:15 PST
Created attachment 104906 [details] [diff] [review]
store mWorkingPath and mResolvedPath in UTF8 and call PR_fooUCS2()

Last patch was rotten so I need to redo the patch.....
I'd like to provide an incremental patch for supporting file i/o issues.

With this patch we can:
- open non-locale file 

dougt: can you review?	I want to check this as soon as we are open for moz 1.3
Comment 6 User image Doug Turner (:dougt) 2002-11-01 16:09:13 PST
Comment on attachment 104906 [details] [diff] [review]
store mWorkingPath and mResolvedPath in UTF8 and call PR_fooUCS2()

r=dougt
Comment 7 User image Roy Yokoyama 2002-11-08 15:14:43 PST
bulk milestone change
Comment 8 User image kinmoz 2002-11-18 17:03:58 PST
Comment on attachment 104906 [details] [diff] [review]
store mWorkingPath and mResolvedPath in UTF8 and call PR_fooUCS2()

sr=kin@netscape.com


==== Put a space after the equals sign:


+    output =NS_ConvertUCS2toUTF8(input);


==== So how does this relate to bug 170852, where you are actually removing
MOZ_UNICODE ifdefs? Are you going to remove the MOZ_UNICODE ifdefs for this
patch, when you land those changes?
Comment 9 User image Jungshik Shin 2003-06-08 03:05:13 PDT
Created attachment 125174 [details] [diff] [review]
another patch 

This patch is based on Roy's patch, but it checks the OS version at the
run-time and calls either UTF16'nized PR_File* APIS or non-UTF16nized PR_File*
APIs. This is not optimized, but is just to show how it can be done. Because
this patch relies on NSPR UTF-16 APIs, NSPR  has to be compiled with
MOZ_UNICODE defined.
Currently UTF-16 APIs are only compiled in with MOZ_UNICODE defined. Can we
turn them on by default on Windows? 

Perhaps, a better approach is to check the OS (9x/ME vs 2k/XP) in xpcom/io and
then to set function pointers accordingly to points to non-UTF16 calls and
UTF-16 calls as was done in widget. 

Whichever way we can take, we  can solve most problems in xpcom/io.

Below is a bit off-topic.

However, PR_*File* APIs are also used directly (not via xpcom/io).  
To fix those cases, we may have to modify NSPR file-related APIs so that on
Windows, file paths in C-string are always interpreted as in UTF-8.  With this
change,  NSPR can internally convert UTF-8 to UTF-16 and invoke 'W' APIs on
Win2k/XP (and on Win9x/ME with Microsoft layers for Unicode, the presence of
which has to be detected when NSPR is initialized) while on Win9x/ME with MSLU
UTF-8 has to be converted to the system code page and 'A' APIs have to be
invoked. It might not be very  realistic because NSPR is not only for Mozilla
but also for other projects. Nonetheless, this is something we have to think
about.
Comment 10 User image Jungshik Shin 2003-06-09 04:54:19 PDT
Created attachment 125219 [details] [diff] [review]
a new patch

I need to check this out on Win9x/ME, but on Win2k, it works fine. 
At the beginning of the patch is a small fix to downloadmanager.js (see bug
208113 comment #8). 

In SpecialSystemDirectory.cpp, I switched to 'W' APIs and added emulators for
them on Win 9x/ME. The approach is similar to what's done in widget/src/windows
by Roy.

This patch also exposes NS_IsWindowsNT() that is available once xpcom is
init'd. Its two applications outside xpcom/io are in xpinstall and
netwerk/base/src where	0x5c is special-cased on DBCS OS. Because with this
patch, native path will be in UTF-8 on Win2k/XP. I changed those checks done
only if the OS is Win 9x/ME. 

I can remove MOZ_UNICODE define in Makefile.in in xpcom if UTF16 APIs in NSPR
are turned on by default for XP_WIN.
Comment 11 User image Jungshik Shin 2003-06-13 16:50:42 PDT
Created attachment 125603 [details] [diff] [review]
another experimental patch (it's working)

This works rather well. I can make and read files of which names include Thai,
Devanagari, Greek, Cyrillic letters altogether !

However, this is still experimental (especially, 'extern nsNativeToUnicode() is
a hack I just played with that I'm gonna get rid of). In terms of actual
implementation, I still need to figure out what the best course is. I'm
wondering how good/bad an idea is to have something like nsWin32API::WinAPIName
(that is accessible  across the tree). This is to avoid some overlap with
what's done in widget/src/windows/nsToolkit.*
Comment 12 User image neil@parkwaycc.co.uk 2003-07-20 12:28:07 PDT
Comment on attachment 125603 [details] [diff] [review]
another experimental patch (it's working)

Bah, this conflicts with an old (but unfortunately no sr) patch in bug 156422.

I wonder what the point of MakeUpperCase is.
Comment 13 User image Jungshik Shin 2003-07-21 19:25:24 PDT
Neil, thanks for the note about bug 156422. It seems easy to make attachment
94582 [details] [diff] [review] (to bug 156422) 'Unicode-aware'. I can make a new patch with attachment
94582 [details] [diff] [review] incorporated if you want.  

As for |MakeUpperCase|, I have little idea other than it's probably to make
filenames case-insensitive on Windows. This bug is not for doing something new,
but for making Mozilla 'unicode-aware' on Win2k/XP so that I just recast
whatever is there in 'W" APIs. If we don't need it, that's nice
Comment 14 User image Dean Tessman 2003-07-21 19:29:35 PDT
The Windows filesystem already is case-insensitive.
Comment 15 User image Jungshik Shin 2003-07-21 19:37:57 PDT
I know that[1] and thought it's a bit strange to have MakeUpperCase. Anyway,
please don't ask me about MakeUpperCase. I just recast it  in W APIs without
thinking much. In this bug, I just want to focus on making Win32 file I/O in
Mozilla Unicode-aware. We can remove it later if it's not necessary. 

[1] Case-insensitivity beyond US-ASCII is not so simple as one may think.  
Comment 16 User image Christian :Biesinger (don't email me, ping me on IRC) 2003-11-27 05:16:30 PST
*** Bug 226928 has been marked as a duplicate of this bug. ***
Comment 17 User image timeless 2003-11-27 10:25:53 PST
err. FAT is case preserving. NTFS can be case sensitive. Please don't ever
change the file name's case from the flavor the os offers.
Comment 18 User image Dean Tessman 2003-11-27 13:59:53 PST
Really?  Where is NTFS case-sensitive?
Comment 19 User image Christian :Biesinger (don't email me, ping me on IRC) 2003-11-27 15:02:39 PST
google found: http://techsupt.winbatch.com/TS/T000001036004F26.html
"Although NTFS does support case sensitive filenames, currently only the POSIX
subsystem uses case sensitive names."
Comment 20 User image Christian :Biesinger (don't email me, ping me on IRC) 2003-11-27 15:42:04 PST
also, there's a flag to CreateFile that allows to create two files with names
differing only in case:
FILE_FLAG_POSIX_SEMANTICS
Comment 21 User image Jungshik Shin 2003-11-27 15:49:18 PST
Is there anyway for us to move this forward? We need to make changes in NSPR and
I have yet to hear from wtc. It's embarrasing thta Mozilla on win32 is not yet
fully Unicode-aware. I have a patch and an idea, but without NSPR fix, not much
can be done.

Comment 22 User image Jungshik Shin 2003-12-11 23:24:01 PST
Adding leaf  for nspr, darin for xpcom and some drivers to Cc, retargetting and
assigning to myself.

Related threads:  (in n.p.m.nspr)
1.  Enabling NSPR UTF16 APIs

  news://news.mozilla.org:119/bfe3c5$4ha2@ripley.netscape.com

2. making |nsWindowsAPI| 
news://news.mozilla.org:119/bfr3cc$nfg1@ripley.netscape.com

I really love to fix this (and a bunch of other related bugs that will be fixed
or become very easy to fix once this is fixed), but I'm stuck because I don't
know how to move forward necessary NSPR changes. Any thought or help would be
appreciated. 
Comment 23 User image Jungshik Shin 2003-12-26 02:22:44 PST
It's easier to use google than 'news' URL.

http://groups.google.com/groups?threadm=bfe3c5%244ha2%40ripley.netscape.com
http://groups.google.com/groups?selm=bfr3cc%24nfg1%40ripley.netscape.com&rnum=2

btw, I wrote to wtc about NSPR Unicode file I/O and getenv/setenv vs
wgetenv/wsetenv (bug 227500)
Comment 24 User image Jungshik Shin 2004-02-21 04:32:30 PST
(In reply to comment #22)

> 2. making |nsWindowsAPI| 
> news://news.mozilla.org:119/bfr3cc$nfg1@ripley.netscape.com
> http://groups.google.com/groups?selm=bfr3cc%24nfg1%40ripley.netscape.com&rnum=2

I've just found |nsINativeAppSupportWin|. It seems that we can put all those W/A
API-related stuffs there including IsWindowsNT()?

Comment 25 User image Christian :Biesinger (don't email me, ping me on IRC) 2004-02-22 14:25:42 PST
(In reply to comment #24)
> I've just found |nsINativeAppSupportWin|. It seems that we can put all those W/A
> API-related stuffs there including IsWindowsNT()?

making large parts of the tree depend on xpfe sounds like no good idea to me
Comment 26 User image Jungshik Shin 2004-02-22 14:37:39 PST
I agree. I didn't realize that it's in xpfe. What I want to have/implement is
something akin to |nsCRT| for Windows APIs. See my news posting mentioned in
comment #23.


Comment 27 User image Christian :Biesinger (don't email me, ping me on IRC) 2004-05-13 15:36:05 PDT
*** Bug 243558 has been marked as a duplicate of this bug. ***
Comment 28 User image Brodie 2004-06-16 04:19:16 PDT
Created attachment 150920 [details]
MZLU (proof of concept)

Here is a proof of concept library for wrapping Win32 calls. It is a very basic
implementation similar to MSLU but freely licenced for Mozilla and other open
source projects under the tri-licence. (I've thus called it MZLU).

Essentially what it does is define symbols like:
  GetTempPathMz 
  CreateFileMz
which are function pointers to functions which have the same signature as the W
version of the WIN32 or CRT API. A client program needs to link with the very
slim C static lib that defines these pointers. The only other thing the static
lib does is have an initialization function to set the pointers to the
appropriate implementation. Hard to tell exactly how much overhead in linked
code, but probably just a few Kb.

On Windows NT based OS the pointers are set to the real W functions.
  e.g. CreateFileMz = ::CreateFileW; 
Thus the penalty for using this library on NT/2K/XP is very low. Just the extra
few Kb for the static lib, plus one more indirection per OS call. 

On 9x/ME systems, these functions are set to wrapped versions of the ansi code
page based API. These wrapped functions don't exist in the C static lib, but
instead exist in a dynamically loaded DLL loaded only by 9x/ME systems. 

I thought about using direct calls to the W functions and on Win9x/ME rewriting
the import tables to point to our wrapped functions, which would save the
single indirection on NT, however the current method allows us to also support
functions which are only conditionally available. e.g. GetDiskFreeSpaceEx (not
available on Win95). On systems which doesn't have it available, the function
pointer will be left NULL. On systems which does support it the function
pointer is set. Thus simplifying the test for availability (although at the
cost of having the DLL loaded from startup, however since these are probably
all major system DLLs which are likely to be loaded anyhow, it is just a
mapping into our process that will be created).

The wrapper functions are implemented to convert between wide/narrow chars
while retaining the function specification. Sometimes this is simple (e.g.
DeleteFile), sometimes more difficult (e.g. _wgetdcwd or GetTempPath). See the
DLL implementation (mzlua) for details.

The functions currently implemented for proof of concept are CRT and WIN32
functions that are used in nsLocalFileWin.cpp ...

    _wgetdcwd
    CopyFile
    CreateDirectory
    CreateFile
    DeleteFile
    GetDiskFreeSpaceEx
    GetDiskFreeSpace
    GetFileAttributes
    GetLogicalDriveStrings
    GetTempPath
    MoveFile
    RemoveDirectory
    ShellExecute

In order to use this library to fully Unicode enable Mozilla on Windows. All
calls to non-Unicode API would need to be re-routed through this library. In
addition, all NSPR UCS2 functions would also need to use this library to allow
them to be called on all OS versions.

Is this acceptable to the Mozilla and NSPR philosophy?
Comment 29 User image Darin Fisher 2004-06-16 08:19:22 PDT
Brodie: Thanks for taking the time to create MZLU.  I don't have a "final"
answer for you, but I think this may be the right direction.  The challenge is
really deciding what we want to do with NSPR.  I haven't heard a final decision
there.  I know that wtc is not thrilled about adding new APIs to NSPR, but
perhaps that is the best solution.  We need to hear from wtc on this matter. 
Jshin: what are your thoughts on this?  I know you spoke about wanting something
like MZLU... is this duplicating work you have already done, or is this the
missing puzzle piece?
Comment 30 User image Brodie 2004-06-21 09:13:33 PDT
It seems strange to not use the MSLU when it provides the vendor recommended 
solution of Unicode on all platforms. I've looked at bug 118013 and bug 162362 
which asserts that there was discussion about this but doesn't note where. Can 
someone point me towards the problems with bundling MSLU with Mozilla? Licence-
wise it seems that one clause might be the problem...

"(c) you distribute your Application containing the Redistributable Components 
pursuant to an End-User License Agreement (which may be "break-the-
seal", "click-wrap" or signed), with terms no less protective than those 
contained herein;". 

Is it possible to create another product which is compatible with the terms of 
the MSLU licence, and then require users to install it in order for Mozilla to 
be used on the appropriate platforms? Not particularly nice, but MSLU is a good 
solution...

If not, then I am quite happy to move all of nsToolkit into MZLU, complete the 
implementation of all API that NSPR requires, and then get all of Moz using 
this. I really want to implement all of the Unicode support here. Working with 
MBCS is a major pain and makes things far more complex than it needs to be.

As for the problem of NSPR and it's API. Perhaps we should just use UTF-8 in 
all NSPR API on Windows (build option of ANSI or UTF-8 to maintain 
compatibility) and just wear the to/from UTF-8/UTF-16 conversions that it 
requires? No API change required, a new build option and different 
implementation internally. Wan-Teh -> what do you think? Others?

I know this doesn't make much difference to English speaking single byte "pizza 
is as international as I get" sort of people, but lets try and get this app 
fully internationalized for the rest of the world... Unicode isn't the future, 
it's already here. Mozilla has fallen behind.
Comment 31 User image Jungshik Shin 2004-06-21 11:07:40 PDT
(In reply to comment #29)
 
Brodie, thank you for 'waking up' darin's interest in this bug :-)

> The challenge is
> really deciding what we want to do with NSPR.  I haven't heard a final decision
> there.  I know that wtc is not thrilled about adding new APIs to NSPR, but
> perhaps that is the best solution.  

I tend to agree about adding new APIs. An alternative (using UTF-8 on Win 2k/XP)
is a bit risky as discussed in the thread at 

http://groups.google.com/groups?threadm=bfe3c5%244ha2%40ripley.netscape.com

> We need to hear from wtc on this matter.

Unfortunately, he hasn't replied to my emails. I've written to him (and leaf,
the other owner of NSPR) at least a few times since summer 2003, but nothing
came back. I understand that he's busy, but it's frustrating not to hear
anything.  Anyway, I really hope this time around he can find some time to
resolve this long standing issue. 

  
> Jshin: what are your thoughts on this?  I know you spoke about wanting something
> like MZLU... is this duplicating work you have already done, or is this the
> missing puzzle piece?

 What's missing is, as you wrote, wtc's decision as to what to do with NSPR file
APIs.   MZLU can solve the problem I talked about in two news postings (comment
#23) and it can simplify a lot of things. I was approaching the problem from a
different angle (although equivalent) and didn't hit upon an idea of rolling out
our own version of MSLU.  

Then, a question arises why not just use MSLU. I asked the question before, but
no resolution has been reached yet. 

It's available on virtually all installations of MS Windows because MS IE comes
with it (we don't have to increase the code size to dupe what's already
available). There's an irony here in that we have to require MS IE be installed
to run Mozilla. However, some other parts of Mozilla already rely on DLLs that
come with MS IE so that relying on MSLU should not be a problem, IMHO.  Even if
MS IE is not installed, it's likely that some other applications (e.g. MS
Office) with MSLU are present. 

I'm adding some more people who may be at a better position to resolve this
issue (whether or not to make Mozilla depend on MSLU). 
Comment 32 User image Jungshik Shin 2004-06-22 17:55:59 PDT
In bug 239279, we discussed the possibility of making Mozilla depend on MSLU.
There are quite a number of bugs nothing to do with file I/O  that can be easily
fixed by using 'W' APIs available for Win9x/ME via MSLU. (see bug 232969, bug
240272, bug 9449, bug 243618 for instance). All those bugs are independent of 
NSPR file I/O APIs.So, let's keep on discussing, in bug 239279, as to whether we
want to make Mozilla depend on MSLU or we want to implement our own (MZLU).
Comment 33 User image Brodie 2004-06-30 00:16:15 PDT
Created attachment 151998 [details]
MZLU v0.1

This version of MZLU implements all of the functions that nsLocalFileWin uses
and that are implemented in nsToolkit/nsWindowsAPI. While we are arguing
over/waiting to hear whether or not to use the actual MSLU library or not,
let's start using MZLU instead. 

It will be very easy to move from calling MZLU functions to calling actual MSLU
wide functions (replace the Mz postfix from all functions with W).

There seems to be a number of places where people are actively adding new
wrappers for A/W, so we could at least move that to a centralized place.

On nsLocalFileWin.cpp, I found that I could get rid of nearly every call to
NSPR apart from PR_Open by going directly to the Win32 API. Since this is a
Windows only component I don't see any reason why we shouldn't do so. This
means that at least for the file handling, we only need PR_Open to support
unicode...
Comment 34 User image Darin Fisher 2004-06-30 08:28:41 PDT
You could also implement PRFileDesc yourself in terms of the WIN32 API, but that
is probably not ideal ;-)

The biggest concerns I have with moving to a full Unicode backend is what to do
with the "native" character encoding defined by nsIFile/nsILocalFile.  If we
keep that using the ANSI codepage, then won't we have a lossy conversion from
nsIFile to file:// URL?  Remember, that file:// URLs are currently generated
from the "native" file path.

I think we might want to solve these problems by using UTF-8 as the "native"
character encoding under Win32 builds.  Or, perhaps we could even have an
optional runtime mode in which that is enabled.

We also have to keep in mind that changing the encoding of file:// URLs affects
interoperability with other applications when users drag-n-drop file:// URLs
from Mozilla to other applications.  We should choose an encoding that is most
compatible with Unicode aware applications.  E.g., how does Microsoft Office
encode non-ASCII file:// URLs?

NOTE: Under most Linux distros, UTF-8 is the default character encoding, and it
is therefore what we use for file:// URLs and nsIFile::nativePath under Linux. 
The same is true of OSX.
Comment 35 User image Christian :Biesinger (don't email me, ping me on IRC) 2004-06-30 10:19:58 PDT
(In reply to comment #34)
> I think we might want to solve these problems by using UTF-8 as the "native"
> character encoding under Win32 builds.

the downside is that mozilla is not backwards-compatible wrt to its file urls
then. for example, sucks if you had a homepage (as a local file) in a directory
containing non-ascii characters before, because the url would no longer work.
Comment 36 User image Darin Fisher 2004-06-30 11:05:19 PDT
Christian: I agree... that's a major concern.  Perhaps we need to utilize a
failover technique.  Or maybe we can unescape file:// URLs and test whether they
are UTF-8 or not (using IsUTF8).  If they are not UTF-8, then we try using the
ANSI codepage.  That might be the best solution.  We could probably do all of
this inside nsURLHelperWin.cpp.
Comment 37 User image Brodie 2004-06-30 23:58:15 PDT
Comment on attachment 151998 [details]
MZLU v0.1

See bug 239279 for updates of MZLU.
Comment 38 User image Jungshik Shin 2004-09-09 15:13:35 PDT
*** Bug 253164 has been marked as a duplicate of this bug. ***
Comment 39 User image chris hofmann 2004-09-28 21:26:42 PDT
jshin,  any progress on this one?  if there still is a chance of a low risk
patch renominate for 1.0
Comment 40 User image Jungshik Shin 2004-09-29 00:13:40 PDT
Sorry I don't have enough time to fix this before 1.0. Even if I have a lot more
time, the patch may be too extensive to be regarded as safe for 1.0. 
Comment 41 User image Jungshik Shin 2004-10-29 10:07:07 PDT
*** Bug 266718 has been marked as a duplicate of this bug. ***
Comment 42 User image OstGote! 2005-01-21 04:20:41 PST
*** Bug 279224 has been marked as a duplicate of this bug. ***
Comment 43 User image Markus Essl 2005-01-28 04:07:45 PST
I am using Mozilla code as a base for an application for e-learning, for 
already some years. Right now we are having troubles because customers in 
russia and polen have problems because of this issue, so I created an own class 
that converts (and creates) the unicode path and converts it to its alternate 
path, and uses this alternate short path to create a NS_NewLocalFile, so that 
PR_Open works correctly. Of course, this is far far away from what should be, 
but so I get my code working and customers satisfied. 

But what about the idea of converting the path to its short variant? The only 
downside is, as far as i can see, is the creating of new files, which will not 
be made with NSPR. But it would be possible to create a shortnamefile with 
nspr, and then rename it. 

BTW, the current solution of converting characters to "_" because "?" is not a 
valid filename is bad too - because of this, if you are going to save a page in 
mozilla (1.7.5), you can select a directory with russian characters, and it 
ends up in a totally different folder. Which is, in my opinion, even worse than 
disallowing such pathnames. 

Is there any way that I can be helpful to you? I would like to help out, just 
tell me what to do.
Comment 44 User image timeless 2005-01-28 04:18:10 PST
it's possible for shortfile variants to be disabled, which means you'll gain
nothing by trying.
Comment 45 User image Markus Essl 2005-01-28 11:53:40 PST
You are right; I was not aware of this. I just read
http://support.microsoft.com/kb/q121007/. 
Comment 46 User image Roy Yokoyama 2005-03-04 11:33:17 PST
I started fixing the unicode file i/o related bugs since 2002 and as far as 
I remember, only stuff waiting for supporting the _full_ unicode in Windows 
is to enable NSPR to call W APIs.  (except that non-ASCII Commandline issue is 
still outstanding, i believe)  I believe I have tested file:// URL and 
drag-drop of non-system filenames before I checked in the code (all with 
MOZ_UNICODE)

There appears to have new path to use MZLU; but bug 239279 turning up to a 
licensing talk.  We also discussed using MSLU years back and decided not to use
it simply because we don't want to introduce another dependency.

Can we turn on the flag (MOZ_UNICODE) in NSPR after we branch out 
for Gecko1.8? 

Darin, Chris? Is WTC still active in mozilla? 
Comment 47 User image Jungshik Shin 2005-03-04 17:46:35 PST
See comment #9, comment #10, comment #11 and comment #22 (two news postings and
responses to them in the newsgroup). Anyway, people don't seem to like
introducing new 'Wide' APIs to NSPR. 
Comment 48 User image Jungshik Shin 2005-05-17 05:45:43 PDT
*** Bug 188383 has been marked as a duplicate of this bug. ***
Comment 49 User image Jungshik Shin 2005-05-21 10:14:04 PDT
darin, what's your opinion of introducing wide APIs to NSPR? 
Alternatives are: 
  1. implementing equivalents of NSPR APIs in xpcom with Windows W APIs
  2. pass UTF-8 (on Windows 2k/XP) to existing NSPR APIs and let NSPR do the   
     conversion 

I'm afraid the second alternative has a performance issue because we have to go
back and forth between UTF-8 and UTF-16 on Win 2k/XP.

 
Comment 50 User image Darin Fisher 2005-05-21 10:44:50 PDT
I'm in favor of adding UTF-16 APIs to NSPR.  I'd like to see us move
nsNativeCharsetUtils into NSPR, and make those conversion routines publicly
available.  I spoke with WTC about this plan, and he agreed that it is the best
way to go.  Neither of us have time to implement it though.  The biggest
challenge, I think, is in supporting those UTF-16 APIs on non-Win32 platforms. 
That's where the code in nsNativeCharsetUtils (or some large part of it) will
come in handy.
Comment 51 User image Darin Fisher 2005-05-21 10:45:56 PDT
One more point: I don't think NSPR should depend on MZLU or something like that
since there really aren't that many wide APIs to implement.
Comment 52 User image Jungshik Shin 2005-05-21 11:22:19 PDT
(In reply to comment #50)
> I'm in favor of adding UTF-16 APIs to NSPR.  I'd like to see us move
> nsNativeCharsetUtils into NSPR, and make those conversion routines publicly
> available.  I spoke with WTC about this plan, and he agreed that it is 
> the best way to go.

 So, WTC has changed his mind since 2003. That's fine because I also agree with
you.    

> challenge, I think, is in supporting those UTF-16 APIs on non-Win32 platforms. 

  We don't need those APIs on platforms other than Windows at least for now. If
implementing NSPR 'W' APIs only on Windows for now is acceptable, I'll begin
with attachment 125603 [details] [diff] [review].

Comment 53 User image Darin Fisher 2005-05-21 12:18:01 PDT
I would really like to see us try to provide the same NSPR API on all platforms.
 I think we can given the code in nsNativeCharsetUtils.cpp.
Comment 54 User image Wan-Teh Chang 2005-05-21 14:30:48 PDT
Yes, let's make the current PR_xxxUTF16 functions official,
and avoid making NSPR depend on MZLU (at least in the first
implementation).
Comment 55 User image Roy Yokoyama 2005-05-21 20:50:09 PDT
PR_xxxUTF16 functions are #ifdef'ed with MOZ_UNICODE.
All NSPR needs to do is to enable the flag.

From the comment in prdir.c : 
Bug 162358: added NSPR file I/O functions that take UTF16 pathnames. The patch
is contributed by Roy Yokoyama <yokoyama@netscape.com>. Modified Files:
config/config.mk prio.h prtypes.h _win95.h primpl.h prdir.c prfile.c w95io.c ptio.c
Comment 56 User image Jungshik Shin 2005-05-21 21:13:36 PDT
(In reply to comment #53)
> I would really like to see us try to provide the same NSPR API on all platforms.
>  I think we can given the code in nsNativeCharsetUtils.cpp.

As you implied, moving nsNativeCharsetUtils to NSPR is more involved than simple
copy'n'paste partly because of the language difference. For the sake of
completenetss, it's good to provide the same NSPR API on all platforms, but do
we really want to hold this Windows-specific bug just for that? How about just
adding a 'dummy' implementation for other platforms for now? (there's no
non-Windows consumer at the moment).  

Comment 57 User image Darin Fisher 2005-05-21 22:09:56 PDT
I'm not against doing things in stages, but we need to decide what form we want
this to be in when Firefox 1.1 ships.  Maybe WTC has some thoughts on this?  I'd
love to see this bug fixed for Firefox 1.1, but we're talking about a pretty
significant API change for NSPR.
Comment 58 User image Jungshik Shin 2005-05-21 22:27:41 PDT
(In reply to comment #57)
> but we're talking about a pretty significant API change for NSPR.

We're not gonna remove existing file APIs (char-based), are we? UTF-16 APIs are
not likely to be used by non-Windows consumers

Btw, what do you think of |if (isNT) use UTF-16 APIs else use 8bit APIs| in file
operations? We can move some of them to NSPR (and probably get rid of them there
using function pointer indirection), but I'm afraid we have to live with some of
them. File operations are  more critical than registry handling performance-wise
so that you might have some concern..
 
Comment 59 User image Darin Fisher 2005-05-21 22:50:53 PDT
> We're not gonna remove existing file APIs (char-based), are we?

No, definitely not.


> UTF-16 APIs are not likely to be used by non-Windows consumers

I suspect they will be used by Mozilla.  Why not?  People work with UTF-16
strings a lot in Mozilla, and having a consistent (Portable!) API is what NSPR
is designed for.  It's pretty odd to have Windows-only APIs exported by NSPR.


> Btw, what do you think of |if (isNT) use UTF-16 APIs else use 8bit APIs| in 
> file operations? We can move some of them to NSPR (and probably get rid of 
> them there using function pointer indirection), but I'm afraid we have to live 
> with some of them. File operations are  more critical than registry handling 
> performance-wise so that you might have some concern..

I don't think it matters that much either way.  That said, I think the current
code that uses GetProcAddress to resolve CreateFileW and friends is wrong. 
Those symbols exist on Windows 9x, but they are simply not implemented.

The current NSPR implementation for these routines assumes that callers will
handle "not implemented" errors appropriately.  I'm not sure I like that.  I'd
prefer to see us implement the UTF-16 functions in all cases.  However, I
suppose we could entertain the idea of making all users of the UTF-16 routines
know how to failover to the ANSI versions.  But, that is the least desirable
solution IMO.
Comment 60 User image Jungshik Shin 2005-05-21 23:29:11 PDT
(In reply to comment #59)
> > We're not gonna remove existing file APIs (char-based), are we?
> 
> No, definitely not.

 The question was rhetorical one because adding UTF16 APIs didn't seem to me as
significant as you think it is.

> > > UTF-16 APIs are not likely to be used by non-Windows consumers
> 
> I suspect they will be used by Mozilla.  Why not?  People work with UTF-16
> strings a lot in Mozilla, and having a consistent (Portable!) API is what NSPR
> is designed for.  It's pretty odd to have Windows-only APIs exported by NSPR.

I'm not saying we will never implement them on other platforms. We need to
implement them on all platforms to use them in cross-platform code which
directly invoke NSPR APIs instead of going through xpcom/io (so my statement
'not likely to be .... other platforms' is not quite right). However, there's no
consumer outside xpcom/io so that I think we can do it in stages adding a very
prominent warning that UTF-16 APIs should not be used on other platforms for now.

> I don't think it matters that much either way.  That said, I think the current
> code that uses GetProcAddress to resolve CreateFileW and friends is wrong. 
> Those symbols exist on Windows 9x, but they are simply not implemented.

I'll change it to check the OS version and do the 'right' thing depending on the
result. 

>  I'd prefer to see us implement the UTF-16 functions in all cases.

So would I. That means, xpcom/io can move some of |if (isNT) ...| over to NSPR.  
Comment 61 User image Brodie 2005-05-22 17:12:40 PDT
It is quite simple to rewrite all of xpcom/io to use direct windows calls apart
from a single call to PROpen. I did this once when experimenting with a unicode
build of mozilla. Not really much point though because the requirement for
Unicode support in moz spreads out far more than just xpcom/io.

Part of the solution is that NSPR needs a wide-string version of the API. I
agree that it needs to be cross-platform, and that it should be implemented in
stages. However what we need to avoid is the case when only stage 1 is ever
done, and the other platforms are never coded up. Cross-platform must include
Windows 9x platforms.

At the higher level, I still think that any solution which involves doing
if(isNT) in application code is wrong. It makes the application more complicated
and clutters the logic. The handling of wide-string to legacy encoding on 9x
platforms should be handled seamlessly by a library. Ideally that library is MS
unicows, and I still believe that we can find a solution to distribute it with
the official mozilla build. Otherwise use opencow (nee MZLU,
http://opencow.sourceforge.net/).
Comment 62 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2005-06-01 20:18:41 PDT
*** Bug 296316 has been marked as a duplicate of this bug. ***
Comment 63 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2005-06-01 20:18:53 PDT
*** Bug 294914 has been marked as a duplicate of this bug. ***
Comment 64 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2005-06-11 10:33:53 PDT
*** Bug 297304 has been marked as a duplicate of this bug. ***
Comment 65 User image Adam Guthrie 2005-08-29 15:16:55 PDT
*** Bug 306335 has been marked as a duplicate of this bug. ***
Comment 66 User image Jungshik Shin 2005-09-29 19:01:05 PDT
*** Bug 310394 has been marked as a duplicate of this bug. ***
Comment 67 User image Elmar Ludwig 2005-10-13 06:10:59 PDT
*** Bug 312287 has been marked as a duplicate of this bug. ***
Comment 68 User image Jungshik Shin 2005-11-08 00:12:00 PST
*** Bug 315353 has been marked as a duplicate of this bug. ***
Comment 69 User image Ray Booysen 2005-11-08 00:14:42 PST
*** Bug 315353 has been marked as a duplicate of this bug. ***
Comment 70 User image :Mook 2005-11-12 03:02:51 PST
*** Bug 316168 has been marked as a duplicate of this bug. ***
Comment 71 User image Syophone 2005-11-12 04:18:38 PST
(In reply to comment #61)
> It is quite simple to rewrite all of xpcom/io to use direct windows calls apart
> from a single call to PROpen. I did this once when experimenting with a unicode
> build of mozilla. Not really much point though because the requirement for
> Unicode support in moz spreads out far more than just xpcom/io.
> 
> Part of the solution is that NSPR needs a wide-string version of the API. I
> agree that it needs to be cross-platform, and that it should be implemented in
> stages. However what we need to avoid is the case when only stage 1 is ever
> done, and the other platforms are never coded up. Cross-platform must include
> Windows 9x platforms.
> 
> At the higher level, I still think that any solution which involves doing
> if(isNT) in application code is wrong. It makes the application more complicated
> and clutters the logic. The handling of wide-string to legacy encoding on 9x
> platforms should be handled seamlessly by a library. Ideally that library is MS
> unicows, and I still believe that we can find a solution to distribute it with
> the official mozilla build. Otherwise use opencow (nee MZLU,
> http://opencow.sourceforge.net/).

But How to patch for Fx 1.5b1/2?
Comment 72 User image 石庭豐 (Seak, Teng-Fong) 2005-12-20 22:55:13 PST
I'm sure people are going to say I'm just making noise here, but I still like to remind that Windows Vista is due to be out next year, and Win98 is to be abandoned even more.  Please, for the well-being of the whole humanity, or at least of the Mozilla community, forget about Win9x family.  The lastest versions of FF and TB (1.5 and 1.0.7 resp.) are stable enough for those who are forced to use Win9x.  We must move forward.

How many times I'm pissed off because TB can't attach a file and I'm forced to change the name to something totally non-sense to suit it.  I know that more and more TB users are switched to Outlook (and Outlook Express) because of the user-friendliness it offers.  The more we are stagnant in this stage, the more TB is getting worse.  We need to be more determined.
Comment 73 User image Darin Fisher 2005-12-21 01:04:48 PST
>  We need to be more determined.

I agree.  Are you volunteering to write code?
Comment 74 User image 石庭豐 (Seak, Teng-Fong) 2005-12-21 03:56:31 PST
Sure.  But I'm mostly a Java and web programmer.  I can't say I'm very efficient in C++.

Anyway, I've tried to take a look at how to do get the source code and learnt that we need to have Cygwin and Visual C++ 6.  I could install VC6 but I would like not to do so if possible.  And then it talks about VS.NET.  I'd probably misunderstood something...

OTOH, in this bug as well as in other bugs, it seems that the Unicode support (NSLU) is already there for years but developpers are just reluctant to use them because that would need extensice testing (in Win9x and WinNT platforms).
Comment 75 User image Jungshik Shin 2005-12-21 10:23:28 PST
Teng-Fong, I'll try to resurrect my 2.5 year old patch (attachment 125603 [details] [diff] [review]), make changes to work with the current code and do  what darin suggested in bug 239279 comment #116. However, my time is limited and won't begin to work on it this year (well, I'm hoping I'll have some free time to work on this in the first few days of next year). However, if you're willing to work on this, you're free to go ahead. As to what compiler to use, see http://developer.mozilla.org/en/docs/Windows_Build_Prerequisites:Free_Microsoft_Compilers
and http://developer.mozilla.org/en/docs/Windows_Build_Prerequisites#Compiler_.26_Linker

Comment 76 User image Jungshik Shin 2006-01-11 07:00:21 PST
Created attachment 208200 [details] [diff] [review]
patch (far far from review-ready)

This is a just wip (hodgepodge of various patches). However, I don't think I'll take the approach taken in this patch. I just began to write a totally different patch that almost exclusilvey uses 'W' APIs on Windows. What I wanna do is to make things work on Win 2k/XP first and see how much effort it takes to make it work on Win 9x/ME if we still want to support them in FF 3.0, which is not very likely.
Comment 77 User image Brendan Eich [:brendan] 2006-01-11 09:59:48 PST
Current plan is not to support Windows 98 in Firefox 3 / Gecko 1.9.  I'm not sure about ME -- cc'ing vlad.

/be
Comment 78 User image Vladimir Vukicevic [:vlad] [:vladv] 2006-01-11 10:24:22 PST
ME's in the same boat as 98 -- (its usage numbers are even lower than 98's)
Comment 79 User image Roy Yokoyama 2006-01-11 10:34:05 PST
I'm glad to see people working on this bug.

XPCOM/IO was the last item I needed to do to fully support Unicode filenames 
back then; but it never materialized for reasons I don't want to get in.

I look forward to your real patch, Jungshik. 
Comment 80 User image Christian :Biesinger (don't email me, ping me on IRC) 2006-01-11 15:59:22 PST
does "not support w9x in gecko 1.9" (it is more than firefox) mean "we won't spend extra effort on w9x" or "we will actively break it and don't want patches for it"?
Comment 81 User image Darin Fisher 2006-01-11 16:02:23 PST
> does "not support w9x in gecko 1.9" (it is more than firefox) mean "we won't
> spend extra effort on w9x" or "we will actively break it and don't want patches
> for it"?

In the discussions I've heard on this topic, the intention is the latter... to accept patches.
Comment 82 User image Darin Fisher 2006-01-11 16:04:07 PST
Sorry, I meant:  In the discussions I've heard on this topic, the intention is to accept patches.  In other words, if members of the community wish to support Win9x, then we should support them in their efforts.
Comment 83 User image Brendan Eich [:brendan] 2006-01-11 17:04:14 PST
Again, vlad should comment, but short of a lot of work, the rendering subsystem moving to Thebes/Cairo in 1.9 means Win98 will perform poorly in some cases.  From what I remember, very poorly.  If someone writes a patch to correct this, it may require a lot of work.  It may require graphics card level hacking.  It may not be possible at all.

/be
Comment 84 User image Caleb 2006-01-12 00:27:43 PST
I have no say in this, but I hope it's OK for me to express my opinion.

Even if someone is dedicated enough to do something as crazy as that, why should the code be littered with hacks/workarounds/etc... for a dying (dead) OS?

Microsoft has long dropped support for Win98SE (http://support.microsoft.com/lifecycle/?p1=6898) and will drop support for WinME in 2006.
More Windows support cycles at http://support.microsoft.com/gp/lifeselectwin.

I believe that the only reason people still use Win9x is because they're stuck with it on their ancient hardware. And once the old computer dies, it will be replaced with something that will run atleast Windows XP.
Comment 85 User image Jungshik Shin 2006-01-13 09:11:17 PST
Created attachment 208388 [details] [diff] [review]
patch (stage 1)

This gets compiled, but my build hasn't yet finished so that I haven't tested it.

mResolvedPath and mWorkingPath are now in nsString and UTF-16 APIs directly invoke 'W' APIs while 'native' APIs call UTF-16 APIs with encoding conversions. Instead of using NSPR UTF-16 APIs, simplified and modified implementations were added to nsLocalFileWin.cpp. I added a flag, PR_LD_PATHW (0x4000) to  PR_LoadLibraryWithFlags to pass a UTF-16 string (casted into 'char *'). 

This doesn't work on Win 9x/ME, but I'm eager to see this fixed in FF 2.0 so that I'll add Win 9x/ME support. SpecialSystemDirectory needs to be patched as well (attachment 208200 [details] [diff] [review] has an unfinished patch for that). To do that, I also have to add an OS-detection code somewhere (perhaps in nsNativeCharsetUtils init. routine). I may or may not fix xpcom/obsolete/nsSpecialSystemDirectory.
Comment 86 User image Jungshik Shin 2006-01-14 15:09:00 PST
Created attachment 208506 [details] [diff] [review]
another checkpoint (nsLocalFileWin)


With this patch applied, a trunk build works as well as a trunk build without it (except for a potential slow-down because 'native' methods are implemented in terms of UTF-16 methods and our codebase use 'native' methods more often than UTF-16 methods). However, there are a lot more to do to enable even a simple thing like opening a file whose name has characters not covered by the current 'legacy' codepage. That's because our code use 'native' (lossy) APIs all over the places. 
When I added a warning to GetNativePath, my console got bombarded with warnings.

In many cases, we need to do something like

#ifdef XP_WIN
 use UTF-16 APIs
#else
 use 'native' APIs
#endif

An alternative is to make 'native' on Win 2k/XP UTF-8 (as done in my 'hodgepodge' patch) while leaving as it is on Win 9x/ME. In addition, for non-file related use of 'native' (registry, cmd line, env. variables), we might introduce 'nativeA' (as opposed to 'nativeW'). However, this is not compatible with a long-standing convention that 'native' filename can be fed to '(f)open'-like functions so that it's not such a good idea. 

Another alternative is to add 'UTF8' methods to nsILocalFile....

Whichever option we take, we have a lot of files to 'fix'. Of course, that has to be done in a separate bug. 

 
BTW, this patch is a lot easier to read than attachment 208388 [details] [diff] [review] thanks to the way methods are ordered in nsLocalFileWin.cpp
Comment 87 User image Jungshik Shin 2006-01-14 21:01:05 PST
(In reply to comment #86)

> than UTF-16 methods). However, there are a lot more to do to enable even a
> simple thing like opening a file whose name has characters not covered by the
> current 'legacy' codepage. That's because our code use 'native' (lossy) APIs

Oops. Just with this patch, opening a file with non-'native' characters in its name works file if it's done via File|Open (For a moment, I forgot that nsFilePicker was made to deal with the full Unicode range a long time ago by Roy). I guess opening with a double clicking should work if I apply the latest patch in bug 278161.
 
Comment 88 User image Jungshik Shin 2006-01-19 20:13:07 PST
Created attachment 209051 [details] [diff] [review]
yet another checkpoint (with support for Win 9x/ME)

It's still in WIP (especially when it comes to supporting Win 9x/ME)
Comment 89 User image neil@parkwaycc.co.uk 2006-01-20 04:57:19 PST
Would it be silly to use short names for 8-bit paths and long names for 16-bit?
Comment 90 User image Jungshik Shin 2006-01-22 20:24:55 PST
Created attachment 209325 [details] [diff] [review]
patch confirmed to work on Windows ME

I've tested a debug build with this patch on Windows ME (en-US) as well as Windows 2k (ko) to find that it actually works as intended.

There are still some loose ends to tie up. They include potential errors (buffer overrun, memory leak, use of replacement char '?' vs '_' in W2M conversion, potential string API 'link' issue, xpcom/obsolete support, whether or not to expose emulated 'W' APIs globally and how to do that if necessary etc) in my 'W' API emulation on Windows 9x/ME. I also need to make sure that I can refer to the addresses of 'W' APIs on Windows 9x/ME (although they're not actually implemented) instead of using GetProcAddress(?). My test on Windows ME indicates that it's possible, but  Wi 9x/ME differ slightly from each other  in what APIs they have so that actual testing on Win 95/98 is ncessary.

Due to a problem described in this thread (http://groups.google.com/group/netscape.public.mozilla.builds/browse_thread/thread/736f3b8791ed959b), I can't make an optimized build (even without profile added, cvpack gives me the same error when linking gklayout). If anybody is interested in testing a debug build (~13MB zipped) on Win 95/98, I'll put it up somewhere.
Comment 91 User image Jungshik Shin 2006-01-27 05:10:57 PST
Created attachment 209831 [details] [diff] [review]
1.8.x branch patch 

This is a patch for 1.8.x branch. With this patch, MS IE bookmarks with characters outside the system default codepage (Devanagari with Korean locale) were confirmed to be imported. Because this patch is a port of attachment 209325 [details] [diff] [review] to 1.8.x branch, it shares common issues with attachment 209325 [details] [diff] [review].
Comment 92 User image Jungshik Shin 2006-02-05 22:31:32 PST
Created attachment 210834 [details] [diff] [review]
patch that really works on Windows ME

I have no idea what happened with attachment 209325 [details] [diff] [review]. There's no way it could have worked on Windows ME (was I hallucinating? ...) because a dozen of 'W' APIs (which would just lead to 'Not Implemented' error on Win ME) were called with that patch. I thought I had replaced them all with the corresponding nsWinAPIs, but I didn't. Moreover, in NSPR, LoadLibraryW was used, which resulted in the dll load failure on Win ME.

Anyway, after a number of rebootings between Win 2k and Win ME, I finally made this patch work on Windows ME as well as on Windows 2k. While working on that, I realized that nsAppRunner.cpp uses nsILocalFile before xpcom initialization (and nsWinAPIs initialization). As an ad-hoc measure, I exposed NS_StartupWinAPIs so that it can be called in XRE_Main() of nsAppRunner.cpp. If there's a better way than this (e.g. calling it in nsLocalFileWinConstructor...), I'm willing to change it.

Other remaining issues:
 - I didn't change xpcom/obsolete because I need to expose nsWinAPIs outside xpcom/io to fix xpcom/obsolete (alternatively, I have to duplicate a bunch of lines in xpcom/obsolete). I'm not sure what's the best way to expose 'W' API wrapper functions of nsWinAPIs. We may want to do that to avoid the code duplication anyway (even if we don't wanna fix xpcom/obsolete) because some other parts of our code directly call Windows APIs that are wrapped up (for Windows 9x/ME) in xpcom/io/nsWinAPIs (that I added in this patch)

  - A bit more simplification is possible if we abandon Windows 95 (before OSR2), but seamonkey is still supposed to work on Win 95 so that I guess I'll just have to keep them now.
  - I need to inspect the code for emulation of 'W' APIs more thoroughly for possible 'one-by-off' error and buffer overrun, etc.
  - Need to add more comments
  - Need to resolve bug 278161 before resolving this one (the patch uploaded here may be 'polluted' with my interim patch for bug 278161)
  - There may be some Windows CE issues (but given that Darin's nsIWindowsRegKey 
implementation works there, I don't expect many issues although there may be a few problems to work around/fix).

Darin, can you take a look at this patch and give me some feedback (perhaps not in details but in the overall approach)? 
 
wtc, can you also take a look at the NSPR part (the amount of change is relatively small)? 

Thanks.
Comment 93 User image Wan-Teh Chang 2006-02-06 09:41:09 PST
Comment on attachment 210834 [details] [diff] [review]
patch that really works on Windows ME

You should add new prlink.h functions that take
UTF-16 pathnames.  We shouldn't make NSPR users
cast a PRUnichar * string to a char * string
just so we can avoid adding new NSPR functions.

We should also convert all library pathnames to
UTF-8, rather than converting just the ones given
to NSPR in UTF-16.
Comment 94 User image Darin Fisher 2006-02-06 11:19:18 PST
Hrm.. perhaps we should split the NSPR changes out into a separate bug.
Comment 95 User image Jungshik Shin 2006-02-06 17:53:56 PST
(In reply to comment #94)
> Hrm.. perhaps we should split the NSPR changes out into a separate bug.

That's a good idea given that there's apparently a "conflict" of "interest" between NSPR and Firefox et al. ;-) I filed bug 326168.

(In reply to comment #93)
> (From update of attachment 210834 [details] [diff] [review] [edit])
> You should add new prlink.h functions that take
> UTF-16 pathnames.  We shouldn't make NSPR users
> cast a PRUnichar * string to a char * string
> just so we can avoid adding new NSPR functions.

The idea behind that was to minimize NSPR changes necessary to fix this bug, on which I thought we kinda agreed in our email discussion. Perhaps, I was mistaken  or went too far in that direction.

> We should also convert all library pathnames to
> UTF-8, rather than converting just the ones given
> to NSPR in UTF-16.

That's one of what I had in mind when I wrote I needed to add more comment. In pr_UnlockedFindLibrary,  it's only the leaf name (not the whole path) that matters, isn't it? That being the case, it should work either way (in 99.99% of cases) because virtually all DLL names are in ASCII only and the directory separator is the same in UTF-8, ASCII and legacy encodings. Anyway, that's certainly not bullet-proof because somebody may have a non-ASCII dll name.
Comment 96 User image Mike Cowperthwaite 2006-02-08 11:22:44 PST
Does this patch address bug 210445?
Comment 97 User image Jungshik Shin 2006-02-08 18:33:26 PST
(In reply to comment #96)
> Does this patch address bug 210445?

No, it doesn't because to fix that, we need to use 'wmain' for the command line handling instead of main, but we can't do that as long as we support Win 9x/ME. In FF 3.0, perhaps we will switch to wmain because Win 9x/ME support will be dropped.
Comment 98 User image Christian :Biesinger (don't email me, ping me on IRC) 2006-02-08 18:56:13 PST
GetCommandLineW?
Comment 99 User image Christian :Biesinger (don't email me, ping me on IRC) 2006-02-08 19:00:53 PST
remember that we don't currently use main, because mozilla is a gui app, so it uses WinMain.
Comment 100 User image Jungshik Shin 2006-02-08 19:26:04 PST
(In reply to comment #99)
> remember that we don't currently use main, because mozilla is a gui app, so it
> uses WinMain.

Aha. thanks. Anyway, that's beyond the scope of this bug. We already have a bug on that, don't we? 

Darin and others, do you have any comment on my latest patch (even if you haven't gone through it in details but just have had a cursory look) other than NSPR part? 
For PR_LoadLibraryWithFlags, it'd be nice to hear back some opinions in bug 326168 so that we can resolve this long standing bug before long.
 
Comment 101 User image Darin Fisher 2006-02-08 22:13:02 PST
jungshik: I just read over the patch, and I think it is looking really great!
Comment 102 User image neil@parkwaycc.co.uk 2006-02-09 04:13:27 PST
(In reply to comment #98)
>GetCommandLineW?
Note that Win 9x/Me doesn't support CommandLineToArgvW.
Comment 103 User image Jungshik Shin 2006-02-12 09:23:30 PST
Created attachment 211597 [details] [diff] [review]
another update(getting closer)

Thanks, Darin, for taking a look. I'm getting closer. Updated my tree to sync with the trunk and cleaned up a bit. This patch also contains the latest patch for bug 326168 (with a typo fixed)
Comment 104 User image Jungshik Shin 2006-02-17 15:07:18 PST
Created attachment 212261 [details] [diff] [review]
another update (should work on Win95)

includes the latest patch for bug 326168
Comment 105 User image Jungshik Shin 2006-02-18 09:00:05 PST
Created attachment 212321 [details] [diff] [review]
patch for 1.8.x branch 

With this patch and the necko part of the latest patch for bug 278161, I can open a file whose name has chars. outside the default codepage on Windows 2k. It also works fine on Windows ME. It should also work on Windows 95/98 (which are basically the same as Win ME), but I can't test it myself.
Comment 106 User image Jungshik Shin 2006-02-18 09:06:11 PST
Kimura-san, can you test my patches (for trunk and for branch) on Windows 95? 
Thanks tons in advance.
Comment 107 User image Jungshik Shin 2006-02-18 09:09:12 PST
Created attachment 212322 [details] [diff] [review]
patch for trunk 

attachment 212261 [details] [diff] [review] has a missing file. (prtypes.h)
Comment 108 User image Jungshik Shin 2006-02-18 09:13:32 PST
Created attachment 212323 [details] [diff] [review]
a partial patch for bug 278161 necessary for testing my patch here

To test attachment 212312 [details] [diff] [review] or attachment 212322 [details] [diff] [review] on Win 2k/XP/Vista (to see if a filename with characters outside the default repertoire works), this partial patch for bug 278161 needs to be applied.
Comment 109 User image Masatoshi Kimura [:emk] 2006-02-18 12:38:34 PST
Comment on attachment 212322 [details] [diff] [review]
patch for trunk 

> nsGetFileVersionInfo       nsWinAPIs::mGetFileVersionInfo = GetFileVersionInfoW;
> nsGetFileVersionInfoSize   nsWinAPIs::mGetFileVersionInfoSize = GetFileVersionInfoSizeW;

VC6 sucks. winver.h bundled with VC6 defined GetFileVersionInfoW as
GetFileVersionInfoW(LPWSTR, DWORD, DWORD, LPVOID);
                    ^^^^^^
It should be
GetFileVersionInfoW(LPCWSTR, DWORD, DWORD, LPVOID);
GetFileVersionInfoW declaration does not also care about constness.

You should cast them to make VC6 happy.
Such as:
> nsGetFileVersionInfo       nsWinAPIs::mGetFileVersionInfo = (nsGetFileVersionInfo)GetFileVersionInfoW;
> nsGetFileVersionInfoSize   nsWinAPIs::mGetFileVersionInfoSize = (nsGetFileVersionInfoSize)GetFileVersionInfoSizeW;

And unfortunately, this didn't work on Win95 because SHGetPathFromIDListW is not exported from Win95 shell32.dll without IE. GetFileAttributesExW and GetDiskFreeSpaceExW are not also exported from Win95 kernel32.dll. You need GetProcAddress to assign function value to these functions.
Moreover, GetFileAttributesExA is not exported from Win95 kernel (even A function). You need to use an approach similar to GetDiskFreeSpaceExA.

With all the above errors resolved, I coudn't start on Win95 yet :-(
I'm digging into the reason.
Comment 110 User image Jungshik Shin 2006-02-18 21:31:46 PST
Thanks for testing on Windows 95.

(In reply to comment #109)
> (From update of attachment 212322 [details] [diff] [review] [edit])
> > nsGetFileVersionInfo       nsWinAPIs::mGetFileVersionInfo = GetFileVersionInfoW;
> > nsGetFileVersionInfoSize   nsWinAPIs::mGetFileVersionInfoSize = GetFileVersionInfoSizeW;
> 
> VC6 sucks. winver.h bundled with VC6 defined GetFileVersionInfoW as
> GetFileVersionInfoW(LPWSTR, DWORD, DWORD, LPVOID);
                     ^^^^^^

> It should be
> GetFileVersionInfoW(LPCWSTR, DWORD, DWORD, LPVOID);
........
> You should cast them to make VC6 happy.

I'm aware of the inconsistency between winver.h of VC++ 6 and that of MS Windows Platform SDK (and later version of VC++), but it doesn't matter as long as you have the include directory for Win PSDK *before* that of VC++ 6 as recommended by the mozilla build guide. I do use VC++ 6 because that's the *only* VC++ I have. 

> And unfortunately, this didn't work on Win95 because SHGetPathFromIDListW is
> not exported from Win95 shell32.dll without IE. GetFileAttributesExW and
> GetDiskFreeSpaceExW are not also exported from Win95 kernel32.dll. You need
> GetProcAddress to assign function value to these functions.
> Moreover, GetFileAttributesExA is not exported from Win95 kernel (even A
> function). You need to use an approach similar to GetDiskFreeSpaceExA.

Thanks. I wrote wrappers for all three of them and both trunk and 1.8.x branch build seem to work fine on Win 2k/ME. I also tried emulating GetFileAttributesExA on WinME to see how it would work on Win95 and it worked well. Anyway, none of these tests can substitute actual tests on Win95 so that your test on Win95 would be appreciated.  

Comment 111 User image Jungshik Shin 2006-02-18 21:35:40 PST
Created attachment 212367 [details] [diff] [review]
trunk patch update addressing issues pointed out in comment #109

Masatoshi, can you try it on Win95? Testing on Win98 and other old Windows would be nice, too.
Comment 112 User image Jungshik Shin 2006-02-18 21:37:26 PST
Created attachment 212368 [details] [diff] [review]
branch patch update
Comment 113 User image Masatoshi Kimura [:emk] 2006-02-19 03:03:41 PST
Comment on attachment 212367 [details] [diff] [review]
trunk patch update addressing issues pointed out in comment #109

"Program start error" does no longer occur, but it still fails with "This program has performed an illegal operation and will be shut down. If the problem persists, contact the program vendor."(actually in Japanese) We are the program vendor :-(
I'm building a debug version...
Comment 114 User image Jungshik Shin 2006-02-19 03:31:55 PST
Thanks again for testing. Hmm... that's tough. 
BTW, I put up my debug build (trunk) at 
http://i18nl10n.com/moztest/ff.debug.zip for others without a build set-up to test. 
(make a directory for it and unzip it in the directory and 'firefox' binary will be in the directory). On Windows 2k/XP, by setting the environment variable WINAPI_USE_API, one can sorta emulate Win 9x/ME. With that set, it would behave like ff 1.5/trunk. Without that set, it would be able to access the full unicode repertoire in file operations (e.g. File | open). Be aware that this patch alone does NOT fix all the bugs blocked by this bug. However, opening a file with Japanese name on French Windows 2k/XP should be possible. Saving a file to a desktop should work on Russian Windows 2k/XP whose default codepage is *changed* from the default (Windows-1251)  to one that cannot represent Russian (e.g. Windows-1252 - French, German, English, etc). 
Comment 115 User image Masatoshi Kimura [:emk] 2006-02-19 04:51:58 PST
OK. I found a crash reason.
On Win95 withoutIE, gGetSpecialPathProc will be initialized NS_GetSpecialFolderPath, but gGetSpecialFolderPathA aren't initialized because old shell32 doesn't export even A-version of GetSpecialFolderPath.
Therefore NS_GetSpecialFolderPath tries to call null pointer, then crash.
We should fall back to SHGetSpecialFolderLocation code path in this case.
Comment 116 User image Masatoshi Kimura [:emk] 2006-02-19 05:17:12 PST
That is,
+           gGetSpecialFolderPathA = (nsGetSpecialFolderPathA) 
+               GetProcAddress(gShell32DLLInst, "SHGetSpecialFolderPathA");
+           gGetSpecialFolderPath = NS_GetSpecialFolderPath;
should be like this.
+           gGetSpecialFolderPathA = (nsGetSpecialFolderPathA) 
+               GetProcAddress(gShell32DLLInst, "SHGetSpecialFolderPathA");
+           if (gGetSpecialFolderPathA)
+               gGetSpecialFolderPath = NS_GetSpecialFolderPath;
With this change, I could start, open Japanese directory, and open Japanese filename successfully on Win95!
Some minor problem remains:
I could no longer open local root directrory (e.g. file:///C:/).
This is a regression because I can open it without a patch.
Comment 117 User image Benjamin Smedberg [:bsmedberg] 2006-02-19 05:29:55 PST
jshin, what is the target release for this work? On trunk we may and should use the W APIs directly because we're dropping support for anything less than win2k. The dynamic loading only makes sense if we're targeting ff2.
Comment 118 User image Jungshik Shin 2006-02-19 06:00:16 PST
(In reply to comment #117)

> win2k. The dynamic loading only makes sense if we're targeting ff2.

I'm targeting it at FF2. And even for FF3, there _might_ be some "non-GUI" consumers of XPCOM. BTW, whether we use W APIs directly or not, our codebase has tons of issues to deal with to take the full advantage of Unicode support on Win2k or later.

(In reply to comment #116)

> With this change, I could start, open Japanese directory, and open Japanese
> filename successfully on Win95!

Thanks !!

> Some minor problem remains:
> I could no longer open local root directrory (e.g. file:///C:/).
> This is a regression because I can open it without a patch.

Using GetFileAttributesEx rather FindFirstFile, I wanted to avoid the issue, but on Win95, I have to deal with it. See 

http://lxr.mozilla.org/seamonkey/source/nsprpub/pr/src/md/windows/w95io.c#803

I can port that code over here, but we may as well just forget about Win95 without IE (and possibly Win NT 3.5x).

Comment 119 User image Jungshik Shin 2006-02-19 08:58:36 PST
Benjamin, Darin, Wan-Teh and I all agreed, in an offline discussion, that in the long run, we need to implement NSPR UTF-16 APIs and call them in xpcom i/o. The following is my plan I sent to Darin and Wan-Teh offline. 

1. Finish what I've been doing and (try to) include it in FF 2.0 (and
there are many places throughout our codebase I need to fix so that
fixing bug 162361 has a real impact in mozilla)
2. Implement UTF-16 APIs on Win32 (it's relatively easy because we
don't have to worry about charset name variants and iconv/wchar_t
chaos on Unix) and possibly on Mac OS X. In this step, some lines of
code added to nsLocalFileWin will become redudant.
3. Implement UTF-16 APIs on Unix and other platforms

After step 2 is complete, we may want to make trunk  use NSPR for WINNT (where NSPR UTF16 APIs will use 'W' APIs) instead of WIN95. 

Anyway, I could have used/can use a lot of '#ifdef MOZILLA_1_8_BRANCH' to use 'W' APIs directly, but given the above plan, it's not worth bothering to...
 
Comment 120 User image Masatoshi Kimura [:emk] 2006-02-24 15:51:11 PST
(In reply to comment #110)
> I'm aware of the inconsistency between winver.h of VC++ 6 and that of MS
> Windows Platform SDK (and later version of VC++), but it doesn't matter as long
> as you have the include directory for Win PSDK *before* that of VC++ 6 as
> recommended by the mozilla build guide. I do use VC++ 6 because that's the
> *only* VC++ I have. 
But this may cause tbird tbox bustage. See bug 327675. Tbird tbox seems to not follow the build guide:-)
It doesn't matter if this patch land after upgrading the tinderboxes, of course.
Comment 121 User image Jungshik Shin 2006-03-09 18:50:40 PST
Created attachment 214627 [details] [diff] [review]
patch updated (for a new patch for bug 326168)

bug 326168 is about to be fixed with a slightly different patch than included in the previous patch uploaded to this bug. This patch doesn't include NSPR patch any more, but was updated to work with the latest patch for bug 326168.

BTW, below is the result of the startup time measurement for my optimized trunk build without and with this patch. Because our tree uses nsILocalFile 'native' methods a lot, we have 5.9% performance loss [1][2]. Darin, offline, suggested that we replace them with 'UTF16' methods to gain performance on Windows 2k or later while sacrificing Linux and OS X a little (not very much because OS X uses UTF-8 and modern Linux uses UTF-8 as well).

00003.485
00003.164
00002.844
00002.804
00002.794
-----------
average: 3.0182


00003.725
00003.555
00002.794
00002.924
00002.984
-------------
average: 3.1964

[1] tinderbox startup numbers are about 500ms, but I'm getting several times larger numbers here assuming the unit is in second. I'm not sure why. 
[2] Variation is rather high so that 5.9% so that 5.9% may not be that much meaningful.
Comment 122 User image Jungshik Shin 2006-03-09 18:50:55 PST
Created attachment 214628 [details] [diff] [review]
patch updated (for a new patch for bug 326168)

bug 326168 is about to be fixed with a slightly different patch than included in the previous patch uploaded to this bug. This patch doesn't include NSPR patch any more, but was updated to work with the latest patch for bug 326168.

BTW, below is the result of the startup time measurement for my optimized trunk build without and with this patch. Because our tree uses nsILocalFile 'native' methods a lot, we have 5.9% performance loss [1][2]. Darin, offline, suggested that we replace them with 'UTF16' methods to gain performance on Windows 2k or later while sacrificing Linux and OS X a little (not very much because OS X uses UTF-8 and modern Linux uses UTF-8 as well).

00003.485
00003.164
00002.844
00002.804
00002.794
-----------
average: 3.0182


00003.725
00003.555
00002.794
00002.924
00002.984
-------------
average: 3.1964

[1] tinderbox startup numbers are about 500ms, but I'm getting several times larger numbers here assuming the unit is in second. I'm not sure why. 
[2] Variation is rather high so that 5.9% so that 5.9% may not be that much meaningful.
Comment 123 User image Darin Fisher 2006-03-09 21:44:20 PST
jshin: the tinderbox startup tests record the lowest value of the five runs instead of the average.  unfortunately it is really hard to get consistent startup numbers, so i guess someone figured this was a better way to estimate startup time.  i think we might be better off throwing out the best and the worst and then averaging the middle three values or something like that ;-)
Comment 124 User image Jungshik Shin 2006-03-10 00:36:41 PST
(In reply to comment #121)

> BTW, below is the result of the startup time measurement for my optimized trunk

The numbers reported earlier turned out to be completely bogus. I wrote a shell script as following (to follow the instruction at
http://www.mozilla.org/performance/measureStartup.html )
and ran it from an xterm (under cygwin). 

-------------------
export NS_TIMELINE_ENABLE=1

for i in `seq 0 5`
do
./firefox -P "Default User" file:///c:/temp/quit.html > startup.log.$i 2>&1
done 
-------------

Somehow the lap time reported for 'main1' depends on when I move focus to firefox. (when firefox is started from an xterm under cygwin, the focus doesn't automatically move to firefox) That is, it can be made arbitrarily long. Using 'measure-simple.pl' didn't work either. I also tried it under a non-xterm cygwin console, but it has a similar but different problem. I'll try what tinderbox does (method 2). 

Incidentally, I'll  exclude two extreme points before averaging as suggested by Darin. 
Comment 125 User image Jungshik Shin 2006-03-11 01:37:40 PST
Here's the tally of 15 runs with and without my patch (optimized static trunk) on Windows 2k(P3 700Mhz, 512MB RAM). (btw, the numbers are sorted)

patched	not patched
------  ---------
1813	1812
1813	1882
1872	1892
1872	1893
1882	1903
1883	1903
1892	1903
1893	1903
1893	1913
1902	1913
1912	1913
1913	1913
1913	1923
1913	1933
1913	1943
-------------
1885	1902.8  : average (all 15 points)
32	29      : std. deviation
1888	1906    : average(excluding max/min points)

Two-sided Student t-test (assuming the equal variance) and Welch(sp?) t-test (not assuming the equal variance) gave me 0.135269 and 0.135387, which indicates that they're different. Apparently, we have some performance gain with the patch.

Comment 126 User image Darin Fisher 2006-03-11 08:23:31 PST
nice!
Comment 127 User image Jungshik Shin 2006-03-13 00:32:56 PST
I had to run before posting my previous comment and made a couple of mistakes. 

First, the comparison was made between my build with the latest patches for this and bug 278161 and my build with only the patch for bug 278161. The patch for bug 278161 is likely to give a perf. edge to the build with this patch applied so that the comparison is not fair. 

Second, my conclusion was wrong based on the p-value of 0.13(one-sided p-value is 0.065). With that high p-value, the null hypothesis (two builds are equal in terms of startup time) should be accepted. That is, there's no strong evidence that two builds have any perf. difference.

I made a new measurement (23 startups each) with fresh builds, one with only the patch for this bug and the other without any patch applied. One-sided t-test (with H_0 : patched is slower than unpatched) gave me p-value of 0.0074. With the significance level 0.01, H_0 is rejected so that my patched build is rather likely to be faster than unpatched. This is a little unexpected given my first point above.

Anyway, the bottom line here is that my patch here does NOT make startup time longer. That is, we don't have to worry about the startup performance.  
Comment 128 User image Jungshik Shin 2006-03-13 00:34:57 PST
Comment on attachment 214628 [details] [diff] [review]
patch updated (for a new patch for bug 326168)

Asking for review only for now.
Comment 129 User image Darin Fisher 2006-03-15 13:13:22 PST
Created attachment 215170 [details]
review comments from darin on attachment 214628 [details] [diff] [review]
Comment 130 User image Darin Fisher 2006-03-15 13:14:32 PST
Comment on attachment 214628 [details] [diff] [review]
patch updated (for a new patch for bug 326168)

Please see my attached review comments.
Comment 131 User image Jungshik Shin 2006-03-19 18:40:54 PST
Created attachment 215618 [details] [diff] [review]
trunk patch addressing Darin's review comment

Darin, thanks a lot for your thorough review. I think I addressed all of your concerns. 

I used 'MAX_PATH' (it seems it's virtually identical to '_MAX_PATH' so that I thought I'd rather 'save' space in the source code :-)). I changed some boundary checking parts and static buffer size because I found that MAX_PATH(=_MAX_PATH) includes the terminating null (it's for paths like "C:\<256 chars>NULL"). 

Also added are a few more error checkings in SpecialSystemDirectory.cpp. The current trunk  doesn't do that, but I thought it's better to be safe. 
 
I added nsWinAPIs::sDummy and initialized it with nsWinAPIs::GlobalInit(). I don't have to call NS_StartupWin in nsXREDir...cpp any more. I kept it in nsXPCOMInit.cpp just in case. Anyway, both static optimized build and non-static debug build worked fine. 

For OM check with SetLength, I added a template helper function |SetLengthAndCheck| to nsWinAPIs.cpp because the same pattern appears several times with nsAutoString and nsCAutoString. 

I deleted all the commented out codes and unncessary comments while adding a rather long comment about helper functions in nsLocalFileWin (OpenDir, ReadDir, OpenFile, etc.) that will eventually be removed once NSPR implements them. 

As for not bothering to take care of root path and paths ending with a slash on Win95 (nsWinAPIs.cpp : GetFileAttributesEx), it's very rare to see Win 95 without MS IE 4 or later. Even on such a machine, the only problem is that a user can't open a root path or a path ending with a slash. We can just release-note it instead of copying a raher big chunk of code from NSPR. Btw, MS's own emulation of GetFileAttributesEx in an SDK header file doesn't do that either.

I haven't yet tested this patch on real Windows ME (I'm building a non-cairo build now), but with WINAPI_USE_ANSI on Windows 2k, a non-static debug build worked fine.
Comment 132 User image Jungshik Shin 2006-03-20 08:04:43 PST
Created attachment 215662 [details] [diff] [review]
updated trunk patch to make it work on real Win 9x/ME

attachment 215618 [details] [diff] [review] has a critical problem. It doesn't work on Win 9x/ME. 'WINAPI_USE_ANSI' cannot be a true substitute for testing it on an actual Win 9x/ME. |nsWinAPIs::sDummy| was defined _before_ function pointers for Win APIs are so that function pointers set to our emulated 'W' APIs in |GlobalInit| were reverted back to native 'W' APIs which are just stubs on Win 9x/ME. 

By initializing |sDummy| with |GlobalInit| _after_ function pointers for Win APIs, I was able to avoid the problem. I actually tested a non-cairo debug (non-static) on Windows ME and it worked well. I haven't yet tested an optimized static build (non-cairo), but I guess/hope a static build doesn't have any problem.

Still a question remains : Can we rely on the behavior of VC++ 8.0 that the order static variables are initialized is the same as the order they're defined in the source file? Does C++ standard say anything about it?
Comment 133 User image Vladimir Vukicevic [:vlad] [:vladv] 2006-03-20 11:16:10 PST
(In reply to comment #132)
> Still a question remains : Can we rely on the behavior of VC++ 8.0 that the
> order static variables are initialized is the same as the order they're defined
> in the source file? Does C++ standard say anything about it? 

If it works in VC6, then it's fine for the 1.8 branch; for the trunk, Win9x/ME are not supported, so it's a moot point.
Comment 134 User image Benjamin Smedberg [:bsmedberg] 2006-03-20 11:22:46 PST
You should not rely on the order of initializing static vars. Can't we do lazy-init in the nsLocalFile constructor or a similar place?
Comment 135 User image Christian :Biesinger (don't email me, ping me on IRC) 2006-03-20 13:53:51 PST
the C++ standard does say that order of initialization is order of declaration.
(you can't do a similar thing in C)
Comment 136 User image Jungshik Shin 2006-03-20 16:17:37 PST
(In reply to comment #134)
> You should not rely on the order of initializing static vars. Can't we do
> lazy-init in the nsLocalFile constructor or a similar place?

I can (that's one of two alternatives Darin mentioned in his review comment), but because that's a hot spot, I was worried about the overload of a calling NS_StartupWinAPI(). It may not be a problem (can be buried in 'noise'. I need to measure it). If it's not a perf-hit, perhaps it's better to take this approach. 

(In reply to comment #135)
> the C++ standard does say that order of initialization is order of declaration.

The order of *declaration* (rather than the order of *definition*)?? Hmm... function pointers are public and declared before |sDummy| (in the defintion of |class nsWinAPIs| in nsWinAPIs.h) which is private, but |sDummy| was initialized *before* function pointers until I moved its definition *after* the definition of function pointers (in nsWinAPIs.cpp). I googled it (newsgroup search) and it seems that it's the order of *definitions* that matters, but it's not definitive . [1]
 
(In reply to comment #133)

> If it works in VC6, then it's fine for the 1.8 branch

I can't build 1.8 branch with VC6 any more because VC6 and VC8-express cannot coexist on a single machine. I had to switch to VC7 toolkit to build both 1.8 branch and trunk on a single Win2k box. Anyway, my 1.8 (optimized static) build with VC7 toolkit was just completed and I'm about to test it under Win ME.


> for the trunk, Win9x/ME are not supported, so it's a moot point.

I know.. that's why I built a non-cairo build. In an unlikely(rare)  case of non-GUI embedders,  it might not ;-)

[1] C++ draft standard (perhaps of C++ 98) has the following. 
<quote> 
3.6.2 Initialization of nonlocal objects [basic.start.init]
1 The storage for objects with static storage duration (3.7.1) shall be zeroinitialized (8.5) before any other
initialization takes place. Objects of POD types (3.9) with static storage duration initialized with constant expressions (5.19) shall be initialized before any dynamic initialization takes place. Objects of namespace
scope with static storage duration _defined_ in the _same_ translation unit and dynamically initialized shall be initialized in the _order_ in which their _definition_ appears in the translation unit.
</quote>

So, it's the order of definitions that counts, but it's about objects of 'namespace scope'
Comment 137 User image Christian :Biesinger (don't email me, ping me on IRC) 2006-03-20 16:31:07 PST
Oh... sorry, yeah, I think I actually mean definition

Comment 138 User image Masatoshi Kimura [:emk] 2006-03-20 18:13:27 PST
(In reply to comment #133)
> If it works in VC6, then it's fine for the 1.8 branch; for the trunk, Win9x/ME
> are not supported, so it's a moot point.
At least installer should start on Win9x to fix bug 330208. Do you mean bug 330208 should be WONTFIXed?

(In reply to comment #136)
> I can't build 1.8 branch with VC6 any more because VC6 and VC8-express cannot
> coexist on a single machine. 
Really? I can build trunk with VC8 and build 1.8 branch with VC6 on the same machine. But my VC8 is Standard Edition, so it may make a difference.

> > for the trunk, Win9x/ME are not supported, so it's a moot point.
> I know.. that's why I built a non-cairo build. In an unlikely(rare)  case of
> non-GUI embedders,  it might not ;-)
Moreover, some tinderboxen do not still enable cairo. Have SeaMonkey guys said they would migrate to cairo build?
Comment 139 User image Stuart Parmenter 2006-03-20 19:03:44 PST
(In reply to comment #138)
> > > for the trunk, Win9x/ME are not supported, so it's a moot point.
> > I know.. that's why I built a non-cairo build. In an unlikely(rare)  case of
> > non-GUI embedders,  it might not ;-)
> Moreover, some tinderboxen do not still enable cairo. Have SeaMonkey guys said
> they would migrate to cairo build?
> 
Gecko as a platform is dropping support for old versions of Windows (Win95,98,ME) for 1.9.  Whenever SeaMonkey wishes to release with this gecko version they'll also drop support for those.

I was told by the seamonkey council that they were going to work off the 1.8 branch for a long time and would migrate to 1.9 when they were ready.  Because of this, upgrading the SeaMonkey windows tinderboxes has not been a priority.
Comment 140 User image Jungshik Shin 2006-03-20 19:19:00 PST
(In reply to comment #136)

> I can't build 1.8 branch with VC6 any more because VC6 and VC8-express cannot
> coexist on a single machine. I had to switch to VC7 toolkit to build both 1.8
> branch and trunk on a single Win2k box. Anyway, my 1.8 (optimized static) build
> with VC7 toolkit was just completed and I'm about to test it under Win ME.

 I'm writing this using a 1.8 branch build (optimized, static) with the latest patch (slightly changed for the branch)  on Windows ME. I can't make a build with VC6, but I found a newsposting in VC++ newsgroup that it's compliant to the standard mentioned by biesi (comment #135 and comment #137) so that I guess we can go with this approach.
 
Comment 141 User image Darin Fisher 2006-03-20 21:07:34 PST
pav: this patch is being developed with the goal of creating something that can ship in FF2.  that may be a tall order, but it's why we are making the effort to support Win9x.
Comment 142 User image Stuart Parmenter 2006-03-21 00:05:32 PST
For ff2 that makes sense, but for the trunk it seems like we're better off removing all the ascii calls entirely..
Comment 143 User image Jungshik Shin 2006-03-21 01:39:18 PST
Created attachment 215753 [details] [diff] [review]
patch for 1.8 branch

This is basically identical to attachment 215662 [details] [diff] [review] except for a few differences necessary for 1.8.x branch. An optimized static build with this patch was tested on a real WinME box.
Comment 144 User image Darin Fisher 2006-03-21 09:06:12 PST
> For ff2 that makes sense, but for the trunk it seems like we're better off
> removing all the ascii calls entirely..

Perhaps.  Maybe someone will want to port the trunk to Win9x?  Are we going to tell them "no" ?
Comment 145 User image Stuart Parmenter 2006-03-21 11:00:25 PST
(In reply to comment #144)
> Perhaps.  Maybe someone will want to port the trunk to Win9x?  Are we going to
> tell them "no" ?
> 

I'm not really sure what the right answer is there.  I think that developers should be able to ignore win9x and we should start moving our code away from it to cleaner and simpler code.  If someone did want to keep it working on win9x it might be better for them to do it as a compatibility library so that we can keep most of the code clean.
Comment 146 User image Shane Caraveo 2006-03-21 11:13:00 PST
(In reply to comment #143)
> Created an attachment (id=215753) [edit]
> patch for 1.8 branch
> 

That patch does not build on 1.8, it makes use of PR_LibSpec_PathnameU which is only on trunk, but that's not the only error.

Comment 147 User image Darin Fisher 2006-03-21 11:25:52 PST
Comment on attachment 215662 [details] [diff] [review]
updated trunk patch to make it work on real Win 9x/ME

>Index: xpcom/build/nsXPComInit.cpp

>@@ -501,14 +504,19 @@ NS_InitXPCOM3(nsIServiceManager* *result
...
>+#ifdef XP_WIN
>+    NS_StartupWinAPIs();
>+#endif

Is this still necessary?


>+// This is a dummy variable to make sure that WinAPI is initialized 
>+// at the very start. Note that |sDummy| must be defined AFTER
>+// all the function pointers for Win APIs are initialized. Otherwise,
>+// what's done in |GlobalInit| would have no effect.
>+// XXX: Can we rely on that |sDummy| is initialized after all
>+// the function pointers for Win APIs are? Does C/C++ standard anything to 
>+// say about the order of initializing static variables?  
>+PRBool nsWinAPIs::sDummy = nsWinAPIs::GlobalInit();

If we have decided that this works and is valid per the C++ spec,
then let's go ahead and change this comment to remove the XXX part.


Perhaps you should seek an additional review on this patch... maybe
bsmedberg would be willing? ;-)
Comment 148 User image Shane Caraveo 2006-03-21 13:25:10 PST
(In reply to comment #146)
> (In reply to comment #143)
> > Created an attachment (id=215753) [edit]
> > patch for 1.8 branch
> > 
> 
> That patch does not build on 1.8, it makes use of PR_LibSpec_PathnameU which is
> only on trunk, but that's not the only error.
> 

I see, it relies on bug 326168
Comment 149 User image Jungshik Shin 2006-03-21 14:52:42 PST
> 
> That patch does not build on 1.8, it makes use of PR_LibSpec_PathnameU which is
> only on trunk, but that's not the only error.

Yes, it relies on the patch for bug 326168 as you realized later. The patch for bug 326168 needs to be slightly changed (the diff context is a little different so that it can't be applied cleanly). 

(In reply to comment #145)
> (In reply to comment #144)
> > Perhaps.  Maybe someone will want to port the trunk to Win9x?  Are we going to
> > tell them "no" ?
> >  

> to cleaner and simpler code.  If someone did want to keep it working on win9x
> it might be better for them to do it as a compatibility library so that we can
> keep most of the code clean.

That's more or less what's done here. Almost everything for Win 9x/ME is confined to WinAPIs.{h,cpp}. Exceptions are that we use 'nsWinAPIs::mCopyFile" instead of  '::CopyFileW' in other files(the same is true of other Win32 APIs we use in xpcom i/o) and that there are a couple of |if NS_UseUnicode() ... else ...| in other files. NSPR needs some changes as well (in bug 326168), but it has its 'own life' so that it's not much relevant here.
Comment 150 User image Christian :Biesinger (don't email me, ping me on IRC) 2006-03-21 14:55:19 PST
when fixed, Bug 330276 would make it very hard to make trunk work on win9x w/o a compatibility library
Comment 151 User image Jungshik Shin 2006-03-21 15:08:04 PST
Created attachment 215818 [details] [diff] [review]
updated trunk patch (NS_StartupWinAPIs is now gone)

I got rid of the unncessary NS_StartupWinAPIs and NS_ShutdownWinAPIs as pointed out by Darin. I also removed 'XXX' comment about the initialization order.
Comment 152 User image Jungshik Shin 2006-03-21 15:56:36 PST
Comment on attachment 215818 [details] [diff] [review]
updated trunk patch (NS_StartupWinAPIs is now gone)

Darin and Benjamin, thanks for review. 

Darin, would you sr or should someone else do it?
Comment 153 User image Darin Fisher 2006-03-21 16:19:42 PST
Comment on attachment 215818 [details] [diff] [review]
updated trunk patch (NS_StartupWinAPIs is now gone)

sr=darin

let me know if you need help getting this landed.  nice work on this patch, jshin!
Comment 154 User image Jungshik Shin 2006-03-21 20:32:44 PST
Thanks, everyone. I've just landed the patch. Without fixing bug 278161, opening a file with characters not covered by the default code page in filename wouldn't work. However, importing IE bookmarks with the same problem should work without fixing bug 278161. So, that should be taken as a test for this patch for now.
Comment 155 User image Jungshik Shin 2006-03-21 23:44:03 PST
Created attachment 215859 [details] [diff] [review]
1.8 branch patch updated (no more NS_StartupWinAPis) 

This is basically the same as attachment 215818 [details] [diff] [review] with a few differences due to the difference between the trunk and the 1.8 branch. This patch relies on NSPR changes in bug 326168 so that the latest patch  uploaded  for 1.8.x branch in that bug also has to be applied. I'll ask for 1.8 approval after some baking of attachment 215818 [details] [diff] [review] in trunk. In the meantime, anybody is welcome to test it on her/his box (especially Win 9x/ME). On Win 9x/ME, some startup (and other) performance loss is expected because we now store file paths in UTF-16 and convert to and from the native encoding for file I/O on Win 9x/ME.
Comment 156 User image Hans-Andreas Engel 2006-03-22 09:19:23 PST
Created attachment 215907 [details] [diff] [review]
Patch for mingw-header include/w32api/winver.h

The fix for this bug broke compilation with MinGW due to an error in the MinGW header.  This header can be corrected with the attached patch; I will also submit it to the MinGW project.
Comment 157 User image Bill Gianopoulos [:WG9s] 2006-03-22 20:47:06 PST
The code checked into the trunk for this bug appears to have broken the Windows Thunderbird build.  Patrocles is red in the Thunderbird tinderbox.
Comment 158 User image Jungshik Shin 2006-03-22 22:49:33 PST
(In reply to comment #157)
> The code checked into the trunk for this bug appears to have broken the Windows
> Thunderbird build.  Patrocles is red in the Thunderbird tinderbox.

See comment #110 and comment #120. I also filed bug 331433. Who can fix patrocles configuration? 

Comment 159 User image Ben Turner (not reading bugmail, use the needinfo flag!) 2006-03-22 23:10:34 PST
preed handles this sort of thing, i believe.
Comment 160 User image Bill Gianopoulos [:WG9s] 2006-03-23 05:10:38 PST
This checkin appears to have caused the regression in bug 331453.
Comment 161 User image Jungshik Shin 2006-03-23 21:18:14 PST
Created attachment 216083 [details] [diff] [review]
fix a "typo" in attachment 215818 [details] [diff] [review]

I'm sorry somehow this stupid mistake crept in. This has a remote chance of being the cause of the regression reported in 331453
Comment 162 User image Darin Fisher 2006-03-23 22:12:45 PST
Comment on attachment 216083 [details] [diff] [review]
fix a "typo" in attachment 215818 [details] [diff] [review]

r+sr=darin
Comment 163 User image Jungshik Shin 2006-03-24 02:33:27 PST
(In reply to comment #162)
> (From update of attachment 216083 [details] [diff] [review] [edit])
> r+sr=darin

thanks. this got landed on the trunk 

Comment 164 User image Jungshik Shin 2006-03-25 18:38:09 PST
Created attachment 216274 [details] [diff] [review]
updated branch patch (with a 'typo' fixed, regression taken care of)

This incorporates attachment 216083 [details] [diff] [review] and the latst patch for bug 331453 (regression in file download)
Comment 165 User image cls 2006-03-28 09:35:28 PST
Created attachment 216551 [details] [diff] [review]
fix mingw bustage v1

|const WCHAR| vs |WCHAR| vs |CHAR| again.
Comment 166 User image Hans-Andreas Engel 2006-03-31 13:46:21 PST
Probably it is better to fix MinGW directly and to use the proper function declarations using |const| (see Comment 156 and Attachment 215907 [details] [diff]), than to change the mozilla code (see Comment 165).

MinGW's CVS is now updated, so the mentioned MinGW bustage should disappear when updating to the most recent MinGW version; see Bug 328499, Comment 48 for instructions. 
Comment 167 User image Jungshik Shin 2006-04-04 19:09:13 PDT
Created attachment 217241 [details] [diff] [review]
branch patch with follow-up patches combined

This patch includes the patches for bug 331453, bug 332123, bug 331433 (a temporary workaround for misconfigured tinderbox) as well as attachment 216083 [details] [diff] [review]. It's been in the trunk for about 10 days and I guess there won't be any more regression, but it might need still more baking on the trunk. (say, 10 more days...)

Darin, can you approve for branch landing when you think this patch has been baked long enough? 

Once this patch is landed on the branch, we can remove nsWinAPIs on the trunk (as Win 9x/ME is not supported any more on trunk: bug 330276)
Comment 168 User image Darin Fisher 2006-04-04 19:30:50 PDT
OK.  Let's shoot for later this week.
Comment 169 User image Darin Fisher 2006-04-07 17:34:47 PDT
Comment on attachment 217241 [details] [diff] [review]
branch patch with follow-up patches combined

a=darin
Comment 170 User image Peter van der Woude [:Peter6] 2006-04-09 01:48:50 PDT
checked in on branch on 2006-04-08 10:12
Comment 171 User image neil@parkwaycc.co.uk 2006-04-09 08:53:30 PDT
Created attachment 217750 [details] [diff] [review]
Fix my branch tinderbox

I would like this patch on the branch. It has also been tested on Windows XP.
Comment 172 User image Jungshik Shin 2006-04-09 18:38:57 PDT
Neil landed his patch for NT 3.51 on 1.8 branch. I had landed my branch patch earlier as noted in comment #170. 
Comment 173 User image Jungshik Shin 2006-04-09 18:43:38 PDT
Comment on attachment 217750 [details] [diff] [review]
Fix my branch tinderbox


Ooops. Sorry I was mistaken by Neil's tinderbox comment. He just applied the patch to his NT 3.51 tinderbox, but not yet landed this patch. 

Yeah, this is what I would have done. 

r=jshin
Comment 174 User image Michael Osipov 2006-04-10 15:27:38 PDT
I can't believe that this is fixed because it's not working
using german windows w/ german locale and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20060410 Firefox/2.0a1
with a simple test &#1054;&#1090;&#1076;&#1099;&#1093;.html (cyrillic script)
fifox gave me: check my first attachment
it replaced the russian letters with all the same %3F==? thi cant me right
I created a second file: tö€ß&#1085;.html
2nd screenshot
firefox failed of course of the russian &#1085; bug also won't display in the browser the € symbol instead %80 is shows %AC
this test with some special characters but no russian ones
and it works
see 3rd screenshot
looks perfect even with the € symbol and correct %80

Can somebody explain this? I can't, that's why I don't understand why this is marked as resolved fixed!
Comment 175 User image Michael Osipov 2006-04-10 15:29:09 PDT
Created attachment 217937 [details]
1st screenshot

fails russian script
Comment 176 User image Michael Osipov 2006-04-10 15:30:26 PDT
Created attachment 217938 [details]
2nd screenshot

fails russian script and additionally € sign
Comment 177 User image Michael Osipov 2006-04-10 15:31:23 PDT
Created attachment 217939 [details]
3nd screenshot

renders perfectly
Comment 178 User image Jungshik Shin 2006-04-10 16:53:47 PDT
(In reply to comment #174)
> I can't believe that this is fixed because it's not working
> using german windows w/ german locale and Mozilla/5.0 (Windows; U; Windows NT
> 5.1; en-US; rv:1.8) Gecko/20060410 Firefox/2.0a1
> with a simple test &#1054;&#1090;&#1076;&#1099;&#1093;.html (cyrillic script)

Please, set View | Character Encoding to UTF-8 before posting any non-ASCII character. You don't need to attach three screenshots to show what's already well-known. Bug 278161 is not yet fixed on 1.8 branch, which is why you still have the problem. On the trunk, bug 278161 has been fixed so that it should work fine. (see comment #154).
Comment 179 User image Michael Osipov 2006-04-15 18:27:57 PDT
Jungshik,

I just downloaded Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060415 Firefox/3.0a1

still fails!

what am I doing wrong?
Comment 180 User image Jungshik Shin 2006-04-16 09:37:43 PDT
(In reply to comment #179)

> I just downloaded Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1)
> Gecko/20060415 Firefox/3.0a1

> still fails!

How did you try to open the file? Did you try to open it by double-clicking on a file? That doesn't work yet because our command line handling and parameter passing (for file types associated with firefox) don't yet use 'W' APIs (there are bugs for both issues, but I don't remember bug numbers at the moment). Drag&Drop doesn't work either, but will be fixed very soon on the trunk.

However, opening a file with 'File | Open' works fine. So does opening a file by typing 'file:///........' in the url bar (probably since the patch for bug 261929 was checked in.) I've just downloaded the latest trunk build and confirmed that both methods worked fine (opening a Hindi-named file)
Comment 181 User image Michael Osipov 2006-04-16 10:20:06 PDT
Jungshik,

I used double-click only!
That's probably the reason.

Well, lets wait to have all opening ways patched!
Comment 182 User image Jungshik Shin 2006-04-16 18:19:29 PDT
(In reply to comment #181)
> I used double-click only!
> That's probably the reason.

That is the reason :-). I should have figured that out from question marks in your screenshot. Why don't you try 'file | open' or typing 'file://..../<cyrilic file name>' in the url bar.
 
> Well, lets wait to have all opening ways patched!

I couldn't find a bug on 'double click and file opening'. The closest I found is bug 268290. I also filed bug 334282.
 

Comment 183 User image Jungshik Shin 2006-04-16 20:06:29 PDT
(In reply to comment #182)

> I couldn't find a bug on 'double click and file opening'. The closest I found
> is bug 268290. I also filed bug 334282.
 
I meant bug 267989. Bug 334282 was duped to bug 282285


Comment 184 User image Michael Osipov 2006-04-17 04:30:09 PDT
Jungshik,

just tried 1.9a1: 2006041604 trunk

works as you have described altough produced while handling there files high CPU load which lead to browser crash
Comment 185 User image 石庭豐 (Seak, Teng-Fong) 2006-11-02 00:35:17 PST
I've just filed a bug relating to Unicode filename. Please see if it could be included in this bug's dependency tree:
https://bugzilla.mozilla.org/show_bug.cgi?id=359148

I know, I know, this bug is closed.
Comment 186 User image Kevin Brosnan 2007-12-04 15:00:27 PST
*** Bug 368647 has been marked as a duplicate of this bug. ***
Comment 187 User image neil@parkwaycc.co.uk 2010-09-13 02:19:17 PDT
Why is GetNativeCanonicalPath lossy? I thought short paths were always ASCII.
Comment 188 User image Masatoshi Kimura [:emk] 2010-09-13 06:16:46 PDT
Short path names are not guaranteed to be present. If the short path is not exist, GetShortPathName will return the input path without modification which may contain non-ASCII characters.

Note You need to log in before you can comment on or make changes to this bug.