29341 - fix nsStdURL GetFile/SetFile

Status: VERIFIED FIXED

(Core :: Networking, defect, P3)

Description

[Warren Harris]

We need to fix nsStdURL's translation to and from nsIFiles (and consequently 
to and from native paths):

1. I don't think SetFile works since it just calls SetPath directly from the 
nsIFile's native path. I don't think SetPath takes that format.

2. We need to percent encode characters from the nsIFile in the platform 
encoding for the underlying file system. I originally thought we would encode 
them in a canonical unicode encoding, but that breaks backwards compatibility 
with 4.x. Frank has a document describing this (Frank, can you publish a link to 
it?). Also see bug 28784 for related information on convertFileToURL and 
convertURLToFileCharset in the webshell.

Comment 1

[Andreas Otte]

It seems we need platform specific converter functions from nsIFile to
file-url-format and the other way around. Should be located in the nsIFile
implementation. The converter from nsIFile to file-url has to encode/escape any
reserved url characters that could possibly interfere with urlparsing. The
converter in the other direction has to unescape the parts of the file url and
construct a valid nsIFile object for the platform in use.

Is there any more documentation on platform specfic character encodings and
stuff like that or some code I can take a look on? I also have to take a much
deeper look into nsIFile/nsIFileSpec.

Comment 2

[Andreas Otte]

Luckily I could not find any usage of nsStdURL::SetFile. I suggest we extend the
nsIFile interface with a GetUrlPath method which has to be implemented in each
nsLocalFile....cpp version. This method should convert the native nsIFile into
an url-path that can be fed into nsStdURL::SetPath.

Comment 3

[Andreas Otte]

Attachment: patch to fix SetFile for the mac

Comment 4

[Andreas Otte]

From looking at davim's work on bug 11701 the above patch should fix SetFile,
GetFile should also be handled by david's changes for bug 11701 (cc-ing davidm).

Comment 5

[Andreas Otte]

Attachment: Better patch that does also escaping of the path before giving it to SetPath

Comment 6

[selmer (gone)]

In addition to all this URL stuff, profile manager needs to be able to access 
and create files/directories that have non-ascii characters in them.  That seems 
to require Frank's changes for file system character set too.  It seems like 
escaping may make sense there too if a path has characters that can't be 
represented in the file system character set.  Can this use share that patch 
too?

Comment 7

[Andreas Otte]

Attachment: Fixed patch for the XP_PC case

Comment 8

[Warren Harris]

Andreas: I've looked at these changes, but I don't really see how they work. I 
would have expected that in each case (win, mac, unix) that we fix up the native 
path string and then call SetPath in order to break out the file base name, 
extension, etc. It doesn't look like that's happening here.

Comment 9

[Andreas Otte]

got the changes reviewed by warren, fix for first part of this bug checked in,
handing back to warren for reassignmend to i18n.

Comment 10

[Warren Harris]

I'm not sure what you mean by the i18n issues. Do you just mean using 
GetFile/SetFile instead of convertFileToURL and convertURLToFileCharset? Frank 
-- can you handle that part?

Comment 11

[Frank Tang]

I know we need to have additional code for Japanese on PC because the 0x5C 
could also be a second byte of a shift_jis characters. I am not sure how many 
other issue we need to take care here. 

Comment 12

[Frank Tang]

[e-mail] can you help to take a look at this. You help to fix 
one 5c problem before. Thanks.

Comment 13

[Makoto Kato [:m_kato]]

0x5c problem will fix by the following code.

#if defined (XP_PC)
        // Replace \ with / to convert to an url
        char* s = ePath;
          while (s) {
            if (::IsDBCSLeadByte(*s) && *(s+1) != nsnull)
              s++;
            else if (*s == '\\')
              *s = '/';
            s++;
          }
#endif

Comment 14

[Frank Tang]

Mark it M16

Comment 15

[Frank Tang]

all the nsIFile and nsIFileSpec related issue should be look together. 

Comment 16

[Makoto Kato [:m_kato]]

Attachment: fix code

Comment 17

[Frank Tang]

patch verify. ready to check in.

Comment 18

[Frank Tang]

check in m_kato's patch. remaining problem have been solved already.

Comment 19

[Tom Everingham]

verified