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

Status: VERIFIED FIXED

(Core :: Networking: File, defect)

Description

[Frank Tang]

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 .

Comment 1

[Frank Tang]

This is a pretty bad one. I suggest we put adt1 here. 

Comment 2

[Roy Yokoyama]

adding keyword regression

Comment 3

[Frank Tang]

I have to say I should not put [adt1] there for dougt's manager. 

Comment 4

[Yuying Long]

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. 

Comment 5

[Bradley Baetz (:bbaetz)]

Dupe of bug 141845

*** This bug has been marked as a duplicate of 141845 ***

Comment 6

[Yuying Long]

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.

Comment 7

[Frank Tang]

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.

Comment 8

[Doug Turner (:dougt)]

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.

Comment 9

[Bradley Baetz (:bbaetz)]

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

Comment 10

[Shanjian Li]

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. 

Comment 11

[Shanjian Li]

reassign to myself. 

Comment 12

[Shanjian Li]

Attachment: patch

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.

Comment 13

[Shanjian Li]

Doug, could you r my patch?

Comment 14

[Doug Turner (:dougt)]

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?

Comment 15

[Shanjian Li]

Thanks, doug. I will update the comment. 

>>Do we still need the IsDBCSLeadByte check?
Theoritically not, but practically yes. _mbsrchr is slow. 

Comment 16

[Doug Turner (:dougt)]

but you already know that path[len] == '\\'

Comment 17

[Shanjian Li]

Yes, but I don't know if it is the 2nd byte of a multi-byte character or not. 

Comment 18

[benc]

shanjian: do you have anyone in Intl QA who can take over qa of this bug?

Comment 19

[Shanjian Li]

alec, could you sr this bug?

Comment 20

[Alec Flett]

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 '\\'

Comment 21

[Shanjian Li]

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.

Comment 22

[Alec Flett]

I don't understand what you mean? it sounds like you're saying mbsrchr doesn't work?

Comment 23

[Shanjian Li]

No, mbsrchr works fine. What I mean here is that mbsrchr performs the character
searching from the beginning to the end.  

Comment 24

[Shanjian Li]

fix checked in to trunk. 

Comment 25

[Yuying Long]

Verified fixed in 05-08 trunk build / WinXP-SC.

Since it's a [adt1], are we going to check into branch?

Comment 26

[Frank Tang]

please consider taking this if we are going to resping beta candidate. 

Comment 27

[Frank Tang]

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 28

[Asa Dotzler [:asa]]

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

a=asa (on behalf of drivers) for checkin to the 1.0 branch

Comment 29

[Frank Tang]

ylong- if you verified on trunk then please mark it as VERIFIED so I can ask adt
to take it for rtm. thanks.

Comment 30

[Yuying Long]

add "verified on trunk" in status whiteboard.

Comment 31

[scottputterman]

adding adt1.0.0+ for checkin to the 1.0 branch.

Comment 32

[Shanjian Li]

fix checked into branch. 

Comment 33

[Yuying Long]

Verified fixed on 05-29 branch build ? WinXP-JA.