file:// protocol does not display non ASCII folder name under cjk window system

VERIFIED FIXED

Status

()

defect
VERIFIED FIXED
18 years ago
17 years ago

People

(Reporter: ftang, Assigned: shanjian)

Tracking

({qawanted, regression})

Trunk
x86
Windows 2000
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [adt1])

Attachments

(1 attachment)

this problem is a regression from n6.2 or m0.9.4 branch
this problem is happened when we display the folder name by using the xul
approach . We may have other issue with the html version but this bug is
unrelated to that.

way to reproduce
1. running under a cjk window system
2. create a new folder name under c: drive It should automatically generate a
localized folder name mean "New folder"
3. open n6.2 and look at "file:///c|" and you will see the folder display correctly
4. open 2002050206 client, and do the same thing, you will see the folder
display as empty string

I think this is an adt1 problem. Non english user won't be able to use Mozilla to 

This bug are not related to the fix in 141247. That fix does not cause neither
fix this problem .
This is a pretty bad one. I suggest we put adt1 here. 
Keywords: nsbeta1
Whiteboard: [adt1]
adding keyword regression
Keywords: regression
I have to say I should not put [adt1] there for dougt's manager. 
I saw this on WinXP-SC with folder name with pure CJK characters, but if the
folder is combined cjk with ascii together, e.g. "<chinese>abc", then will
display fine.

Don't see same problem on WinME-JA though. 
Dupe of bug 141845

*** This bug has been marked as a duplicate of 141845 ***
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → DUPLICATE
Verified as dup of bug 141845, and bug 141845 was marked as dup of bug 102812 now.

Maybe it's good to add any new comments into bug 102812.
Status: RESOLVED → VERIFIED
I don't think this is a dup of  bug 102812.  bug 102812 is about the html
version. but this bug is about the xul version which we currently use for
mozilla rc1.
Status: VERIFIED → REOPENED
Resolution: DUPLICATE → ---
bbaetz owns the directory viewer.  bbaetz, toss it back to me if you don't have 
time and I will see if someone in intl can look at it.
Assignee: dougt → bbaetz
Status: REOPENED → NEW
I'm still hoping to get html file:/// view in soonish, but time is a problem for
me ATM

what milestone did this break in? Note that file urls use rdf:file, not the
standard streamconverting stuff, so I'd look for something there. Does this work
in 0.9.9 or 1.0RC1? If so, this could be related to darin's nsIFile changes, or
maybe even the <tree> redo.

-> dougt
Assignee: bbaetz → dougt
Frank tang asked me to take a look at this bug, since it is kind of important to
i18n. I traced the problem all the way from stream and tree construction back to
file url operation. Now I pinned down the problem. I am going to reassign this
one to myself.  Patch will follow. 
Status: NEW → ASSIGNED
reassign to myself. 
Assignee: dougt → shanjian
Status: ASSIGNED → NEW
Posted patch patchSplinter Review
To just check if previous byte is leadbyte is not enough. The 2nd byte of a
multi-byte character is almost always in the lead byte range, and in this case
the trailing "\" should be removed as well.
Doug, could you r my patch?
Status: NEW → ASSIGNED
Comment on attachment 82594 [details] [diff] [review]
patch

Please update the comment:
     // kill any trailing '\' provided it isn't the second char of DBCS

Do we still need the IsDBCSLeadByte check?
Attachment #82594 - Flags: review+
Thanks, doug. I will update the comment. 

>>Do we still need the IsDBCSLeadByte check?
Theoritically not, but practically yes. _mbsrchr is slow. 
but you already know that path[len] == '\\'
Yes, but I don't know if it is the 2nd byte of a multi-byte character or not. 
shanjian: do you have anyone in Intl QA who can take over qa of this bug?
Keywords: qawanted
alec, could you sr this bug?
QA Contact: benc → ylong
Comment on attachment 82594 [details] [diff] [review]
patch

_mbsrchr does seem like overkill here, since you just want to check the last
character, not the whole string.. 

sr=alecf but if there is another string routine to find the last character of a
multibyte string, we should be using that.

isn't there some nextchar() routine which could allow us to say

const unsigned char* last_mb_char(const unsigned char* foo)
{
  const unsigned char* last = foo;
  while (foo = nextchar(foo))
    last = foo;
  return last;
}

because otherwise, mbsrchr will continue to walk backwards down the string even
if the last character is not a '\\'
Attachment #82594 - Flags: superreview+
This backward searching assumption does not work very well for multi-byte string
unless you have speciall knowledge about the encoding, and we certainly don't
want to complicate the thing here. In other words, you have to start from the
beginning of the word in order to identify character boundary.
I don't understand what you mean? it sounds like you're saying mbsrchr doesn't work?
No, mbsrchr works fine. What I mean here is that mbsrchr performs the character
searching from the beginning to the end.  
fix checked in to trunk. 
Status: ASSIGNED → RESOLVED
Closed: 18 years ago18 years ago
Resolution: --- → FIXED
Verified fixed in 05-08 trunk build / WinXP-SC.

Since it's a [adt1], are we going to check into branch?
Blocks: 141008
Keywords: nsbeta1nsbeta1+
please consider taking this if we are going to resping beta candidate. 
Keywords: adt1.0.0
this problem will hit most non ascii file path on multibyte window platform. The
patch is basically no risk for single byte window platforms since the
IsDBCSLeadByte will always return false and so (!IsDBCSLeadByte) is always true
under single byte window system and never hit the new code shanjian added. 

Comment on attachment 82594 [details] [diff] [review]
patch

a=asa (on behalf of drivers) for checkin to the 1.0 branch
Attachment #82594 - Flags: approval+
ylong- if you verified on trunk then please mark it as VERIFIED so I can ask adt
to take it for rtm. thanks.
add "verified on trunk" in status whiteboard.
Status: RESOLVED → VERIFIED
Whiteboard: [adt1] → [adt1], verified on trunk
Blocks: 143047
adding adt1.0.0+ for checkin to the 1.0 branch.
Keywords: adt1.0.0adt1.0.0+
fix checked into branch. 
Keywords: fixed1.0.0
Blocks: 146292
No longer blocks: 141008
Verified fixed on 05-29 branch build ? WinXP-JA.
Whiteboard: [adt1], verified on trunk → [adt1]
You need to log in before you can comment on or make changes to this bug.