John, I have a proposed fix for this problem. I also have added a new test case to the FilesTest application. Some of the test cases in FilesTest had to be updated since they were passing non-native paths into a nsFileSpec constructor. I have not tried compiling this with the rest of the product, but I am confident that it should work. If it does not, please reassign to me to fix the the various places it break. My bet is that some may be using non-native paths to init nsFileSpec without going through nsFilePath. Here is the context diff: Index: nsFileSpecMac.cpp =================================================================== RCS file: /cvsroot/mozilla/base/src/mac/nsFileSpecMac.cpp,v retrieving revision 1.19 diff -w -c -2 -r1.19 nsFileSpecMac.cpp *** nsFileSpecMac.cpp 1999/04/15 02:17:41 1.19 --- nsFileSpecMac.cpp 1999/04/23 23:47:41 *************** *** 293,302 **** if (inLength < 255) { ! Str255 ppath; ! MacFileHelpers::PLstrcpy(ppath, inPathNamePtr); if (isRelative) ! err = ::FSMakeFSSpec(ioSpec.vRefNum, ioSpec.parID, ppath, &ioSpec); else ! err = ::FSMakeFSSpec(0, 0, ppath, &ioSpec); } else if (!isRelative) --- 293,302 ---- if (inLength < 255) { ! Str255 pascalpath; ! MacFileHelpers::PLstrcpy(pascalpath, inPathNamePtr); if (isRelative) ! err = ::FSMakeFSSpec(ioSpec.vRefNum, ioSpec.parID, pascalpath, & ioSpec); else ! err = ::FSMakeFSSpec(0, 0, pascalpath, &ioSpec); } else if (!isRelative) *************** *** 576,584 **** mSpec.vRefNum = 0; mSpec.parID = 0; ! // Convert unix (non-encoded) path to a spec. ! mError = NS_FILE_RESULT( ! MacFileHelpers::FSSpecFromUnixPath( ! inString, ! mSpec, false, false, true, inCreateDirs)); if (mError == NS_FILE_RESULT(fnfErr)) mError = NS_OK; --- 576,582 ---- mSpec.vRefNum = 0; mSpec.parID = 0; ! ! mError = NS_FILE_RESULT(MacFileHelpers::FSSpecFromPathname(inString, mSpec, inCreateDirs)); ! if (mError == NS_FILE_RESULT(fnfErr)) mError = NS_OK; *************** *** 591,599 **** mSpec.vRefNum = 0; mSpec.parID = 0; ! // Convert unix (non-encoded) path to a spec. ! mError = NS_FILE_RESULT( ! MacFileHelpers::FSSpecFromUnixPath( ! nsAutoCString(inString), ! mSpec, false, false, true, inCreateDirs)); if (mError == NS_FILE_RESULT(fnfErr)) mError = NS_OK; --- 589,595 ---- mSpec.vRefNum = 0; mSpec.parID = 0; ! ! mError = NS_FILE_RESULT(MacFileHelpers::FSSpecFromPathname(nsAutoCString(inString), mSpec, inCreateDirs)); ! if (mError == NS_FILE_RESULT(fnfErr)) mError = NS_OK; *************** *** 624,630 **** mSpec.vRefNum = 0; mSpec.parID = 0; ! // Convert unix (non-encoded) path to a spec. ! mError = NS_FILE_RESULT( ! MacFileHelpers::FSSpecFromUnixPath(inString, mSpec, false, false, true)); if (mError == NS_FILE_RESULT(fnfErr)) mError = NS_OK; --- 620,626 ---- mSpec.vRefNum = 0; mSpec.parID = 0; ! ! mError = NS_FILE_RESULT(MacFileHelpers::FSSpecFromPathname(inString, mSpec, true)); ! if (mError == NS_FILE_RESULT(fnfErr)) mError = NS_OK; *************** *** 784,793 **** if (NS_SUCCEEDED(mError) && isDirectory) { mSpec.parID = dirID; // mSpec.vRefNum is already correct. // Convert unix path (which is unencoded) to a spec ! mError = NS_FILE_RESULT( ! MacFileHelpers::FSSpecFromUnixPath( ! inRelativePath, mSpec, false, false, true, true)); if (mError == NS_FILE_RESULT(fnfErr)) mError = NS_OK; --- 780,822 ---- if (NS_SUCCEEDED(mError) && isDirectory) { + // First determine if it is a UNIX or Mac style path + // We will do this by looking for a '/'. If we find one, it is a UNIX path. + // If it is a UNIX path, we will also look for ':' and assert if we find one. + + if (strchr(inRelativePath, '/')) + { + // Looks like a UNIX path. + NS_ASSERTION(strchr(inRelativePath, ':') == nsnull, "Can not determine path type"); + mSpec.parID = dirID; // mSpec.vRefNum is already correct. // Convert unix path (which is unencoded) to a spec ! mError = NS_FILE_RESULT( MacFileHelpers::FSSpecFromUnixPath( inRelativePath, ! mSpec, ! false, ! false, ! true, ! true)); ! } ! else ! { ! // We must be a mac path! ! ! mSpec.parID = dirID; ! ! ! nsSimpleCharString relativePath; ! if (*inRelativePath != ':') ! { ! // mac relative paths need to start with colons. ! relativePath += ":"; ! } ! ! relativePath += inRelativePath; ! ! mError = NS_FILE_RESULT(MacFileHelpers::FSSpecFromPathname(relativePath, mSpec, true)); ! ! } ! if (mError == NS_FILE_RESULT(fnfErr)) mError = NS_OK; *************** *** 951,956 **** nsFilePath::nsFilePath(const char* inString, PRBool inCreateDirs) //----------------------------------------------------------------------------- ----------- - : mFileSpec(inString, inCreateDirs) { // Make canonical and absolute. char * path = MacFileHelpers::PathNameFromFSSpec( mFileSpec, true ); --- 980,1000 ---- nsFilePath::nsFilePath(const char* inString, PRBool inCreateDirs) //----------------------------------------------------------------------------- ----------- { + + FSSpec inSpec; + + inSpec.vRefNum = 0; + inSpec.parID = 0; + + MacFileHelpers::FSSpecFromUnixPath( inString, + inSpec, + false, + false, + true, + inCreateDirs); + + mFileSpec = nsFileSpec(inSpec); + + // Make canonical and absolute. char * path = MacFileHelpers::PathNameFromFSSpec( mFileSpec, true ); *************** *** 963,968 **** nsFilePath::nsFilePath(const nsString& inString, PRBool inCreateDirs) //----------------------------------------------------------------------------- ----------- - : mFileSpec(nsAutoCString(inString), inCreateDirs) { // Make canonical and absolute. char * path = MacFileHelpers::PathNameFromFSSpec( mFileSpec, true ); --- 1007,1025 ---- nsFilePath::nsFilePath(const nsString& inString, PRBool inCreateDirs) //----------------------------------------------------------------------------- ----------- { + FSSpec inSpec; + + inSpec.vRefNum = 0; + inSpec.parID = 0; + + MacFileHelpers::FSSpecFromUnixPath( nsAutoCString(inString), + inSpec, + false, + false, + true, + inCreateDirs); + + mFileSpec = nsFileSpec(inSpec); + // Make canonical and absolute. char * path = MacFileHelpers::PathNameFromFSSpec( mFileSpec, true ); Index: FilesTest.cpp =================================================================== RCS file: /cvsroot/mozilla/base/tests/FilesTest.cpp,v retrieving revision 1.26 diff -r1.26 FilesTest.cpp 44a45 > int FileSpecAppendTest(nsFileSpec& parent, const char* relativePath); 493a495,519 > > //---------------------------------------------------------------------------------------- > int FilesTest::FileSpecAppendTest(nsFileSpec& parent, const char* relativePath) > //---------------------------------------------------------------------------------------- > { > nsFilePath initalPath(parent); > const char* initialPathString = (const char*)initalPath; > mConsole << "Inital nsFileSpec:\n\t" << initialPathString << nsEndl; > > nsFileSpec fileSpec(initalPath); > > mConsole << "Adding:\t" << relativePath << nsEndl; > > fileSpec += relativePath; > > nsFilePath resultPath(fileSpec); > const char* resultPathString = (const char*)resultPath; > mConsole << "Result:\n\t" << resultPathString << nsEndl; > > Inspect(); > return Passed(); > > } > > 498,500c524,529 < nsFileSpec aFileSpec(aFile, PR_FALSE); < nsFileSpec bFileSpec(bFile, PR_FALSE); < nsFileSpec cFileSpec(bFile, PR_FALSE); // this should == bFile --- > nsFilePath aFilePath(aFile, PR_TRUE); > nsFilePath bFilePath(bFile, PR_TRUE); > > nsFileSpec aFileSpec(aFilePath); > nsFileSpec bFileSpec(bFilePath); > nsFileSpec cFileSpec(bFilePath); // this should == bFile 515c544 < nsFileSpec dirPath(dir, PR_TRUE); --- > nsFileSpec dirPath( nsFilePath(dir, PR_TRUE) ); 522c551 < nsFileSpec mySpec(file, PR_TRUE); // relative path. --- > nsFileSpec mySpec( nsFilePath(file, PR_TRUE) ); // relative path. 528c557 < nsFileSpec filePath(file); --- > nsFileSpec filePath( nsFilePath(file, PR_TRUE) ); 545c574 < nsFileSpec dirPath(dir, PR_TRUE); --- > nsFileSpec dirPath(nsFilePath(dir, PR_TRUE)); 552c581 < nsFileSpec srcSpec(file, PR_TRUE); // relative path. --- > nsFileSpec srcSpec(nsFilePath(file, PR_TRUE)); // relative path. 880c909,912 < --- > void foobar(const char* it) > { > printf("%s", it); > } 889a922,926 > nsFileSpec ado("doodoo"); > foobar(ado); > > > 924a962,976 > > Banner("FileSpecAppendTest"); > if (FileSpecAppendTest(parent, "nested/unix/file.txt") != 0) > return -1; > > #ifdef XP_PC > if (FileSpecAppendTest(parent, "nested\\windows\\file.txt") != 0) > return -1; > #endif > > #ifdef XP_MAC > if (FileSpecAppendTest(parent, "nested:mac:file.txt") != 0) > return -1; > #endif > 945c997 < --- > 957a1010,1012 > > //FilesTest::Execute() needs a native filepath. > 962,963c1017 < if NS_FAILED(Execute("/Projects/Nav45_BRANCH/ns/cmd/macfe/"\ < "projects/client45/Client45PPC", "")) --- > if NS_FAILED(Execute("MacHD:Apple Extras:AppleScript:Script Editor", ""))

Set milestone to M5. John, should you fix this for M5?

This is blocking dougt a little, but I am not sure whether he requires it before M5. I was planning to fix it for M5, but is it required? Doug?

This is not *required* for M5.

Doug Turner says he doesn't need this until M6.

Accepting

Changing to P1 the priority of bugs upon whose solution others are waiting.

Fix checked in. Assigning dougt as qa contact.

Bulk moving to XPCOM, in preparation for removal of XP File Handling component. (XPFH has received two bugs in ~5 months, and is no longer in active use.)