286 /** 287 * Parent will be null when this is at the top of the volume. 288 */ 289 readonly attribute nsIFile parent; Looking at nsLocalFileUnix and nsLocalFileWin, that's not true in those impls. In particular, the parent of "/" on unix is "/". Not sure what the parent is on Win, exactly. Could we fix the comment or the impls, please? I just got into an infinite loop because of this (was walking up the dir tree, and assumed that I would hit null when I hit the top).

oops. wrong component. Sorry for the spam...

Yeah, the implementions has the bug. We wanted a way to know when you reached the top of the volume. Pete got any time to hit this?

I'll take it. I just ordered parts to build myself a windows box. Akk! So soon i will be able to work on the windows nsIFIle bugs. --pete

Pete: Is there any kind of external testcase that could be run for this bug?

Sure try this: js> var f = Components.classes['@mozilla.org/file/local;1'].createInstance(Components.interfaces.nsILocalFile); js> f; [xpconnect wrapped nsILocalFile @ 0x80b2a50] js> f.initWithPath('/'); js> f.parent; [xpconnect wrapped nsIFile @ 0x80adea8] js> f.parent.path; / js> Parent of '/' should be null. This is an easy one to fix on unix. --pete

On Unix and similar filesystems, the "parent" of / is /: : osc; cd / : /; cd .. : /; pwd / Do we have good reason for not preserving that behaviour?

I agree w/ shaver. Parent of '/' is '/'. Seems correct to me. > I just got into an infinite loop because of this Seems client logic is wrong. Wouldn't you just use Equals() in this case to determine when your parent nsIFile reached the top of the volume? Comments bz? --pete

> Seems client logic is wrong. Wouldn't you just use Equals() No, because that would only work on Unix. On Windows/Mac it would not. So you have to check both in the current world. _Further_, nsIFile.idl (which is frozen, I must add) has, as I point out in comment 0: 304 /** 305 * Parent will be null when this is at the top of the volume. 306 */ 307 readonly attribute nsIFile parent; which is why my code was written the way it was when I ran into the infinite loop. Please don't blame client logic that is based on the documentation in the interface. Now you can expect people to figure out for themselves that the Unix implementation does not actually work the way the interface says it should work.... or the two could agree. Or something.

Ah, lets blame dougt then. ;-) Well, like i said, implementing it to work this way on unix shouldn't be a big deal. --pete

We want XP semantics. Null termination of the ancestor line among *objects* linked by *pointers* is cleanest for XPCOM nsIFile (Unix i-numbers don't have an obvious "null" value, so /.. => / made more sense as a terminator to ken and dmr, I suppose). Nuff said. /be

qawanted - debug build needed to confirm/verify

js> f; [xpconnect wrapped nsILocalFile @ 0x810d900] js> f.initWithPath('/'); js> f.parent; null js>

Doug can you review this bro? Thanks --pete

Comment on attachment 98233 [details] [diff] [review] fix r=dougt. Lets get this into 1.2a.

Comment on attachment 98233 [details] [diff] [review] fix sr=bzbarsky

Checked in. Thanks for the speedy review guys. --pete

No harm done this time, but you should read the tinderbox rules before checking in. This patch needed a= from a driver@mozilla.org. I can do that posthumously (so to speak). /be

Shoot, i think i needed a=. Sorry.I can pull it back if need be. I just realized this an got mid air collision. I'm rusty Brendan. ;-) --pete

Is this still fixed? I was going to fix bug 129054 on my macho build and it doesn't exhibit the problem (it doesn't return null for parent).

Cathy, I don't have a trunk build handy but it still works using 1.3 RELEASE. js> f.path; / js> f.parent; null js>

-> whiteboxqa