This bug originates from investigations during bug 58696 (see https://bugzilla.mozilla.org/show_bug.cgi?id=58696#c34). This will basically be a tidy up patch to hopefully reduce code base and memory usage consisting of: 1) remove the hidden call to migrate from the constructor of nsILDAPPrefsService and call it explictly where required. 2) There is some near duplicate code in: http://lxr.mozilla.org/seamonkey/source/mailnews/addrbook/src/nsLDAPPrefsService.js#76 (in the constructor) and http://lxr.mozilla.org/seamonkey/source/mailnews/addrbook/prefs/resources/content/pref-directory.js#141 which is sorting out the available functions. We could probably combine into one function in nsILDAPPrefsService. 3) The constructor code could then be moved into migrate - as that is the only bit that actually uses it, and we won't then need to store availDirectories in nsILDAPPrefsService.

This patch seperates out the migrate function from the constructor of the LDAP Prefs service. In doing this, we are able to clear down the constructor and not unnecessarily store a list of available directories. There is still some duplication of processing of lists, but that's for the next patch.

Comment on attachment 201334 [details] [diff] [review] Seperate out the ldap migrate function from the constructor how about calling the migrate method something like "migratePrefsIfNeeded" or something, since we will rarely actually migrate prefs.

Same as before, but changed name of migrate function as per David's suggestion.

Comment on attachment 201456 [details] [diff] [review] Seperate out the ldap migrate function v2 (checked in) >Index: mailnews/addrbook/resources/content/addressbook.js >=================================================================== >+ if (ldapPrefs) >+ ldapPrefs.migratePrefsIfNeeded(); >+ I tend to prefer bracing all "then" and "else" clauses, because it helps avoid mistakes in future edits of the code when someone wants to add another statement to the clause in question. But up to you. The other thing you might consider is that you use dump() in various places to report errors. If you think it would help end users / sysadmins diagnose issues, you could alternately use Components.utils.reportError(), which will send a message to the JS console.

Comment on attachment 201456 [details] [diff] [review] Seperate out the ldap migrate function v2 (checked in) Checked in the first patch with dmose comments addressed. /cvsroot/mozilla/mailnews/addrbook/prefs/resources/content/pref-directory.js,v <-- pref-directory.js new revision: 1.27; previous revision: 1.26 done Checking in mailnews/addrbook/public/nsILDAPPrefsService.idl; /cvsroot/mozilla/mailnews/addrbook/public/nsILDAPPrefsService.idl,v <-- nsILDAPPrefsService.idl new revision: 1.4; previous revision: 1.3 done Checking in mailnews/addrbook/resources/content/addressbook.js; /cvsroot/mozilla/mailnews/addrbook/resources/content/addressbook.js,v <-- addressbook.js new revision: 1.119; previous revision: 1.118 done Checking in mailnews/addrbook/src/nsLDAPPrefsService.js; /cvsroot/mozilla/mailnews/addrbook/src/nsLDAPPrefsService.js,v <-- nsLDAPPrefsService.js new revision: 1.19; previous revision: 1.18 done Checking in mailnews/base/src/nsMessengerMigrator.cpp; /cvsroot/mozilla/mailnews/base/src/nsMessengerMigrator.cpp,v <-- nsMessengerMigrator.cpp new revision: 1.116; previous revision: 1.115 done Checking in mailnews/compose/resources/content/MsgComposeCommands.js; /cvsroot/mozilla/mailnews/compose/resources/content/MsgComposeCommands.js,v <-- MsgComposeCommands.js new revision: 1.373; previous revision: 1.372 done Checking in mail/components/addrbook/content/addressbook.js; /cvsroot/mozilla/mail/components/addrbook/content/addressbook.js,v <-- addressbook.js new revision: 1.24; previous revision: 1.23 done Checking in mail/components/compose/content/MsgComposeCommands.js; /cvsroot/mozilla/mail/components/compose/content/MsgComposeCommands.js,v <-- MsgComposeCommands.js new revision: 1.81; previous revision: 1.80 done Checking in mail/components/preferences/compose.js; /cvsroot/mozilla/mail/components/preferences/compose.js,v <-- compose.js new revision: 1.7; previous revision: 1.6

Due to the various bugs that have fixed our preferences, we no longer need nsILDAPPrefsService :-) Time for it to go.

Comment on attachment 337712 [details] [diff] [review] [checked in] Remove the LDAP Prefs Service yay!

Comment on attachment 337712 [details] [diff] [review] [checked in] Remove the LDAP Prefs Service You may (or may not) want to remove a couple of other references I spotted: mailnews/mailnews.pkg mozilla/modules/plugin/os2wrapper/moz_IDs_Input.lst

Comment on attachment 337712 [details] [diff] [review] [checked in] Remove the LDAP Prefs Service Checked in changeset id: 308:0d42e942671e

This is now fixed (you can't do many more revisions on it once you've removed it ;-) ).