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)
Tracking
()
VERIFIED
WONTFIX
mozilla0.9.6
People
(Reporter: teruko, Assigned: tetsuroy)
References
Details
(Keywords: intl, Whiteboard: need /sr=)
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•23 years ago
|
||
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
Assignee | ||
Updated•23 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.2
Comment 2•23 years ago
|
||
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
Assignee | ||
Comment 3•23 years ago
|
||
Marking moz 0.9.2. Fix is in hand (see 58866)
Target Milestone: mozilla0.9.3 → mozilla0.9.2
Comment 4•23 years ago
|
||
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
Assignee | ||
Comment 5•23 years ago
|
||
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 → ---
Assignee | ||
Comment 7•23 years ago
|
||
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.
Assignee | ||
Comment 9•23 years ago
|
||
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?
Assignee | ||
Comment 10•23 years ago
|
||
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.....
Assignee | ||
Comment 11•23 years ago
|
||
Comment 13•23 years ago
|
||
hard to review it without you. drag me into your office tomorrow morning and
let's review together.
Assignee | ||
Comment 14•23 years ago
|
||
Comment 15•23 years ago
|
||
r=ftang
Assignee | ||
Updated•23 years ago
|
Whiteboard: ask for /r=??? → need /sr=
Comment 16•23 years ago
|
||
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)?
Assignee | ||
Comment 17•23 years ago
|
||
Assignee | ||
Comment 18•23 years ago
|
||
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)) {
Comment 19•23 years ago
|
||
You should have jag help you with your string stuff: it seems like it could be
improved.
Assignee | ||
Comment 20•23 years ago
|
||
Changing the milestone.
Target Milestone: mozilla0.9.3 → mozilla0.9.4
Comment 21•23 years ago
|
||
> {
> 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));
Comment 22•23 years ago
|
||
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.
Assignee | ||
Comment 23•23 years ago
|
||
Assignee | ||
Comment 24•23 years ago
|
||
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
Comment 25•23 years ago
|
||
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.
Comment 26•23 years ago
|
||
Assignee | ||
Updated•23 years ago
|
Target Milestone: mozilla0.9.4 → ---
Comment 27•23 years ago
|
||
mass change, switching qa contact from jonrubin to ruixu.
QA Contact: jonrubin → ruixu
Assignee | ||
Updated•23 years ago
|
Target Milestone: --- → mozilla0.9.5
Assignee | ||
Updated•23 years ago
|
Attachment #40364 -
Attachment is obsolete: true
Assignee | ||
Updated•23 years ago
|
Attachment #43030 -
Attachment is obsolete: true
Assignee | ||
Updated•23 years ago
|
Attachment #43230 -
Attachment is obsolete: true
Assignee | ||
Updated•23 years ago
|
Attachment #44986 -
Attachment is obsolete: true
Assignee | ||
Comment 28•23 years ago
|
||
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
Comment 30•23 years ago
|
||
see what N4.x and IE did about half of % escape and half Japanese URL.
P4 for now
Priority: -- → P4
Assignee | ||
Comment 31•23 years ago
|
||
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 ago → 23 years ago
Resolution: --- → WONTFIX
Comment 32•23 years ago
|
||
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.
Comment 33•22 years ago
|
||
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.
Description
•