The prefs bootstrapping is not in place. This code needs to be written for mozilla. Should the path to these prefs be done with chrome URLs? It seems likely that they should, if this is the way we internationalize them. I need help on finding documentation for this stuff, so I'm cc-ing hyatt and ftang.

platform=all

Changing plumbing bugs to M5 milestone, and accepting them.

Changed target milestone to M6.

*** Bug 5598 has been marked as a duplicate of this bug. ***

Fixed on Macintosh.

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

Moving this to M7. I think this needs to be redone in the light of the to-be- written new prefs architecture. Also, there are still javascript parsing bugs in my way of getting this right, and the windows prefs stuff is all over the floor.

*** Bug 5947 has been marked as a duplicate of this bug. ***

Please look at bug #5947 (link above) for Mac-specific information).

*** Bug 5139 has been marked as a duplicate of this bug. ***

I'm doing this as follows: The build will be changed so that the config files will be exported to the components directory. Next, the prefs manager will do this at startup: 1. Read initpref.js and all.js 2. Read any other .js files in the components directory. 3. Read the platform-specific files (the names will be hard-coded) such as winpref.js, unixpref.js, and macprefs.js).

You don't want to use different file names. The chrome platform URLs are designed so that you can have a single way of referring to a platform-specific object... e.g., chrome://global/platform/platformPrefs.js could be used and it would automatically point to either a Windows, Mac, or GTK file, etc. etc. Preferences files should be retrieved using chrome URLs.

Of course there may be ordering issues here... I don't know how early prefs are read in. As long as you know which profile you're using by the time you need to read in the prefs files, then chrome URLs should function just fine.

Well actually, you don't. The default prefs are read in before the profile is picked.

I checked in the fix as advertised above. Moving the netscape.cfg case to M9.

Changing summary from "Apprunner must read netscape.cfg on startup"

Bulk reassigning mcmullen@netscape.com bugs to don@netscape.com.

dp, can your folks handle this issue?

Is this us or profile folks ?

DP, this bug is about having the pref backend recognize the netscape.cfg file (and any other parts of the old prefs hierarchy that we want to bring forward.) Profiles only determines where profile related stuff comes from. Prefs makes it happen. In addition, the netscape.cfg file doesn't live in any profile - it's application level.

moving target milestone to M11.

moving target milestone to M11.

moving to M15

Moving all libPref component bugs to new Preferences: Backend component. libPref component will be deleted.

spam: moving qa contact on some bugs from paulmac to sairuh@netscape.com

This feature is already checked in, but is currently disabled because, I could not check in the ns\modules\libpref\src\init\netscape.cfg file due to CVS problems. I have been trying to check in the file since 05/12 with Leaf's help, but the CVS problem was resolved only on 05/17 after the tree is closed. To enable the feature I will need to check in the makefile's in the commercial tree, and uncomment the line of which calls this feature. Also, I will need to update the package files. I have these changes ready to go in.If the feature is enabled, it would work for optimized windows and linux builds. I have put the feature only for optimized builds, since we do not have the psm.exe available in the seamonkey tree. Also, it is currently disabled for the Mac, because we currently do not have a psm.exe available for the Mac. Neeti Neeti

Putting on [nsbeta2+][6/01] radar. This work must be done by 06/01 or we may pull this for PR2.

Checked in the fix to enable this feature optimized windows and linux builds. Neeti

Had to turn off this feature, because libpref is not able to find psm the the first time the browser runs on optimized builds( for cases where the component.reg has been deleted). It runs fine the next time. We can reproduce the problem if we delete the component.reg and start the browser. Works fine on debug builds. See bug 40581 for details. Neeti

Reassigning libpref bug to mcafee

Move to M17 ...

Due to slip in schedule, moving this bug from [6/01] to [Will be minus on 6/15] for fix deadline.

Move to M19 since this bug has a time limit ...

is this a netscape-only feature? Why would mozilla need netscape.cfg?

varada says mozilla-side will have a generic.cfg file. Can someone attach a sample .cfg file for testing?

marking netscape-only. sample netscape.cfg is checked in, ns/modules/libpref/src/init/netscape.cfg two functions to look at here are nsPref::useLockPrefFile(), pref_VerifyLockFile(). Feature only works in debug, it is currently commented out as a workaround. Optimized bits: browser hangs.? Neeti says we're blocked on another bug but we're not sure what that is yet.

6/15 has passed... so I'm cleaning whiteboard by marking this beta2 minus. This does not appear to be a "user visible" feature. If I understand the bug, we are trying to set up a "bootstrap prefs" file to make it easy to configure. If this is crucial, please nominate for beta3, or renominate for beta2 by deleting the beta2 minus. Thanks, Jim

added nsonly kw since this is netscape [commercial] only. chris, should there be another bug filed for the mozilla case, ie, having a generic.cfg?

over to selmer

plussing based on a meeting with bijals and selmer. Steve is the wrong owner, though.

Neeti, can you take this bug? You did the major work on this back in May.

Steve: can we find another owner for this bug? neeti is very doomed with the cache. Per Neeti, most of the work is done and works fine on debug builds and someone just needs to investigate why it doesn't work in the release builds. You can seek more details from her, but she won't be able to get to this bug. Thanks!

Assigning this to Dan and removing "+" so that PDT reviews and understands the importance of this bug. This bug has significant impact on CCK. One of the most important customizations in CCK is the user agent string. We not only tag the user agent string with a "-CCK" whenever someone uses CCK, but we also allow distributors to tag their bits. Without this tagging, we lose the ability to track our distributors. Because of tracking, we know today that CCK brings in millions dollars in revenue. This tagging is required by the Browser Distribution team to track their various distribution deals.

Readding plus while I investigate. Bijal: you need to remove the entire nsbeta3 part, not just the plus.

Removed [nsbeta3+] from status whiteboard.

Sorry, am just swamped and will not be able to do this.

I know of at least one non-netscape contributor who is very interested in having this functionality working, and could perhaps even be talked into contributing code (no promises, of course :-). If noone objects, I'd like to remove the "Netscape Confidential" bit from this bug. Thoughts?

I don't see any secret stuff in this bug. The netscape.cfg algorithm isn't supposed to rely on secrecy anyway. I'd want Bij and Dan to agree though.

Dan V. and I agreed that we would not get to this feature for Seamonkey timeframe.

I agree that this bug should not be secret, especially now that NSS is completely open. (I think the original instigation of secrecy was the fact that we called into the then non-public crypto libraries). The code that does exist is pretty general, and uses the "vendor" name to form <vendor>.cfg, it's not Netscape specific and appears to all be in the Mozilla tree. We seem to be in agreement, I've flipped the privacy bit

Dan is propably referring to us, so I´ll add my 2 cents. We might be able to contribute with implementational help, as we need a specific feature in the prefs bootstrap. We require a hook for a module which lets us fetch (security related) prefs from a third party server at startup, so we could also do some additional things. According to Dans proposal, I am setting myself on the CC list.

Is there a sample netscape.cfg (or any other vendor.cfg) around that can be used to enable the feature and get to the observed hang/crash?

Varada, can you help Andreas with that?

Anyone with a netscape.cfg ... ?

I have attached a netscape.cfg file to the bug.

You can also get a netscape.cfg from www.ufaq.org, specifically from http://www.ufaq.org/downloads.html

reassigning to chipc

adding myself to the CC - I've reviewed the latest patches from chip, and they look ok. chip, please attach the patches to the bug

The new files (02/20/01) are as follows: netscp6.cfg (ns/modules/libpref/src/init) This is the netscape.cfg renamed to follow the new naming convention mozilla.cfg (mozilla/modules/libpref/src/init) This is the cfg for mozilla Manifest_cfg (mozilla/modules/libpref/src/init) The diff's should change the following files: ns/modules/libpref/src/init/all-ns.js ns/modules/libpref/src/init/Manifest_cfg mozilla/modules/libpref/src/init/all.js mozilla/modules/libpref/src/nsPref.cpp mozilla/modules/libpref/public/nsIPref.idl mozilla/profile/src/nsProfile.cpp The .js files comment out the need for the lock file (OFF BY DEFAULT) So, if you're building with these patchs... there should be no difference in functionality. To turn the use of the cfg on...you need to edit the all.js or all-ns.js file depending on whether you are using mozilla or netscp6 PSM is necessary to read the hashed file (at least at this point). So, if you're testing cfg functionality you must build with PSM.

The current patches look good and work as advertised. r=bnesse

Since the only thing used in PSM is the MD5 hash (IIRC) couldn't we bring a copy of that out into some other place in the product? Firing off PSM at startup is not going to help our performance any. Adding perf keyword so we can track the impact of this.

We're trying to figure out if we can eliminate it completely... no answer on that yet. For the time being there will be no impact as long as it stays turned off.

After PSM 2.0, will we really be "firing up PSM" for this? We explored getting just the md5 lib out of PSM and it was deemed feasible. We never got it, so maybe it was harder than advertised. With PSM back in-process, this should be trivial now...

re: PSM2 - yes, if PSM2 is our only source of an MD5 implementation, then we'll be loading the PSM2 dlls at startup, which still has noticable cost. I still think that we need a small MD5 implementation in an existing library, to reduce footprint, even if it exists in libpref itself. an md5 hash is a very small routine (2-5k compiled) and I think it would be worth avoiding PSM to have a duplicate MD5 hash sitting around somewhere, already loaded.

ok, sr=alecf on the condition that 1) the standard mozilla builds, without PSM continue to run 2) the mozilla builds WITH PSM also run i.e. dont' require "netscape.cfg" or anything

This fix does not require the cfg for either mozilla or netscp6 with or without PSM. However, if the associated all*.js file for each application is modified (removing the comments infront of pref("general.config.filename"... pref("general.config.vendor"... ) Then PSM at startup is necessary for both mozilla AND netscp6.

setting milestone to 0.8.1

The changes here are for the following reasons: 1 - PSM is no longer required for the cfg 2 - some variable name changes were made for consistency 3 - the function getLockPrefFileInfo() - was removed (no longer used) 4 - general.config.vendor is not checked until after the cfg if read This way it is possible to lock this pref and force the file name 5 - pref_Alert(return_error); were replaced with #ifdef NS_DEBUG printf(return_error); #endif to avoid any use of GUI (as GUI is not initialized yet) New diff for nsPref.cpp: Index: mozilla/modules/libpref/src/nsPref.cpp =================================================================== RCS file: /cvsroot/mozilla/modules/libpref/src/nsPref.cpp,v retrieving revision 3.143 diff -u -r3.143 nsPref.cpp --- mozilla/modules/libpref/src/nsPref.cpp 2001/03/20 14:34:53 3.143 +++ mozilla/modules/libpref/src/nsPref.cpp 2001/03/23 13:13:12 @@ -134,7 +134,6 @@ nsresult useDefaultPrefFile(); nsresult useUserPrefFile(); nsresult useLockPrefFile(); - nsresult getLockPrefFileInfo(); nsresult unregisterObservers(); @@ -326,50 +325,6 @@ } // nsPref::useUserPrefFile //------------------------------------------------------------------------------ ---------- -nsresult nsPref::getLockPrefFileInfo() -//----------------------------------------------------------------------------- ----------- -{ - nsresult rv = NS_OK; - - gLockFileName = nsnull; - gLockVendor = nsnull; - - if (gLockInfoRead) - return rv; - - gLockInfoRead = PR_TRUE; - - if (NS_SUCCEEDED(rv = CopyCharPref("general.config.filename", - &gLockFileName)) && (gLockFileName)) - { -#ifdef NS_DEBUG - printf("\ngLockFileName %s \n", gLockFileName); -#endif - if (NS_SUCCEEDED(rv = CopyCharPref("general.config.vendor", - &gLockVendor)) && (gLockVendor)) - { -#ifdef NS_DEBUG - printf("\ngLockVendor %s \n", gLockVendor); -#endif - } - else - { - /* No vendor name specified, hence cannot verify it */ - rv = NS_ERROR_FAILURE; - return rv; - } - } - else - { - /* config file is not specified, hence no need to read it.*/ - rv = NS_OK; - return rv; - } - return rv; -} // nsPref::getLockPrefFileInfo - - -//----------------------------------------------------------------------------- ----------- nsresult nsPref::useLockPrefFile() //------------------------------------------------------------------------------ ---------- { @@ -400,122 +355,54 @@ #endif } - if (NS_SUCCEEDED(rv = CopyCharPref("general.config.vendor", - getter_Copies(lockVendor)) && (lockVendor))) - { -#ifdef NS_DEBUG - printf("\nlockVendor %s \n", (const char *)lockVendor); -#endif - } - - if ((gLockFileName == nsnull) && (gLockVendor == nsnull)) - { - if ((lockFileName == nsnull) && (lockVendor == nsnull)) - { - return NS_OK; - } - if ((lockFileName == nsnull) || (lockVendor == nsnull)) - { - return_error = PL_strdup("The required configuration filename or vendor name is incorrect.Please check your preferences."); - if (return_error) - { - pref_Alert(return_error); - return NS_ERROR_FAILURE; - } - } - - fileNameLen = PL_strlen(lockFileName); - vendorLen = PL_strlen(lockVendor); - if (PL_strncmp(lockFileName, lockVendor, fileNameLen -4) != 0) - { - return_error = PL_strdup("The required configuration filename and vendor name do not match.Please check your preferences."); - if (return_error) - { - pref_Alert(return_error); - PR_FREEIF(return_error); - return NS_ERROR_FAILURE; - } - } - else - { - configFile = nsXPIDLCString::Copy(lockFileName); - if(configFile == nsnull) - return NS_ERROR_FAILURE; - } - } - else if ((gLockFileName == nsnull) || (gLockVendor == nsnull)) - { - return_error = PL_strdup("The required configuration filename or vendor name is incorrect.Please contact your administrator."); - if (return_error) - { - pref_Alert(return_error); - PR_FREEIF(return_error); - return NS_ERROR_FAILURE; - } - } - else - { - fileNameLen = PL_strlen(gLockFileName); - vendorLen = PL_strlen(gLockVendor); - if (PL_strncmp(gLockFileName, gLockVendor, fileNameLen -4) != 0) - { - return_error = PL_strdup("The required configuration filename and vendor name do not match.Please contact your administrator."); - if (return_error) - { - pref_Alert(return_error); - PR_FREEIF(return_error); - return NS_ERROR_FAILURE; - } - } - else - { + if (lockFileName == nsnull) + { + return NS_OK; + } + else + { configFile = nsXPIDLCString::Copy(lockFileName); if(configFile == nsnull) return NS_ERROR_FAILURE; - } } - - { - nsCOMPtr<nsIFile> aFile; - rv = NS_GetSpecialDirectory(NS_XPCOM_CURRENT_PROCESS_DIR, getter_AddRefs(aFile)); + nsCOMPtr<nsIFile> aFile; + rv = NS_GetSpecialDirectory(NS_XPCOM_CURRENT_PROCESS_DIR, getter_AddRefs(aFile)); #ifdef XP_MAC aFile->Append("Essential Files"); #endif // TODO: Make the rest of this code use nsIFile and // avoid this conversion - rv = _nsIFileToFileSpec(aFile, getter_AddRefs(lockPrefFile)); + rv = _nsIFileToFileSpec(aFile, getter_AddRefs(lockPrefFile)); - if (NS_SUCCEEDED(rv) && lockPrefFile) + if (NS_SUCCEEDED(rv) && lockPrefFile) + { + if (NS_SUCCEEDED(lockPrefFile->AppendRelativeUnixPath(configFile))) { - if (NS_SUCCEEDED(lockPrefFile->AppendRelativeUnixPath(configFile))) - { - if (NS_FAILED(StartUp())) - return NS_ERROR_FAILURE; + if (NS_FAILED(StartUp())) + return NS_ERROR_FAILURE; - JS_BeginRequest(gMochaContext); - result = pref_OpenFileSpec(lockPrefFile, PR_TRUE, PR_TRUE, PR_FALSE, PR_FALSE); - if (result != PREF_NOERROR) + JS_BeginRequest(gMochaContext); + result = pref_OpenFileSpec(lockPrefFile, PR_TRUE, PR_TRUE, PR_FALSE, PR_FALSE); + if (result != PREF_NOERROR) + { + rv = NS_ERROR_FAILURE; + if (result != PREF_BAD_LOCKFILE) { - rv = NS_ERROR_FAILURE; - if (result != PREF_BAD_LOCKFILE) + return_error = PL_strcpy(PL_strdup("The required configuration file "), lockFileName); + return_error = PL_strcpy(return_error, PL_strdup(" could not be found. Please reinstall the software or contact your administrator.")); + if (return_error) { - return_error = PL_strdup("The required configuration file netscape.cfg could not be found. Please reinstall the software or contact your administrator."); - if (return_error) - { - pref_Alert(return_error); - PR_FREEIF(return_error); - } +#ifdef NS_DEBUG + printf(return_error); +#endif + PR_FREEIF(return_error); } } - JS_EndRequest(gMochaContext); } + JS_EndRequest(gMochaContext); } -#ifdef DEBUG_tao - GetLocalizedUnicharPref("browser.startup.homepage",getter_Copies(prefVal)); - printf("\nStartup homepage %s \n", NS_ConvertUCS2toUTF8(prefVal).get()); -#endif } return rv; } // nsPref::useLockPrefFile @@ -571,24 +458,28 @@ { nsresult rv = StartUp(); // just to be sure if (NS_SUCCEEDED(rv)) { - rv = getLockPrefFileInfo(); rv = useDefaultPrefFile(); // really should return a value... } if (NS_SUCCEEDED(rv)) useUserPrefFile(); -/* -#ifndef NS_DEBUG -#ifndef XP_MAC - rv = useLockPrefFile(); -#endif -#endif -*/ JS_MaybeGC(gMochaContext); return rv; } //------------------------------------------------------------------------------ ---------- +NS_IMETHODIMP nsPref::ReadCFGFile() +//----------------------------------------------------------------------------- ----------- +{ + nsresult rv; +// if (NS_SUCCEEDED(rv)) { +// rv = getLockPrefFileInfo(); +// } + rv = useLockPrefFile(); + return rv; +} + +//----------------------------------------------------------------------------- ----------- NS_IMETHODIMP nsPref::ReadUserPrefsFrom(nsIFileSpec* inFile) //------------------------------------------------------------------------------ ---------- { @@ -970,7 +861,7 @@ printf("\n --> nsPref::GetLocalizedUnicharPref(%s) --", pref); #endif // if the user has set this pref, then just return the user value - if (PREF_HasUserPref(pref)) + if (PREF_HasUserPref(pref) || PREF_PrefIsLocked(pref)) return CopyUnicharPref(pref, return_buf); // user has not set the pref, so the default value @@ -1510,8 +1401,10 @@ return_error = PL_strdup("An error occurred reading the startup configuration file. Please contact your administrator."); if (return_error) { - pref_Alert(return_error); - PR_Free(return_error); +#ifdef NS_DEBUG + printf(return_error); +#endif + PR_FREEIF(return_error); } PR_Free(readBuf); return result; @@ -1550,27 +1443,25 @@ // // xx xx xx xx xx xx xx xx xx xx xx xx xx xx xx xx // where each 'xx' is a hex value. */ //------------------------------------------------------------------------------ ---------- -PRBool pref_VerifyLockFileSpec(char* buf, long buflen) +PRBool pref_VerifyLockFileSpec(char* readBuf, long fileLength) //------------------------------------------------------------------------------ ---------- { PRBool success = PR_FALSE; const int obscure_value = 7; - const long hash_length = 51; /* len = 48 chars of MD5 + // + EOL */ - unsigned char *digest; - char szHash[64]; + //const long hash_length = 51; /* len = 48 chars of MD5 + // + EOL */ + //unsigned char *digest; + //char szHash[64]; /* Unobscure file by subtracting some value from every char. */ long i; - for (i = 0; i < buflen; i++) - buf[i] -= obscure_value; + for (i = 0; i < fileLength; i++) + readBuf[i] -= obscure_value; - if (buflen >= hash_length) +/* if (fileLength >= hash_length) { - const unsigned char magic_key[] = "VonGloda5652TX75235ISBN"; - unsigned char *pStart = (unsigned char*) buf + hash_length; - unsigned int len; - + unsigned char *pStart = (unsigned char*) readBuf + hash_length; + nsresult rv; NS_WITH_SERVICE(nsISignatureVerifier, verifier, SIGNATURE_VERIFIER_CONTRACTID, &rv); @@ -1581,15 +1472,17 @@ rv = verifier->HashBegin(nsISignatureVerifier::MD5, &id); if (NS_FAILED(rv)) return success; + const unsigned char magic_key[] = "VonGloda5652TX75235ISBN"; rv = verifier->HashUpdate(id, (const char*)magic_key, sizeof(magic_key)); if (NS_FAILED(rv)) return success; - rv = verifier->HashUpdate(id, (const char*)pStart, (unsigned int)(buflen - hash_length)); + rv = verifier->HashUpdate(id, (const char*)pStart, (unsigned int)(fileLength - hash_length)); if (NS_FAILED(rv)) return success; digest = (unsigned char*)PR_MALLOC(nsISignatureVerifier::MD5_LENGTH); if (digest == nsnull) return success; + unsigned int len; rv = verifier->HashEnd(id,&digest, &len, nsISignatureVerifier::MD5_LENGTH); if (NS_FAILED(rv)) { PR_FREEIF(digest); return success; } @@ -1601,20 +1494,14 @@ (int)digest[8],(int)digest[9],(int)digest[10],(int)digest[11], (int)digest[12],(int)digest[13],(int)digest[14],(int)digest[15]); -#ifdef DEBUG_neeti - printf("%02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x", - (int)digest[0],(int)digest[1],(int)digest[2],(int)digest[3], - (int)digest[4],(int)digest[5],(int)digest[6],(int)digest[7], - (int)digest[8],(int)digest[9],(int)digest[10],(int)digest[11], - (int)digest[12],(int)digest[13],(int)digest[14],(int)digest[15]); -#endif - - success = ( PL_strncmp((const char*) buf + 3, szHash, (PRUint32)(hash_length - 4)) == 0 ); + success = ( PL_strncmp((const char*) readBuf + 3, szHash, (PRUint32)(hash_length - 4)) == 0 ); PR_FREEIF(digest); - } + }*/ + if(readBuf) + success = PR_SUCCESS; return success; } Index: mozilla/modules/libpref/src/nsPref.cpp =================================================================== RCS file: /cvsroot/mozilla/modules/libpref/src/nsPref.cpp,v retrieving revision 3.143 diff -u -r3.143 nsPref.cpp --- mozilla/modules/libpref/src/nsPref.cpp 2001/03/20 14:34:53 3.143 +++ mozilla/modules/libpref/src/nsPref.cpp 2001/03/23 13:26:53 @@ -134,7 +134,6 @@ nsresult useDefaultPrefFile(); nsresult useUserPrefFile(); nsresult useLockPrefFile(); - nsresult getLockPrefFileInfo(); nsresult unregisterObservers(); @@ -326,50 +325,6 @@ } // nsPref::useUserPrefFile //------------------------------------------------------------------------------ ---------- -nsresult nsPref::getLockPrefFileInfo() -//----------------------------------------------------------------------------- ----------- -{ - nsresult rv = NS_OK; - - gLockFileName = nsnull; - gLockVendor = nsnull; - - if (gLockInfoRead) - return rv; - - gLockInfoRead = PR_TRUE; - - if (NS_SUCCEEDED(rv = CopyCharPref("general.config.filename", - &gLockFileName)) && (gLockFileName)) - { -#ifdef NS_DEBUG - printf("\ngLockFileName %s \n", gLockFileName); -#endif - if (NS_SUCCEEDED(rv = CopyCharPref("general.config.vendor", - &gLockVendor)) && (gLockVendor)) - { -#ifdef NS_DEBUG - printf("\ngLockVendor %s \n", gLockVendor); -#endif - } - else - { - /* No vendor name specified, hence cannot verify it */ - rv = NS_ERROR_FAILURE; - return rv; - } - } - else - { - /* config file is not specified, hence no need to read it.*/ - rv = NS_OK; - return rv; - } - return rv; -} // nsPref::getLockPrefFileInfo - - -//----------------------------------------------------------------------------- ----------- nsresult nsPref::useLockPrefFile() //------------------------------------------------------------------------------ ---------- { @@ -400,122 +355,88 @@ #endif } - if (NS_SUCCEEDED(rv = CopyCharPref("general.config.vendor", - getter_Copies(lockVendor)) && (lockVendor))) - { -#ifdef NS_DEBUG - printf("\nlockVendor %s \n", (const char *)lockVendor); -#endif - } - - if ((gLockFileName == nsnull) && (gLockVendor == nsnull)) - { - if ((lockFileName == nsnull) && (lockVendor == nsnull)) - { - return NS_OK; - } - if ((lockFileName == nsnull) || (lockVendor == nsnull)) - { - return_error = PL_strdup("The required configuration filename or vendor name is incorrect.Please check your preferences."); - if (return_error) - { - pref_Alert(return_error); - return NS_ERROR_FAILURE; - } - } - - fileNameLen = PL_strlen(lockFileName); - vendorLen = PL_strlen(lockVendor); - if (PL_strncmp(lockFileName, lockVendor, fileNameLen -4) != 0) - { - return_error = PL_strdup("The required configuration filename and vendor name do not match.Please check your preferences."); - if (return_error) - { - pref_Alert(return_error); - PR_FREEIF(return_error); - return NS_ERROR_FAILURE; - } - } - else - { - configFile = nsXPIDLCString::Copy(lockFileName); - if(configFile == nsnull) - return NS_ERROR_FAILURE; - } - } - else if ((gLockFileName == nsnull) || (gLockVendor == nsnull)) - { - return_error = PL_strdup("The required configuration filename or vendor name is incorrect.Please contact your administrator."); - if (return_error) - { - pref_Alert(return_error); - PR_FREEIF(return_error); - return NS_ERROR_FAILURE; - } - } - else - { - fileNameLen = PL_strlen(gLockFileName); - vendorLen = PL_strlen(gLockVendor); - if (PL_strncmp(gLockFileName, gLockVendor, fileNameLen -4) != 0) - { - return_error = PL_strdup("The required configuration filename and vendor name do not match.Please contact your administrator."); - if (return_error) - { - pref_Alert(return_error); - PR_FREEIF(return_error); - return NS_ERROR_FAILURE; - } - } - else - { + if (lockFileName == nsnull) + { + return NS_OK; + } + else + { configFile = nsXPIDLCString::Copy(lockFileName); if(configFile == nsnull) return NS_ERROR_FAILURE; - } } - - { - nsCOMPtr<nsIFile> aFile; - rv = NS_GetSpecialDirectory(NS_XPCOM_CURRENT_PROCESS_DIR, getter_AddRefs(aFile)); + nsCOMPtr<nsIFile> aFile; + rv = NS_GetSpecialDirectory(NS_XPCOM_CURRENT_PROCESS_DIR, getter_AddRefs(aFile)); #ifdef XP_MAC aFile->Append("Essential Files"); #endif // TODO: Make the rest of this code use nsIFile and // avoid this conversion - rv = _nsIFileToFileSpec(aFile, getter_AddRefs(lockPrefFile)); + rv = _nsIFileToFileSpec(aFile, getter_AddRefs(lockPrefFile)); - if (NS_SUCCEEDED(rv) && lockPrefFile) + if (NS_SUCCEEDED(rv) && lockPrefFile) + { + if (NS_SUCCEEDED(lockPrefFile->AppendRelativeUnixPath(configFile))) { - if (NS_SUCCEEDED(lockPrefFile->AppendRelativeUnixPath(configFile))) - { - if (NS_FAILED(StartUp())) - return NS_ERROR_FAILURE; + if (NS_FAILED(StartUp())) + return NS_ERROR_FAILURE; - JS_BeginRequest(gMochaContext); - result = pref_OpenFileSpec(lockPrefFile, PR_TRUE, PR_TRUE, PR_FALSE, PR_FALSE); - if (result != PREF_NOERROR) + JS_BeginRequest(gMochaContext); + result = pref_OpenFileSpec(lockPrefFile, PR_TRUE, PR_TRUE, PR_FALSE, PR_FALSE); + if (result != PREF_NOERROR) + { + rv = NS_ERROR_FAILURE; + if (result != PREF_BAD_LOCKFILE) { - rv = NS_ERROR_FAILURE; - if (result != PREF_BAD_LOCKFILE) + return_error = PL_strcpy(PL_strdup("The required configuration file "), lockFileName); + return_error = PL_strcpy(return_error, PL_strdup(" could not be found. Please reinstall the software or contact your administrator.")); + if (return_error) { - return_error = PL_strdup("The required configuration file netscape.cfg could not be found. Please reinstall the software or contact your administrator."); - if (return_error) - { - pref_Alert(return_error); - PR_FREEIF(return_error); - } +#ifdef NS_DEBUG + printf(return_error); +#endif + PR_FREEIF(return_error); } } - JS_EndRequest(gMochaContext); } - } -#ifdef DEBUG_tao - GetLocalizedUnicharPref("browser.startup.homepage",getter_Copies(prefVal)); - printf("\nStartup homepage %s \n", NS_ConvertUCS2toUTF8(prefVal).get()); + if (NS_SUCCEEDED(rv = CopyCharPref("general.config.vendor", + getter_Copies(lockVendor)) && (lockVendor))) + { +#ifdef NS_DEBUG + printf("\nlockVendor %s \n", (const char *)lockVendor); +#endif + } + if (lockVendor == nsnull) + { + return_error = PL_strdup("The required vendor name is incorrect.Please check your preferences."); + if (return_error) + { +#ifdef NS_DEBUG + printf(return_error); +#endif + return NS_ERROR_FAILURE; + } + } + + fileNameLen = PL_strlen(lockFileName); + vendorLen = PL_strlen(lockVendor); + if (PL_strncmp(lockFileName, lockVendor, fileNameLen -4) != 0) + { + return_error = PL_strdup("The required configuration filename and vendor name do not match. Please contact your administrator."); + if (return_error) + { +#ifdef NS_DEBUG + printf(return_error); #endif + PR_FREEIF(return_error); + return NS_ERROR_FAILURE; + } + } + + JS_EndRequest(gMochaContext); + } } return rv; } // nsPref::useLockPrefFile @@ -571,24 +492,28 @@ { nsresult rv = StartUp(); // just to be sure if (NS_SUCCEEDED(rv)) { - rv = getLockPrefFileInfo(); rv = useDefaultPrefFile(); // really should return a value... } if (NS_SUCCEEDED(rv)) useUserPrefFile(); -/* -#ifndef NS_DEBUG -#ifndef XP_MAC - rv = useLockPrefFile(); -#endif -#endif -*/ JS_MaybeGC(gMochaContext); return rv; } //------------------------------------------------------------------------------ ---------- +NS_IMETHODIMP nsPref::ReadCFGFile() +//----------------------------------------------------------------------------- ----------- +{ + nsresult rv; +// if (NS_SUCCEEDED(rv)) { +// rv = getLockPrefFileInfo(); +// } + rv = useLockPrefFile(); + return rv; +} + +//----------------------------------------------------------------------------- ----------- NS_IMETHODIMP nsPref::ReadUserPrefsFrom(nsIFileSpec* inFile) //------------------------------------------------------------------------------ ---------- { @@ -970,7 +895,7 @@ printf("\n --> nsPref::GetLocalizedUnicharPref(%s) --", pref); #endif // if the user has set this pref, then just return the user value - if (PREF_HasUserPref(pref)) + if (PREF_HasUserPref(pref) || PREF_PrefIsLocked(pref)) return CopyUnicharPref(pref, return_buf); // user has not set the pref, so the default value @@ -1510,8 +1435,10 @@ return_error = PL_strdup("An error occurred reading the startup configuration file. Please contact your administrator."); if (return_error) { - pref_Alert(return_error); - PR_Free(return_error); +#ifdef NS_DEBUG + printf(return_error); +#endif + PR_FREEIF(return_error); } PR_Free(readBuf); return result; @@ -1550,27 +1477,25 @@ // // xx xx xx xx xx xx xx xx xx xx xx xx xx xx xx xx // where each 'xx' is a hex value. */ //------------------------------------------------------------------------------ ---------- -PRBool pref_VerifyLockFileSpec(char* buf, long buflen) +PRBool pref_VerifyLockFileSpec(char* readBuf, long fileLength) //------------------------------------------------------------------------------ ---------- { PRBool success = PR_FALSE; const int obscure_value = 7; - const long hash_length = 51; /* len = 48 chars of MD5 + // + EOL */ - unsigned char *digest; - char szHash[64]; + //const long hash_length = 51; /* len = 48 chars of MD5 + // + EOL */ + //unsigned char *digest; + //char szHash[64]; /* Unobscure file by subtracting some value from every char. */ long i; - for (i = 0; i < buflen; i++) - buf[i] -= obscure_value; + for (i = 0; i < fileLength; i++) + readBuf[i] -= obscure_value; - if (buflen >= hash_length) +/* if (fileLength >= hash_length) { - const unsigned char magic_key[] = "VonGloda5652TX75235ISBN"; - unsigned char *pStart = (unsigned char*) buf + hash_length; - unsigned int len; - + unsigned char *pStart = (unsigned char*) readBuf + hash_length; + nsresult rv; NS_WITH_SERVICE(nsISignatureVerifier, verifier, SIGNATURE_VERIFIER_CONTRACTID, &rv); @@ -1581,15 +1506,17 @@ rv = verifier->HashBegin(nsISignatureVerifier::MD5, &id); if (NS_FAILED(rv)) return success; + const unsigned char magic_key[] = "VonGloda5652TX75235ISBN"; rv = verifier->HashUpdate(id, (const char*)magic_key, sizeof(magic_key)); if (NS_FAILED(rv)) return success; - rv = verifier->HashUpdate(id, (const char*)pStart, (unsigned int)(buflen - hash_length)); + rv = verifier->HashUpdate(id, (const char*)pStart, (unsigned int)(fileLength - hash_length)); if (NS_FAILED(rv)) return success; digest = (unsigned char*)PR_MALLOC(nsISignatureVerifier::MD5_LENGTH); if (digest == nsnull) return success; + unsigned int len; rv = verifier->HashEnd(id,&digest, &len, nsISignatureVerifier::MD5_LENGTH); if (NS_FAILED(rv)) { PR_FREEIF(digest); return success; } @@ -1601,20 +1528,14 @@ (int)digest[8],(int)digest[9],(int)digest[10],(int)digest[11], (int)digest[12],(int)digest[13],(int)digest[14],(int)digest[15]); -#ifdef DEBUG_neeti - printf("%02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x", - (int)digest[0],(int)digest[1],(int)digest[2],(int)digest[3], - (int)digest[4],(int)digest[5],(int)digest[6],(int)digest[7], - (int)digest[8],(int)digest[9],(int)digest[10],(int)digest[11], - (int)digest[12],(int)digest[13],(int)digest[14],(int)digest[15]); -#endif - - success = ( PL_strncmp((const char*) buf + 3, szHash, (PRUint32)(hash_length - 4)) == 0 ); + success = ( PL_strncmp((const char*) readBuf + 3, szHash, (PRUint32)(hash_length - 4)) == 0 ); PR_FREEIF(digest); - } + }*/ + if(readBuf) + success = PR_SUCCESS; return success; } Awaiting review (and sr) for check-in

Ay carumba! please use attachments for patches. Not only does it make the actual comments in the bug easier to read and print, it makes it much easier to download and test the patch, and saves re-review time by making it possible to diff against previously reviewed patches.

To clarify the new attachments: 03/23/01 15:36 - diffs for the mozilla tree nsPref.cpp, nsProfile.cpp, all.js, nsIPref.idl 03/23/01 15:41 - diffs for the makefiles (mozilla tree) 03/23/01 15:45 - diffs for the makefiels (ns tree) old attachments still valid: 02/20/01 15:23 - MANIFEST_CFG 02/20/01 15:22 - mozilla.cfg 02/20/01 15:21 - netscp6.cfg 02/20/01 15:16 - diffs for MANIFEST_CFG and all-ns.js

Patch thoughts: I would move the #ifdef NS_DEBUG to encompass the creation of return_error, not just the printf. No point in creating it if you're not displaying it. You appear to be leaking at least one instance of return_error (in the if (lockVendor == nsnull) check.) Why have ReadCFGFile call useLockPrefFile. Can't you just move the remaining code from useLockPrefFile into ReadCFGFile?

Thanks - made changes to #ifdef NS_DEBUG to include the creating of return_error. (diff on the way) However... the reason for keeping useLockPrefFile separate is: useLockPrefFile returns nsresult... which, although I am not using it for anything in ReadCFGFile at present - there might be uses for it in the future (error messages relating to problems with the cfg file) that can be handled in the prefs framework... and not in the profile initialization area (which is what calls ReadCFGFile).

argh, trying to review this bug again, bugzilla keeps losing my comments on THIS bug for some reason. anyway, - return_error contains non-localized strings, which makes me think that all uses of it should be confined to NS_DEBUG... which means that the variable declaration ITSELF should be inside a NS_DEBUG to ensure that it is never used in release builds. Also, why are you randomly PL_strdup'ing static strings? It makes no sense. why not just say return_error = "foo" instead of return_error = PL_strdup("foo"); Also, I don't understand this line: + return_error = PL_strcpy(return_error, PL_strdup(" could not be found. Please reinstall the software or contact your administrator.")); you're losing the value of the 2nd strdup - maybe you meant strcat? but if you meant strcat, you would be writing over random memory. Why don't you just get rid of return_error entirely, and just put #ifdef'ed printf()s in the places where you get an error? - similarly, this code makes no sense in a non-debug environment. This code is twice as complicated and hard to read as it needs to be: + if (NS_SUCCEEDED(rv = CopyCharPref("general.config.vendor", + getter_Copies(lockVendor)) && (lockVendor))) + { +#ifdef NS_DEBUG + printf("\nlockVendor %s \n", (const char *)lockVendor); +#endif + } You should just write + rv = CopyCharPref("general.config.vendor", getter_Copies(lockVendor)); + NS_ENSURE_SUCCESS(rv, rv); I think you really just need to learn to stop using printf() and start using the debugger to determine if your code is working correctly. all develoepers (i.e. those running with NS_DEBUG) don't need to see a million printfs() at startup. - please don't unnecessarily abbreviate function or variable names. ReadCFGFile() should be ReadConfigFile - in pref_VerifyLockFileSpec please don't use C comments to remove blocks of code - either use #if 0, or remove the code entirely. Please read section 2, item 6 of http://www.mozilla.org/hacking/portable-cpp.html (and read that whole document if you haven't already!)

Please fix your indenting..stop using tabs in source code... Also, you're using JS_BeginRequest() and JS_EndRequest(), but you allow for early exit from between these two! This is bad - you need to make sure JS_EndRequest is called.

+ if (lockFileName == nsnull) + { + return NS_OK; + } + else else-after-return is a non-sequitur. + nsCOMPtr<nsIFile> aFile; + rv = NS_GetSpecialDirectory(NS_XPCOM_CURRENT_PROCESS_DIR, getter_AddRefs(aFile)); You never check the result code there -- will _nsIFileToFileSpec (spit) handle that, or crash on a null pointer? Also, why not convert over to nsIFile, while you're whacking this code anyway? gInstance->StartUp(); - } + } return gInstance; Looks like random tab-damage. Please make sure that there are no tabs in the lines you check in, and that indentation conforms to the dominant (hopefully universal) style for this file -- which should be reflected in the modeline. +NS_IMETHODIMP nsPref::ReadConfigFile() +//---------------------------------------------------------------------------------------- +{ + nsresult rv; + rv = useLockPrefFile(); + return rv; +} Why not just |return useLockPrefFile();|? - const long hash_length = 51; /* len = 48 chars of MD5 + // + EOL */ - unsigned char *digest; - char szHash[64]; + //const long hash_length = 51; /* len = 48 chars of MD5 + // + EOL */ + //unsigned char *digest; + //char szHash[64]; Why not delete these, rather than just commenting them out? + //This segment was left here as a reference - just in case anyone decides to reimplement it! People can use CVS history for that, I think. Let's try and keep the dead code to a minimum. + -I$(PUBLIC)/raptor \ Do we actually have a "raptor" directory? Do you still need all those include directories? +install:: + $(INSTALL) -m 444 $(srcdir)/mozilla.cfg $(DIST)/bin You put it in $(DIST)/bin, but you don't seem to be packaging it anywhere. Is this a necessary file, or not? (If not, should it be unconditionally installed into $(DIST)/bin?)

Question: If the cfg if off by default and the only reason to turn it on would be because the cfg actually has data internal (meaning cfg was created) is there any reason to have a cfg as part of the build? If not... I will turn off the installing of both cfg files in the makefiles

if that is how it works, then yeah, leave it out of the build... Also, I'm not real keen on the .cfg file being in the same directory as the binary, now that I think of it - we have enough junk in there already... we should probably find a better spot. how about something like defaults\eclient or something? (I welcome better names)

how about default/prefs.. the directory already exists on all platforms (initialization of the .js files) and this is a pref file.

Latest diff has the following changed (beyond corrections) 1 - The makefiles are not changed as it has been decided to NOT include the cfg in the build... but it should be added to the source tree 2 - conversion to nsIFile not done as part of this project

Chip, how does the enterprise IT department deploy a browser that REQUIRES the cfg file? The bits they deploy should refuse to work if the user deletes or corrupts the cfg file. Does that happen somewhere else?

The activation of the requirement of the config file is done via enabling the pref("general.config.filename", "netscp6.cfg"); preference in all-ns.js. This activates the code which contains the functionality you seek. Removal of or tampering with the will cause an error to be thrown at this point.

selmer: the commercial build should require this by default, via the prefs that chip listed. all-ns.js would should be stored in a read-only location.

As Brian mentioned... the general.config.vendor pref can be set and locked in the cfg file. (it can also be set in the all.js or all-ns.js files) This value is checked after the cfg is read (so the pref can be set within the cfg file).... if the value doesn't match the name of the file it fails. So... yes, it would be possible for a use to take the cfg file and alter an external all-ns.js file to match the names and function. But, the security of the file is greater than that with 4.x - and with the complaints about having PSM startup this early in the initialization phaze... I opted for this "small" bit of security (which, again, is more than 4.x) but doesn't require PSM. IF that level of security is required: 1 - I see that as a different bug 2 - I suggest we implement it as part of the profile management Determine if the profile needs PSM - and if so... check the cfg at that point (which is much later in the startup process and would not be a requirement for all profiles).

Comments on this patch: - indenting is still screwed up, please stop using tabs. just look at the diff, it's VERY obvious. - you've got to comment code to explain what is going on... for instance: - it was not obvious to me why you were checking general.config.vendor AFTER loading the config file - then I realized that the config file would have that pref in it. - what is this magic number "-4" you're subtracting from fileNameLen - _I_ now realize it refers to the length of ".cfg" but that's because I'm intimately familiar with this code. - this patch deserves at least 20 lines of comments sprinkled througout it explaining what you are doing. comments cost nothing. - you're returning PREF_ERROR from a method that is supposed to return an nsresult - they are not the same thing. you should be returning an error that's prefixed with NS_ERROR_* - like NS_ERROR_FAILURE - I thought we agreed to get rid of return_error - I still see it declared in this patch, even though I don't see it used. please remove it so nobody (including yourself) is tempted to use it again. Are you looking at compiler warnings? are there other variables that are unused? - please don't extend the PREF_ERROR system - it should eventually go away and be replaced with nsresults, but in the mean time all new code should use nsresult.

Almost there.... some of your changes are just adding unnecessary clutter and making it hard to read. why say: if (foo) { rv = NS_ERROR_FAILURE; return rv; } when you could say: if (foo) return NS_ERROR_FAILURE; Also, never compare directly against NS_OK, instead use the NS_SUCCEEDED() macro, like the rest of the codebase does.

+ if (NS_SUCCEEDED(rv = CopyCharPref("general.config.filename", + getter_Copies(lockFileName)) && (lockFileName))) + { +#ifdef NS_DEBUG + printf("\nlockFile %s \n", (const char *)lockFileName); +#endif + } + if (lockFileName == nsnull) + return rv; 1) We don't need more debug spew. Make it DEBUG_chipc if you _must_ leave it in, but best is to remove it and use a debugger if it starts to fail. 2) You check NS_SUCCEEDED before your console message, but then check lockFileName against nsnull (explicitly, unlike in the previous check!) to determine whether or not you're going to bail early. Can CopyCharPref hand back a null string and a success code? I'd think not, so just check NS_FAILED(rv) and then bail with rv if so. + nsCOMPtr<nsIFile> aFile; + rv = NS_GetSpecialDirectory(NS_XPCOM_CURRENT_PROCESS_DIR, getter_AddRefs(aFile)); + if (NS_SUCCEEDED(rv)) + { Here, and in the code quoted above, you diverge from the dominant brace placement style (cuddled with if). Please fix. + // TODO: Make the rest of this code use nsIFile and + // avoid this conversion + rv = _nsIFileToFileSpec(aFile, getter_AddRefs(lockPrefFile)); 1) _nsIFileToFileSpec is a non-ANSI name (compilers own everything in the global namespace that begins with an underscore) 2) I asked about this in my previous comments, and didn't get an answer, and the code remains: why not just convert to nsIFile now, while you're in there? + if (NS_FAILED(StartUp())) + return NS_ERROR_FAILURE; + // failure here means hash table is not started (more serious problem exists) Does that mean that you should be propagating the error code (such as NS_ERROR_OUT_OF_MEMORY) back to the caller, for better diagnostics? + return rv; Do you really want to return NS_OK, or whatever success code CopyCharPref happens to return? NS_IF_RELEASE(mFileSpec); Not changed, but: use nsCOMPtr? + if (NS_FAILED(fileSpec->ResolveSymlink())) + return NS_ERROR_FAILURE; + if (!Exists(fileSpec)) - return PREF_ERROR; + return NS_ERROR_FAILURE; char* readBuf; if (NS_FAILED(fileSpec->GetFileContents(&readBuf))) - return PREF_ERROR; + return NS_ERROR_FAILURE; + 1) You can unify all of these to improve readability: if (NS_FAILED(fileSpec->ResolveSymlink()) || !Exists(fileSpec) || NS_FAILED(fileSpec->GetFileContents(&readBuf))) return NS_ERROR_FAILURE; 2) Could you use nsXPIDLCString here, so that you don't have to free the readBuf by hand, and you use the shared allocator correctly? + /* Unobscure file by subtracting some value from every char. */ + long i; + for (i = 0; i < fileLength; i++) + readBuf[i] -= obscure_value; - - success = ( PL_strncmp((const char*) buf + 3, szHash, (PRUint32)(hash_length - 4)) == 0 ); + if(!readBuf) + return NS_ERROR_FAILURE; Unless I'm _totally_ misreading this diff, you're: 1) checking for a null readBuf _after_ you dereference it, and 2) never comparing it to anything What's the purpose of VerifyLockFileSpec? - PR_FALSE) == PREF_NOERROR); + PR_FALSE) == NS_OK); alecf asked you to use NS_SUCCEEDED rather than comparison with NS_OK, I think.

the reason for not moving to nsIFile is because after speaking with Brian Nesse (who is converting nsPref.cpp to nsPrefBranch.cpp and nsPrefService.cpp) to change it would mean changing it in several other functions, functions that are called by practically everyone calling into the prefs framework - which would be a major task... and out of the scope of this fix. 1) We don't need more debug spew. - done 2) You check NS_SUCCEEDED before your console message - done Here, and in the code quoted above, you diverge from the dominant brace - done 1) _nsIFileToFileSpec is a non-ANSI name - see above 2) I asked about this in my previous comments - see above Does that mean that you should be propagating the error code - there are a host of reasons (not just memory) that could case a failure Do you really want to return NS_OK - Yes... at this point NS_IF_RELEASE(mFileSpec); Not changed, but: use nsCOMPtr? - Brian Nesse is doing this as part of his work with nsPrefBranch/nsPrefService 1) You can unify all of these to improve readability: - done 2) Could you use nsXPIDLCString here - no because I couldn't figure out a way to get nsXPIDLCString to allow me a way to edit the string readBuf (get() returns a const char *) What's the purpose of VerifyLockFileSpec? - to unhash the readBuf (comment exists) alecf asked you to use NS_SUCCEEDED rather than comparison with NS_OK - done diff's on their way (when I am done with testing)

this makes no sense: + if(NS_FAILED(rv = CopyCharPref("general.config.filename",getter_Copies(lockFileName)) && (lockFileName))) + return rv; + and is totally unreadable. try: + rv = CopyCharPref("general.config.filename", getter_Copies(lockFileName)); + if (NS_FAILED(rv) || !lockFileName) return NS_OK; you should follow the same pattern for the "general.config.vendor" pref as well, and avoid the indented if and invalid success value if lockVendor is null. you're still returning PREF_ERROR from pref_OpenFileSpec, which you changed to be an nsresult. while you're touching that code, can you remove the XP_WIN code which pops up a MessageBox? Also, your tabs are STILL messed up on some lines, and you're removing the broken GetVersionNumber() foo that rickg checked in yesterday. This makes me think that you are editing the file and then whacking it back down on top of what's already in CVS...and worries me that there are other bugs that you're backing out. do you read your own diffs? You should ALWAYS read over the whole diff before attaching it.

eclient issue...adding trix and I to Cc.

waiting for sr & r

r=bnesse

sr=alecf with a few minor comments: - do NOT check in all.js - there is no reason to add extra comments to that file. There should be documentation which supports it instead - the comment that refers to mozilla vs. 'netscp6' - please just explain that if the pref exists, then read the config file - there may be other clients besides netscape 6 (or 6.5, or 7.0, etc) which use this file - regarding the actual name of the .cfg file - let's call it something better than "netscp6" - netscape 6 has already been released - Netscape is already working on Netscape 6.5, and what are you going to do when it's called "Netscape 7"? - At some point, we really need to move this stuff out of nsAppRunner.cpp into an app-startup system or profile-startup mechanism.. not now obviously, but it's something to plan for.

Given alecf's conditions, a new patch should be attached before checkin. /be

In 4.x we had netscape.exe and netscape.cfg. For 6.x we have netscp6.exe and propose netscp6.cfg as the config file name. Logic being: the config file has the same name as the exe file, but with cfg extension. Are we changing the Windows exe name for 6.5?

I doubt we're changing the .exe name, but just because someone made a bad choice there doesn't mean we have to continue mirroring it... the only reason the old netscape.exe matched the old netscape.cfg is that both were produced by a company called "Netscape" :) we don't go calling the prefs file netscape.js/netscp6.js nor do we call the history file netscape.dat/netscp6.dat - let's break that habit right now..

Actually, I'm not sure this discussion of the "naming of the cfg file" isn't moot. Since we don't include the pref in the all.js (or all-ns.js) file, and the preference can be named anything the e-customer wants to.... (and everyone else won't bother even having one).... Let e-customers figure out for themselves what to name it. Fix was landed by mitesh yesterday and the tree is green. Marking FIXED.

over to trix for verification...adding sarah to Cc.

verified on 20010515 builds on win32,mac, & linux_glibc