58481 - appendRelativePath needs to work like append

Status: RESOLVED WONTFIX

(Core :: XPCOM, defect, P3)

Description

[Pete Collins (MDG)]

Here you go Doug.

> I don't think that there is a day that goes by that I don't kick myself 
> for allowing |appendRelativePath|.  Initially, when we just and 
> |append|, this parsing logic was in the clients hands - life was good.  
> Now we have a shitty implementation in appendRelativePath that does not 
> handle every case.

Comment 1

[Peter "jag" Annema]

Yay!

If this is what I think it is that is.



You mean that something like appending "/home/jag/" to "/tmp/" will get me "/home/jag/", "../foo/bar.html" will get me "/foo/bar.html" and "baz.html" gets me "/tmp/baz.html"? Of course on a Windows machine this wouldn't make sense, but in that case appending "d:\foo" to "c:\bar" would resolve to "d:\foo".



Please tell me this is that bug, otherwise I'll need to file it :-)

(Why that really odd restriction of appendRelativePath, taking away path resolution from the only entity which can correctly do it, namely the native backend?)



I do suggest we use something like "resolveWithPath" instead of "appendRelativePath" ;-)

Comment 2

[Pete Collins (MDG)]

It is a case of nsIFile doing the right thing w/ a path string based on the
platform.

fileObj='C:\tmp';

So if a string to append w/ is '/foo/bar/foo.gif' 

appendRelativePath would format the string correctly to:

'C:\tmp\foo\bar\foo.gif'

--pete

Comment 3

[Doug Turner (:dougt)]

adding people that usually seam to care about changes in nsIFile. 

Comment 4

[Pete Collins (MDG)]

assigning to myself.

--pete

Comment 5

[Pete Collins (MDG)]

Here is what i came up w/ as a solution for this problem:
We do a replacement of the ':' char for mac and the '\' for win.

Then we would implement simular on the other platforms. 
Ex: mac would check for '/' and '\'

Win would check for: '/' and ':'

This is not perfect, since you can use these characters in fil and dir names.
So that's why i need some comments.

--pete


NS_IMETHODIMP
nsLocalFile::AppendRelativePath(const char *fragment)
{
    CHECK_mPath();
    NS_ENSURE_ARG(fragment);

    if (strlen(fragment)==0)
        return NS_OK;

    // No leading '/'
    if (*fragment == '/')
        return NS_ERROR_FILE_UNRECOGNIZED_PATH;

    PRUnichar chanonical='/';
    nsCAutoString repStr(fragment);

    repStr.ReplaceChar(":", chanonical);
    repStr.ReplaceChar("\\", chanonical);

    mPath.Adopt(ToNewCString(mPath + NS_LITERAL_CSTRING("/") +
                                nsDependentCString(repStr)));

    InvalidateCache();
    return NS_OK;
}

Comment 6

[Brendan Eich [:brendan]]

No Unix implementation should do any character transliteration, please -- you
will only break valid paths, and never do good.  Isn't the whole point of
nsIFile (not nsIUnixFile, n.b.) that you shouldn't have to worry about native
pathname rules?  I.e., you can use URI-like (or NSPR-like, or Unix-like) path names?

/be

Comment 7

[Pete Collins (MDG)]

So, should this be handled by the client?

This bug was a result of doing zip extraction from js where archives are saved
on all platforms w/ the '/' deliminating pathnames.

So in this case if i call appendRelativePath on say windows, i am appending
'/from/my/zip/archive' to a local win file path.

This caused a lot of extra overhead in my client code to 
- find out the platform
- parse the archive string path to a platfrom specific local path

Where it seems appendRelativePath should behave like append and deal w/ this
type of platfrom discrepancy.

What do you suggest Brendan?

Thanks

--pete

Comment 8

[jag (Peter Annema)]

One way around that for you would to be to use urls of the form
file:///[base]/[path] for dealing with the "unixy" paths in your zip file, then
convert those urls to nsIFiles.

As for this method, I think it shouldn't do any conversion trickery at all, but
just append the path handed to it. I expect it to be used in cases where people
know the path to be appended to be in the OS' format.

Comment 9

[Pete Collins (MDG)]

Yep. agree. I'm going to mark WONTFIX. Unless someone disagrees.

We'll leave it up to the client for now . . .

--pete