Closed Bug 46863 Opened 24 years ago Closed 24 years ago

nsIPref needs to be split and cleaned up

Categories

(Core :: Preferences: Backend, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla0.9.1

People

(Reporter: jud, Assigned: bnesse)

References

Details

(Keywords: arch, embed)

Attachments

(19 files)

(deleted), text/plain
Details
(deleted), text/plain
Details
(deleted), patch
Details | Diff | Splinter Review
(deleted), text/plain
Details
(deleted), text/plain
Details
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), text/plain
Details
(deleted), text/plain
Details
(deleted), patch
Details | Diff | Splinter Review
(deleted), text/plain
Details
(deleted), patch
Details | Diff | Splinter Review
(deleted), text/plain
Details
(deleted), text/plain
Details
(deleted), patch
Details | Diff | Splinter Review
This will allow embeddors to use a simple interface versus our beefy nsIPref interface.
I checked in my original proposal for these interfaces a while back in modules/libpref/public but I haven't gotten a chance to hook them up (and spam the tree with the required changes)
Keywords: embed
Keywords: nsbeta3
The idea is to have simple getters and setter routines on a public pref interface. I wonder if we should just combine prefs and profile interfaces? It seems we have such a tight relationship between the two that we should just make them a single iface. The public iface needs to provide a serialization (SavePrefsFile()) method on it so the embeddor can flush changes to disk at their leisure.
oh god no. Please don't mix prefs and profiles. Prefs are just one aspect of the user's personal data that has to do with profiles, not to mention neither nsIPref interface nor the implementation depends on nsIProfile, and I'd like to keep it that way. Also, we have a scheme (nsIProfileStartupListener.idl) that can be used to break the tie between nsProfile.cpp and nsIPref
-> conrad.
Assignee: valeski → conrad
Whiteboard: [nsbeta3+]
Target Milestone: --- → M18
Yes, the prefs and profiles should be independent. The prefs are a tree of settings which embedders should use. Changing the profile will change which tree is currently active to but shouldn't change the way one gets/sets with the active tree. What needs to be done is make the prefs use nsIProfileStartupListener. I'll work on this.
changing to new email
Assignee: conrad → ccarlen
re-accepted at new email
Status: NEW → ASSIGNED
Priority: P3 → P1
per email with Jud, changing nsbeta3+ to nsbeta3- on all "embed" keyword bugs since embedding changes will not be made in the mn6 branch. If you feel this bug fix needs to go into mn6 branch, please list the reasons/user impact/risk and nominate for rtm. Thanks.
Whiteboard: [nsbeta3+] → [nsbeta3-]
Target Milestone: M18 → mozilla0.8
Updating QA Contact
QA Contact: jrgm → mdunn
Blocks: 64833
-> mozilla0.9 Needs some more thought. Also, could have massive repercussions on the tree.
Target Milestone: mozilla0.8 → mozilla0.9
My thoughts on this (which I've expressed elsewhere) split this up into: nsIPrefService - general pref-file related stuff - for reading users prefs, starting up, shutting down, and so forth. You would also attain pref branches (see below) from the pref service. nsIPrefBranch - a "branch" of the prefs "tree". For example "" is the root, and the "browser." branch refers to all prefs that begin with "browser." such as "browser.startup.homepage" nsIPrefMisc - for random stuff that probabaly won't be frozen. nsIPrefBranch would contain the usual getBoolPref(), getIntPref(), etc. as well as callback-related stuff. Initially, the current nsPref object would simply implement all three of these interfaces... that way callers could be initially updated to just ask for the same pref service (i.e. with the same ContractID) but on one of these new interfaces. Later, when this is done, callers could be updated to retrieve the "branch" of prefs they care about, and then deal only with that. nsPrefBranch would become a seperate class which callers could hold strong references to. (instead of holding strong references to an nsIPrefService)
Reassigning to Brian since he's already working on it - doing what alecf suggested 2001-01-23 11:52.
Assignee: ccarlen → bnesse
Status: ASSIGNED → NEW
Component: Embedding APIs → Preferences: Backend
Summary: nsIPref needs to be replaced by nsIPrefsManager and nsIBrowserPrefsManager → nsIPref needs to be split and cleaned up
Accepting, correcting fields, and resetting keywords, as per instructions and adding Conrad to the cc list (because I know he loves receiving these emails :))
Status: NEW → ASSIGNED
Keywords: nsbeta3nsbeta1
OS: Windows 98 → All
Hardware: PC → All
Whiteboard: [nsbeta3-]
Blocks: 70229
An update... The bulk of this is completed. For anyone who cares to look, I have added nsIPrefService.idl and nsIPrefBranch.idl to /libpref/public. I have also added nsPrefService.cpp/.h and nsPrefBranch.cpp/.h to libpref/src. I have versions of nsIPref.idl and nsPref.cpp which wrap these objects and support minimal changes to tree for landing purposes. I will attach them (along with diffs for the rest of the tree(s) after doing some testing...
Many changes based on feedback from previous checkin. Added nsIPrefBranchInternal.idl, nsIPrefLocalizedString.idl, and nsPrefsFactory.cpp. Also updated most of the previously added files. Also adding attachments for new nsIPref.idl, nsPref.cpp and diffs for Mozilla tree changes to make it all build.
Attached file new nsIPref.idl (deleted) —
Attached file new nsPref.cpp (deleted) —
Attached patch mozilla tree patches (deleted) — Splinter Review
Attached file latest revision of nsIPref.idl (deleted) —
Attached file latest revision of nsPref.cpp (deleted) —
I have updated all of the attachments and files in CVS based on the latest round of feedback and changes to the CVS trees. Please look a look at this stuff with a eye towards r/sr. Hoping to get this stuff landed ASAP.
Blocks: 21135
nsIPrefBranch - * the "branch level operations"... please document whether or not the format requires a pre-ceeding '.' or not. nsIPrefBranch.idl (w/ the above addressed) and nsIPrefService.idl r=valeski. I'm going to have to defer to other's for the rest (sorry, time constraints).
Good idea Jud... I should have thought of that before. Conrad (I believe) had also suggested noting the interfaces supported by Get/SetComplexValue at some point. I'll do that as well. Added new diffs for JavaScript changes because I'm stupid...
Comments added to nsIPrefBranch.idl. Please let me know if they fit the bill for you.
comments look good to me. thanks. condense them < 80 columns wide though, there are several lines in the file that overspill editor buffers.
Attached file Latest version of nsIPref.idl (deleted) —
Ok all, I realize that everyone is doomed right now, but I really need to get this stuff reviewed and super-reviewed soon so it can make the 0.9 deadline. Your cooperation is greatly appreciated :)
nsIPrefBranch - * maybe just add that reset resets prefs to their default values, just to be explicit. nsIPrefService - * looks good /cvsroot/mozilla/modules/libpref/public/MANIFEST_IDL - * looks like you need to add the other idls here. SavePrefFile()/ReadUserPrefs() - You're having to change callsites all over the place just to pass in null, why? nsIPref.idl - has duplicate methods that already hang off nsIPrefService - readUserPrefs() - readConfigFile() - resetPrefs() - resetUserPrefs() - savePrefFile() - getBranch() - getDefaultBranch() and these already hang off of nsIPrefBranchInternal - void addObserver(in string aDomain, in nsIObserver aObserver); - void removeObserver(in string aDomain, in nsIObserver aObserver); You need to remove those methods from nsIPref, in favor of the new iface methods. Also, nsIPrefService should be the iface registered w/ the prefs contract id, and callers should be QI'ing nsIPrefSerivice for the internal stuff. This ensures that nsIPref is truly burried and that public prefs interraction only occurs through nsIPrefService.
> * maybe just add that reset resets prefs to their default values, consider it done. > /cvsroot/mozilla/modules/libpref/public/MANIFEST_IDL - > * looks like you need to add the other idls here. I was under the impression that I only needed to Manifest items that I thought others might try and build off of. Based on this I added the one I thought might be useful and removed the one that I know mstoltz doesn't want people to be using. Is my understanding of this incorrect? > SavePrefFile()/ReadUserPrefs() - > You're having to change callsites all over the place just to pass in null, why? SavePrefFile and SavePrefFileAs(filename) were combined into one function as were ReadUserPrefs() and ReadUserPrefsFrom(filename). The remaining functions understand null as "use the default file". > nsIPref.idl - > has duplicate methods that already hang off nsIPrefService The purpose for the duplicate methods is to retain maximum backward compatibility while allowing the landing to happen with a minimum impact on the tree. For instance, also note that many of these are declared Uppercase to retain compatibility with the current JS usage. The long term goal is not to bury nsIPref, but to remove it altogether.
I believe some mac projects still use the MANIFEST_IDL files for idl compilation, you'll need to confirm this case w/ a mac person. either way, you need to either add the new idls to the mac idl file or to a mac project; so the mac builds them. > SavePrefFile and SavePrefFileAs(filename) were combined into one function as > were ReadUserPrefs() and ReadUserPrefsFrom(filename). The remaining functions > understand null as "use the default file". it would be nice if null were assumed, but I guess that's not doable in idl :-(. regarding dup functions. I figured that was what you were doing. It's going to lead to heavy confusion. when can we count on nsIPref being gone? I can see people going bonkers over this for awhile (until nsIPref is gone). interface transitions are hard (span the tree w/ callsites), that's part of the game. are you planning on ditching nsIPref soon?
A file needs to be in a MANIFEST_IDL only if it's included by other .idl files which are not in the same IDL project as those files are.
the reason for keeping nsIPref around for a little while is just to make the carpool landing easier - we can land the new APIs in one landing, then revert the tree in a second landing - better to have two small landings than one giant one. However, we really need to make sure that nsIPref gets out of the tree very quickly (as in 1 week or so from the initial landing) just to avoid the confusion jud refers to... In addition to big screaming commengs in nsIPref.idl, we need to post announcements to seamonkey-internal, mozilla-xpfe, mozilla-prefs, mozilla-xpcom, and maybe mozilla-mail-news to let people know that nsIPref is deprecated and to use the new nsIPrefService/nsIPrefBranch. We also need to explain in nsIPrefService.idl/nsIPrefBranch.idl how to actually use them (some sample JavaScript should be sufficient)
Jud, fortunately I am a Mac person. :) I just decided not to post the project file(s) because they are binaries, not text. I agree on the null, but I also assumed that it couldn't be done in idl. My plan is to start the work of converting all of the modules using nsIPref as soon as this code lands. As Alec says the sooner the better.
cool. for carpool ease, sounds good. let's erradicate nsIPref asap though, or we'll spend a lot of time explaining to people what they're *supposed* to do :-)
Blocks: 5132
*** Bug 46487 has been marked as a duplicate of this bug. ***
Keywords: arch
*** Bug 27159 has been marked as a duplicate of this bug. ***
*** Bug 13266 has been marked as a duplicate of this bug. ***
ok, everything looks good.. my ONLY request at this point is that you match up the root pref branch in GetBranch()/GetDefaultBranch()... i.e. if the user passes in "", return mRootBranch - since the root is the most common, I think that's the most critical one to worry about caching - no need to create extra objects just because of that. Also (not critical) it would be nice if you could comment on what getBranch/GetDefaultBranch are, and what the differences are, in nsIPrefService.idl
*** Bug 72007 has been marked as a duplicate of this bug. ***
r=valeski. erradicate nsIPref ASAP though :-).
Changing milestone to 0.9.1 as sr not received in time for 0.9 cutoff. Will try again after the tree re-opens for 0.9.1 checkins.
So this doesn't compile on windows. the issues are: - unknown #pragma junk - what are those? - in stuff like GetLocalizedUnicharPref, GetFileXPref, etc, you're using raw COM pointers in temporary variables, which don't automatically cast to void **.. my suggestion is to switch to nsCOMPtr<>, which automatically casts to void **. the once place you can't do this is in GetFileXPref() where you must manually cast _retval.
crap. I know I took out some of those (#pragmas). I guess I missed some. I'll look at the nsCOMPtr stuff. I finally got my windows build working, so I should be able to see it for myself now.
you can use #if 0 #pragma mark - #endif if you want to
Probably better to just yank it. I've seen other (unix) compilers which seem to ignore #ifdef code when they are processing code (got bitten in 4.x on that one.)
After some problems with patch (funny linefeed problem) and warnings from patch about missing header information, the build fails with the following errors. nsPref.cpp:77:43: warning: pasting would not give a valid preprocessing token nsPref.cpp:77:43: warning: pasting would not give a valid preprocessing token nsPref.cpp:77:43: warning: pasting would not give a valid preprocessing token nsPref.cpp:77:43: warning: pasting would not give a valid preprocessing token nsPref.cpp:77:43: warning: pasting would not give a valid preprocessing token nsPref.cpp:77:43: warning: pasting would not give a valid preprocessing token nsPref.cpp:77:43: warning: pasting would not give a valid preprocessing token nsPref.cpp: In method `nsresult nsPref::CopyUnicharPref (const char *, PRUnichar **)': nsPref.cpp:385: no matching function for call to `nsDerivedSafe<nsIPrefBranch>::GetComplexValue (const char *&, const nsID &, nsISupportsWString **)' ../../../dist/include/nsIPrefBranch.h:75: candidates are: nsresult nsIPrefBranch::GetComplexValue (const char *, const nsIID &, void **) nsPref.cpp: In method `nsresult nsPref::CopyDefaultUnicharPref (const char *, PRUnichar **)': nsPref.cpp:399: no matching function for call to `nsDerivedSafe<nsIPrefBranch>::GetComplexValue (const char *&, const nsID &, nsISupportsWString **)' ../../../dist/include/nsIPrefBranch.h:75: candidates are: nsresult nsIPrefBranch::GetComplexValue (const char *, const nsIID &, void **) nsPref.cpp: In method `nsresult nsPref::GetLocalizedUnicharPref (const char *, PRUnichar **)': nsPref.cpp:441: no matching function for call to `nsDerivedSafe<nsIPrefBranch>::GetComplexValue (const char *&, const nsID &, nsIPrefLocalizedString **)' ../../../dist/include/nsIPrefBranch.h:75: candidates are: nsresult nsIPrefBranch::GetComplexValue (const char *, const nsIID &, void **) nsPref.cpp: In method `nsresult nsPref::GetDefaultLocalizedUnicharPref (const char *, PRUnichar **)': nsPref.cpp:455: no matching function for call to `nsDerivedSafe<nsIPrefBranch>::GetComplexValue (const char *&, const nsID &, nsIPrefLocalizedString **)' ../../../dist/include/nsIPrefBranch.h:75: candidates are: nsresult nsIPrefBranch::GetComplexValue (const char *, const nsIID &, void **) nsPref.cpp: In method `nsresult nsPref::GetFilePref (const char *, nsIFileSpec **)': nsPref.cpp:469: no matching function for call to `nsDerivedSafe<nsIPrefBranch>::GetComplexValue (const char *&, const nsID &, nsIFileSpec **&)' ../../../dist/include/nsIPrefBranch.h:75: candidates are: nsresult nsIPrefBranch::GetComplexValue (const char *, const nsIID &, void **) nsPref.cpp: In method `nsresult nsPref::GetFileXPref (const char *, nsILocalFile **)': nsPref.cpp:493: no matching function for call to `nsDerivedSafe<nsIPrefBranch>::GetComplexValue (const char *&, const nsID &, nsILocalFile **&)' ../../../dist/include/nsIPrefBranch.h:75: candidates are: nsresult nsIPrefBranch::GetComplexValue (const char *, const nsIID &, void **) nsPref.cpp: In method `nsresult nsPref::CreateChildList (const char *, char **)': nsPref.cpp:572: warning: comparison between signed and unsigned integer expressions nsPref.cpp: In method `nsresult nsPref::EnumerateChildren (const char *, void (*) (const char *, void *), void *)': nsPref.cpp:638: warning: comparison between signed and unsigned integer expressions make[2]: *** [nsPref.o] Error 1 make[2]: Leaving directory `/builds/eddyk/daily/mozilla/modules/libpref/src' make[1]: *** [install] Error 2 make[1]: Leaving directory `/builds/eddyk/daily/mozilla/modules/libpref' make: *** [install] Error 2
I think that fixing my above issues on win32 would also fix all of the linux issues.
I have updated nsIPrefService.cpp to remove the offending pragma's and added attachment with the fixed version of nsPref.cpp. If anyone wants to try these tonight, have at it, else I will see to it in the morning.
oh, one more problem with this patch on windows - you use FALSE when you should be using PR_FALSE
Ok, we have builds on all 3 platforms as per Drivers request. I'm checking once more to make sure the files haven't changed since this morning and then I'm landing it.
Assuming all goes well a=blizzard for 0.9.
All changes landed in both trees.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Reopening: fix backed out due to regressions on Linux and Windows.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Additional info from bryner: >So far we know that it broke homepage prefs and font prefs (on both Windows > and Linux), and middle mouse paste on Linux (this is also pref-controlled). Unlikely this will make 0.9 now... moving milestone.
Target Milestone: mozilla0.9 → mozilla0.9.1
I heard that this might be back in for 0.9 Is that the case, and if so can we get a status update please.
I believe jpm and I have found the cause for the problem. This diff is from a tree that has brian's changes. The interesting line is at the end, where we use Append instead of SetLeafName. It seems that using SetLeafName prevents us from reading the unix.js file. I don't know anything about nsIFile so I don't know if this is proper, but it seems to have fixed the paste and font problem on linux. Index: nsPrefService.cpp =================================================================== RCS file: /cvsroot/mozilla/modules/libpref/src/nsPrefService.cpp,v retrieving revision 1.8 diff -u -r1.8 nsPrefService.cpp --- nsPrefService.cpp 2001/04/19 03:49:00 1.8 +++ nsPrefService.cpp 2001/04/21 06:20:50 @@ -74,7 +74,7 @@ NS_INIT_REFCNT(); - rootBranch = new nsPrefBranch("", FALSE); + rootBranch = new nsPrefBranch("", PR_FALSE); mRootBranch = (nsIPrefBranch *)rootBranch; } @@ -693,7 +693,8 @@ // Finally, parse any other special files (platform-specific ones). for (k = 1; k < (int) (sizeof(specialFiles) / sizeof(char*)); k++) { - rv = aFile3->SetLeafName((char*)specialFiles[k]); + rv = aFile3->Append((char*)specialFiles[k]); + if (NS_SUCCEEDED(rv)) { #ifdef DEBUG_prefs printf("Parsing %s\n", specialFiles[k]);
You're totally right. SetLeafName is not what should be used here. Above, aFile3 starts as a directory from NS_GetSpecialDirectory SetLeafName changes the name of that dir (internally, in the file object). Append, on the other hand, makes a child node, i.e. aFile3/specialFile. Append is what we want.
What happens when the loop is iterated? Wouldn't the second Append() try to append to the first file (.../.../unix.js), not to the parent directory? Should NS_GetSpecialDirectory() be called inside the loop for every instance of aFile3?
Would SetLeafName() have worked inside the loop if Append() (even with a dummy file name) had been called after NS_GetSpecialDirectory(), before the loop?
Also see bug 76863, which was caused by this checkin (crash closing prefs -- has a stack trace).
Add bug 76851 to the list of things that should be tested before this is checked in again.
Status update: Fixes for all of the problems detailed above have been put in place. Tests against all issues, including bug 76863 and bug 76851, have been done on Mac, Windows, and Linux.
Attached patch Latest diffs for Mozilla tree. (deleted) — Splinter Review
Attached file Latest version of nsIPref.idl (deleted) —
Attached file Latest version of nsPreff.cpp (deleted) —
Patches attached to insure that everything is up to date. Latest versions of PrefBranch, Service, and Factory are in CVS. Detail of changes since 4/18: - Fixes in PrefService.cpp for .js loading and localized preference bugs. - Fixes in PrefBranch.cpp for Observer related bug. - Fixes in nsIPref.idl and prefapi.h for protection against function redefinition. valeski has requested that this land as soon as the tree opens for 0.9.1, so reviews and sr's would be greatly appreciated.
Opps. The localized preference bug fix is in nsPrefBranch, not nsPrefService.
in /mozilla/modules/libpref/src/Makefile.in you add the dep on PROFILE. I just spoke w/ conrad on this topic and he believes there's a stray "#include "nsIProfile" " somehwere in teh prefs code that can be removed and thus that dependency doesn't have to be made. Can you pull that #include and dep?
Found it. nsPrefService was including nsIProfileChangeStatus to get the observer strings from the header. Conrad changed this in nsPref with his checkin for bug 65212. I merged the string changes into nsPrefService but didn't remove the include. #include and PROFILE dependency both removed.
ok, sounds good. sr=alecf with the dependancies removed, etc.
r=valeski. thanks.
Coffee is crashing on startup, here, after this landing: #0 0x4013ff52 in ?? () from /builds/mcafee/cmonkey/mozilla/dist/bin/libxpcom.so #1 0x40bcd8cc in nsCOMPtr<nsIFileSpec>::assign_from_helper (this=0xbfffeda4, helper=@0xbfffeda8, aIID=@0x4019a3cc) at ../../../dist/include/nsCOMPtr.h:974 #2 0x40bce966 in nsCOMPtr<nsIFileSpec>::nsCOMPtr (this=0xbfffeda4, helper=@0xbfffeda8) at ../../../dist/include/nsCOMPtr.h:567 #3 0x40bc6550 in nsPrefBranch::SetComplexValue (this=0x8111a00, aPrefName=0x4090d100 "nglayout.debug.crossing_event_dumping", aType=@0x4019a3cc, aValue=0x408e0c9c) at nsPrefBranch.cpp:406 #4 0x40bca97d in nsPrefService::SetComplexValue (this=0x81119d8, aPrefName=0x4090d100 "nglayout.debug.crossing_event_dumping", aType=@0x4019a3cc, aValue=0x408e0c9c) at nsPrefService.h:40 #5 0x40bc3929 in nsPref::SetFilePref (this=0x8111958, pref=0x4090d100 "nglayout.debug.crossing_event_dumping", value=0x408e0c9c, setDefault=0) at nsPref.cpp:482 #6 0x408e0f11 in ?? () from /builds/mcafee/cmonkey/mozilla/dist/bin/components/libwidget_gtk.so #7 0x408deda3 in ?? () from /builds/mcafee/cmonkey/mozilla/dist/bin/components/libwidget_gtk.so #8 0x408ca366 in ?? () from /builds/mcafee/cmonkey/mozilla/dist/bin/components/libwidget_gtk.so
probably not so helpful but on a current non-debug it dumps core on startup. Backtrace: #0 0x407f69bf in nsPref::nsPref () from components/libpref.so #1 0x407f8685 in nsPref::GetInstance () from components/libpref.so #2 0x407f86d5 in nsPrefConstructor () from components/libpref.so #3 0x400b697c in nsGenericFactory::CreateInstance () from libxpcom.so #4 0x400b466a in nsComponentManagerImpl::CreateInstance () from libxpcom.so #5 0x400bb808 in nsComponentManager::CreateInstance () from libxpcom.so #6 0x400bc36f in nsServiceManagerImpl::GetService () from libxpcom.so #7 0x400bc8c0 in nsServiceManager::GetService () from libxpcom.so #8 0x400bbbff in nsGetServiceByCID::operator() () from libxpcom.so #9 0x400c6fd1 in nsCOMPtr_base::assign_from_helper () from libxpcom.so #10 0x4052f8fc in HandleColormapPrefs () from components/libwidget_gtk.so #11 0x4052fb01 in nsAppShell::Create () from components/libwidget_gtk.so #12 0x4050ec73 in nsAppShellService::Initialize () from components/libnsappshell.so #13 0x804cfa2 in main1 () #14 0x804d945 in main ()
So, the second stack crawl seems to imply a total failure to create the nsPref object. The first stack crawl is interesting because the pref that it is having trouble with is NOT a file preference. It's a BOOL pref. This seems to imply that that the whole object lookup table is trashed somehow.
cd dist/bin rm component.reg start your build worked for me.
So what's the deal here? Brian, are you working on this? These same changes caused many problems last time; they were purportedly tested XP this time around, and yet Windows and Linux are now crashing on startup. I don't think forcing everyone to clobber is an acceptable solution to this problem.
Didn't appear to work for mcaffe. He believes clobber_all is the answer. Unfortunately additional bustage appears to have happened now, so no way of telling whether he was right or not just yet...
Blake, mcaffe seems to think it's a dependency issue, but didn't offer any solutions as to how I could fix the problem or as to what I could have done to avoid it. This code was compiled and run on Linux before checkin. If anyone can offer a solution other than clobbering, I'll listen...
Well, as a wise man once told me, "clobber is not a resolution". If you have introduced, or simply triggered, over-weak dependency rules in the prefs code, _fix_them_. (Cc:ing said wise man.)
Yes, clobbering is not sufficient to mark a problem as fixed. We shouldn't have to touch the tinderbox and require a full world rebuild for everyone everytime someone changes an API. I'm pulling a 11:00 tree to reapply just those changes to find the dep problem. Bnesse, I suggest that you do the same. Here's something specific to try. With a completed pre-api change build, touch the files that would have been applied by the patch. Then rebuild and keep a log. Then apply the patch, rebuild and keep a build log. Then clobber your tree and rebuild. Touch the files listed in the patch again. Rebuild keeping a log. Comparing the lists of rebuilt files should give us a starting point to finding out what didn't get rebuilt properly.
data point - pull that died with segfault in linux - rebuilt after removing ./dist/* now works
I'm seeing some pref badness that might be related to this checkin (see bug 77828). Link colors are broken. And, debugging, I note that the pref callback on "browser.visited_color" is being called five (5) times when I confirm the prefs dialog. That seems wrong.
Never mind. The 5 pref callbacks are because there are 5 extant nsPresContexts at that point, each of which has registered a callback.
Not having immediate access to a windows or linux box, I can say that: Ignoring libraries with js only changes for now, all of the linux makefiles with changed project files appear to have the correct "REQUIRES" dependency on "pref". The windows makefiles are a little bit more dicey. They all have, in some combination, some of the following: include <$(DEPTH)\config\config.mak> an LINCS reference (most of which reference "pref", a couple don't) include <$(DEPTH)\config\rules.mak>
There's a discussion of component.reg removal in bug 78011.
The preferences API refactoring has been landed marking as fixed. I have created a new bug (Bug 78522) for the build dependency issues.
Status: REOPENED → RESOLVED
Closed: 24 years ago24 years ago
Resolution: --- → FIXED
reassigned qa contact to Dharma. He's doing nsIPrefs.
QA Contact: mdunn → dsirnapalli
nsIPref is split into nsIPrefBranch and nsIPrefService. verified.
Status: RESOLVED → VERIFIED
No longer blocks: 64833
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: