Closed Bug 86948 Opened 23 years ago Closed 23 years ago

Typing local Non-Ascii directory name in URL bar does not go to the right directory in Browser

Categories

(Core :: Internationalization, defect, P4)

x86
Windows 2000
defect

Tracking

()

VERIFIED WONTFIX
mozilla0.9.6

People

(Reporter: teruko, Assigned: tetsuroy)

References

Details

(Keywords: intl, Whiteboard: need /sr=)

Attachments

(1 file, 4 obsolete files)

If you type the local Non-Ascii directory name in location bar, Netscape browser will show the wrong directory. Steps of reproduce 1. Create Non-Ascii directory in local drive. For example, C:\日本語 2. Copy some files into the directory 3. Launch Netscape and type "file:///C|/日本語" in location bar It will go to "file:///". It does not show the list of the directory. I tested in 6-20-09 Win32 Mtrunk build. This is not reproduciable in 6.0RTM, but this is reproduciable in 6.1RTM.
Added intl keyword and changed QA contact to jonrubin@netscape.com.
Keywords: intl
QA Contact: andreasb → jonrubin
Summary: Typing local Non-Ascii directory name in URL bar does not go to the right directory in Browser → Typing local Non-Ascii directory name in URL bar does not go to the right directory in Browser
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.2
Depends on: 58866
Do not looks like a show stoper. move to moz0.9.3. We won't hold moz0.9.2 for it.
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Marking moz 0.9.2. Fix is in hand (see 58866)
Target Milestone: mozilla0.9.3 → mozilla0.9.2
this bug will be fixed by 58866. But itself should not be consider moz0.9.2 blocker. move to moz0.9.3 and close it while you check in 58866
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Patch 58866 includes the fix for this bug. Marking as FIXED.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Checked this on WinMe-Ja using 062506 trunk build. If I create a Japanese folder and then try to enter the path to that folder in the URL, nothing happens (the last folder or page opened continues to be displayed). If I go to the root (file:///c:/) and then click on the Japanese folder, I can open the folder, but the location bar shows the Japanese folder name as garbage chars.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I just pulled a new trunk build (20010626) on my W2K-Ja and it does display correct folder contents after entering japanese folder name in URL bar. Is this WinMe specific? Can you check?
I just tried the same build (06-26-06 trunk) on Win2k-ja also; the problem seems to be with the "file:///" part of the path, because entering the path in a standard Dos format (c:\日本語) works.
The following condition fails in case of "file:///" : #ifdef XP_PC // Check for \ in the url-string or just a drive (PC) if(kNotFound != aIn.FindChar(PRUnichar('\\')) || ((aIn.Length() == 2 ) && (aIn.Last() == PRUnichar(':') || aIn.Last() == PRUnichar('|')))) #elif XP_UNIX // Check if it starts with / or \ (UNIX) const PRUnichar * up = aIn.GetUnicode(); if((PRUnichar('/') == *up) || (PRUnichar('\\') == *up)) #else // Do nothing (All others for now) #endif ftang : should we add the "file:" case?
Here is what's happening: 1) we attempt to check the given URL for a file name by calling // Check for if it is a file URL FileURIFixup(uriString.GetUnicode(), aURI); if(*aURI) return NS_OK; and the call fails ( when you pass "file:///e:/non-ascii" ) 2) Then we try to create an URL from "file:///e:/non-ascii" by calling // Just try to create an URL out of it NS_NewURI(aURI, uriString, nsnull); I see the non-ASCII data is corrupted in NS_NewURI(). NS_NewURI is type casting the URI string to char ( http://lxr.mozilla.org/seamonkey/source/netwerk/base/public/nsNetUtil.h#84 ) I have a patch (hack) to cover this case. The patch is to add another check for "file:///" in the previous condition ( Roy Yokoyama 2001-06-26 17:29 ) . I'll post a patch in a minute but I am not very proud of this patch.....
Attached patch adding condition for "file:///" (obsolete) (deleted) — Splinter Review
ftang : can you review?
Whiteboard: ask for /r=???
hard to review it without you. drag me into your office tomorrow morning and let's review together.
r=ftang
Whiteboard: ask for /r=??? → need /sr=
It looks like you're copying to an nsAutoString only for the #ifdef XP_WIN code: + nsAutoString inString(aIn); Why not do that only inside the #ifdef XP_WIN block? The XP_UNIX code does not seem to need a copy. Nit: How about indenting this: + else if(kNotFound != inString.FindChar(PRUnichar('\\')) + || ((inString.Length() == 2 ) && (inString.Last() == PRUnichar(':') + || inString.Last() == PRUnichar('|')))) so the grouping is clear: + else if(kNotFound != inString.FindChar(PRUnichar('\\')) + || ((inString.Length() == 2 ) && (inString.Last() == PRUnichar(':') + || inString.Last() == PRUnichar('|')))) (or with the || at the end of each line that continues, per the prevailing style in the file)?
Attached patch indenting else if. (obsolete) (deleted) — Splinter Review
brenden: >Why not do that only inside the #ifdef XP_WIN block? The XP_UNIX code >does not seem to need a copy. However, inString is referenced outside of XP_WIN as >if (PossiblyByteExpandedFileName(inString)) {
You should have jag help you with your string stuff: it seems like it could be improved.
Changing the milestone.
Target Milestone: mozilla0.9.3 → mozilla0.9.4
> { > PRBool attemptFixup = PR_FALSE; >+ nsAutoString inString(aIn); You could actually do something like: #ifdef XP_PC nsAutoString inString(aIn); ... #else const nsString& inString = aIn; #if XP_UNIX ... #endif #endif ... doSomethingWith(inString); Kinda hacky, but it works :-) > #ifdef XP_PC >- // Check for \ in the url-string or just a drive (PC) >- if(kNotFound != aIn.FindChar(PRUnichar('\\')) || ((aIn.Length() == 2 ) && >(aIn.Last() == PRUnichar(':') || aIn.Last() == PRUnichar('|')))) >+ PRInt32 findFileIndex = inString.Find("file:///", PR_TRUE); >+ if (kNotFound != findFileIndex) This will also match strings where file:/// is not at the start of the string (for whatever reason). It would be better to check if index == 0. But what you really want to know is if the first 8 characters are "file:///", with a case insensitive comparison. So do something like: if (Compare(Substring(inString, 0, 8), NS_LITERAL_STRING("file:///"), nsCaseInsensitiveStringComparator()) == 0) > { >- attemptFixup = PR_TRUE; >+ inString.Delete(inString, findFileIndex, 8); There's a shortcut if you want to modify the string itself: inString.Cut(0, 8); >+ inString.ReplaceChar(PRUnichar('/'), PRUnichar('\\')); >+ attemptFixup = PR_TRUE; > } >+ // Check for \ in the url-string or just a drive (PC) >+ else if(kNotFound != inString.FindChar(PRUnichar('\\')) >+ || ((inString.Length() == 2 ) && (inString.Last() == PRUnichar(':') >+ || inString.Last() == PRUnichar('|')))) >+ { >+ attemptFixup = PR_TRUE; >+ } > #elif XP_UNIX > // Check if it starts with / or \ (UNIX) >- const PRUnichar * up = aIn.get(); >+ const PRUnichar * up = inString.GetUnicode(); .GetUnicode() was removed from the source some time ago, use .get() instead. In this specific case, you can actually do something like: if (inString.First() == PRUnichar('/') || inString.First() == PRUnichar('\\')) > if((PRUnichar('/') == *up) || (PRUnichar('\\') == *up)) > { > attemptFixup = PR_TRUE; >@@ -195,13 +205,13 @@ > // We should remove this logic and convert to File system charset here > // once we change nsICmdLineService to use wstring and ensure > // all the Unicode data come in is correctly converted. >- if (PossiblyByteExpandedFileName(aIn)) { >+ if (PossiblyByteExpandedFileName(inString)) { > // removes high byte >- file.AssignWithConversion(aIn); >+ file.AssignWithConversion(inString); > } > else { > // converts from Unicode to FS Charset >- ConvertStringURIToFileCharset(aIn, file); >+ ConvertStringURIToFileCharset(inString, file); > } > > nsresult rv = NS_NewLocalFile(file, PR_FALSE, > getter_AddRefs(filePath));
Of course, .First() and .Last() only work if the string isn't empty (in debug mode they'll assert about this), so you might want to check that aIn isn't an empty string, before you do all this, or require the callers to do that check and assert if they pass in an empty url string.
Attached patch As per jag's suggestion. (obsolete) (deleted) — Splinter Review
bstell: can you apply the patch (08/07/01 15:40) and see if it gets compiled in Linux? jag: I am not sure what you meant by >#ifdef XP_PC > nsAutoString inString(aIn); > ... >#else > const nsString& inString = aIn; >#if XP_UNIX > ... >#endif >#endif
I'll attach a patch to show what I meant there. In my comment I consolidated the two |const nsString& inString = aIn;| lines and added the appropriate #else and #ifdef, but I think what I did in the patch is more clear. + inString.ReplaceChar(PRUnichar('/'), PRUnichar('\\')); This was in the previous patch but not the last. Intentionally? + if(inString.First() == PRUnichar('/') || inString.Frist() == PRUnichar('\\')) You mistyped First. You could indent your code differently to make it more readable. See my patch for an example.
Target Milestone: mozilla0.9.4 → ---
mass change, switching qa contact from jonrubin to ruixu.
QA Contact: jonrubin → ruixu
Target Milestone: --- → mozilla0.9.5
Attachment #40364 - Attachment is obsolete: true
Attachment #43030 - Attachment is obsolete: true
Attachment #43230 - Attachment is obsolete: true
Attachment #44986 - Attachment is obsolete: true
Something changed. The patch doesn't fix this problem anymore. Need to figure out what is going on.
Target Milestone: mozilla0.9.5 → mozilla0.9.6
102984 may be related
Depends on: 102984
see what N4.x and IE did about half of % escape and half Japanese URL. P4 for now
Priority: -- → P4
I don't think we will be supporting mixed % escape and half Japanese in URL bar. IE6 doesn't escape the URL if I open a "c:\<japanese>\test.html" There is a bug 105909 which discusses the above issue. I am marking this bug as WONTFIX
Status: REOPENED → RESOLVED
Closed: 23 years ago23 years ago
Resolution: --- → WONTFIX
Netscape browser now works with "c:\<japanese>\test.html" format via % escape, it doesn't work with "file:///c:/<japanese>/test.html" format, while IE browser works with both these formats without escape. As per Roy's comments, this won't be a bug since mixed % escape and half Japanese will not be supported. Leave as RESOLVED WONTFIX, it will be reviewed later.
Depends on: 104305
It is not reproduciable with NS7.0, recent trunk and branch builds, mark bug as VERIFIED.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: