when I delete addressbooks from the prefs (addressing panel) it doesn't get removed from the directory pane. we're going to have to poke the directory datasource. we'll have to make sure we handle the case where you delete the one you've got selected in the addressbook. we should already be handling this scenario in the addressbook sidebar. we should check with jglick if she wants it to disappear from just autocomplete, but remain in the addressbook (I don't think she will, but we should check.)

it fixes on restart.

reassign QA contact to myself

Agree. If a user removes a directory using either the Prefs: LDAP Directory Servers dialog Or the Address Book window, the directory should be removed from Prefs for autocomplete and the Address Book window. (An alternative we can explore if folks are interested is here: http://www.mozilla.org/mailnews/specs/addressbook/images/LdapSel2.gif http://www.mozilla.org/mailnews/specs/addressbook/#Prefs)

over to srilatha, she's working on making it so "add" from the autocomplete UI shows up (without restart) and making delete work the same may (and modify) is very related.

20020218 builds It doesn't fix on restart for Win2K &Wins NT. Fine with MacOS 9.2 if restart.

The problem it doesn't fix on restart for Win2K & Wins NT is turbo mode. Without turbo mode, it fixes on restart.

Moving to 1.0

Deleting a directory from the preferences window now calls deleteAddressBookPrefs. This a new method I added to nsIAddressBook. This is same as deleteAddressBooks except the first parameter. void deleteAddressBooks(in nsIRDFCompositeDataSource db, in nsISupportsArray parentDir, in nsISupportsArray aResourceArray); + void deleteAddressBookPrefs(in nsIRDFDataSource db, in nsISupportsArray parentDir, in nsISupportsArray aResourceArray); With this patch deleting a directory now goes through the nsDirPrefs code

+ // the rdf service + var RDF = '@mozilla.org/rdf/rdf-service;1' + RDF = Components.classes[RDF].getService(); + RDF = RDF.QueryInterface(Components.interfaces.nsIRDFService); + // get the datasource for the addressdirectory + var addressbookDS = RDF.GetDataSource("rdf:addressdirectory"); + var addressbook = Components.classes["@mozilla.org/addressbook;1"].createInstance(); + addressbook = addressbook.QueryInterface(Components.interfaces.nsIAddressBook); + Shouldnt the above code be in a try - catch block. Same with the 2nd hunk in the patch for this file.

The code that calls the nsDirPrefs and removes the dir entry from the UI should be here. Srilatha told me that it is part of patch in bug # 124059, please make sure the below hunk is part of this patch : NS_IMETHODIMP nsAbBSDirectory::DeleteDirectory(nsIAbDirectory *directory) { nsresult rv = NS_OK; if (!directory) return NS_ERROR_NULL_POINTER; + + // if addressbook is not launched yet mSevers will not be initialized + // callin GetChidNodes will initialize mServers + nsCOMPtr<nsIEnumerator> subDirectories; + rv = GetChildNodes(getter_AddRefs(subDirectories)); + NS_ENSURE_SUCCESS(rv, rv); DIR_Server *server = nsnull; nsVoidKey key((void *)directory); server = (DIR_Server* )mServers.Get (&key); For the above code, since GetChildNodes should be called only if mServers is not initialized, check the mInitialized member before making this call to avoid unnecessary call if not required. Also Srilatha explained me that in case if delete is done from prefs instead of directly accessing nsDirPrefs from the C++ function DeleteAddressBookPrefs() in the patch (id=73918), she needs to go through the RDFResource impl for this (nsABSDirectory) since it is possible that the Address Book window is open at that time and it should be updated. Please document this in your code, maybe in function DeleteAddressBookPrefs().

Discussed in Mail News bug mtg with Engineering QA Mktng PjM. Decided to ADT3 this bug.

20020402 trunk build on WIn2k It fixes on restart for Wins in turbo mode (&non turbo mode).

20020415 1.0 branch build In turbo mode, deleting LDAP Directory in Prefs, the Directory Server still shows up in Address book window, addressbook Sidebar and Select Address dlg after restart.

Changing nsbeta1+ [adt3] bugs to nsbeta1- on behalf of the adt. If you have any questions about this, please email adt@netscape.com. You can search for "changing adt3 bugs" to quickly find and delete these bug mails.

Changing nsbeta1+ [adt3] bugs to nsbeta1- on behalf of the adt. If you have any questions about this, please email adt@netscape.com. You can search for "changing adt3 bugs" to quickly find and delete these bug mails.

+ // the rdf service + var RDF = '@mozilla.org/rdf/rdf-service;1' + RDF = Components.classes[RDF].getService(); + RDF = RDF.QueryInterface(Components.interfaces.nsIRDFService); instead of using the same RDF var repeatedly maybe you can combine the above three lines into one statement. Also please add comments for function DeleteAddressBookPrefs() as I mentioned above in comment # 10. do the above and you have r=rdayal.

updated the patch based on rajiv's comments.

Comment on attachment 82856 [details] [diff] [review] updated patch. r=rdayal based on comment #17

Looks good. Comments: 1) maybe it's my C / C++ upbringing, but I'd rather you declare var RDF and var addressbook in the proper scope. I know it works from JS the way you did it, but it makes the code harder to read. You declare them in the first try block, and use them later. 2) + var addressbook = Components.classes["@mozilla.org/addressbook;1"].createInstance(); + addressbook = addressbook.QueryInterface(Components.interfaces.nsIAddressBook); Why not: + var addressbook = Components.classes["@mozilla.org/addressbook;1"].createInstance(Components.interfaces.nsIAddressBook); 3) + catch(ex){} since we don't expect failure, I'd add a dump statement to your two catch blocks. Failing silently makes it hard to debug. 4) + var parentDir = RDF.GetResource("moz-abdirectory://").QueryInterface(Components.interfaces.nsIAbDirectory); can you add a comment about how "moz-abdirectory://" is the special URI for the boot strap directory? 5) + var selectedABURI = "moz-abldapdirectory://" + gDeletedDirectories[i]; can you add a comment about how the resource for a LDAP directory is that schema + the prefname? 6) void deleteAddressBooks(in nsIRDFCompositeDataSource db, in nsISupportsArray parentDir, in nsISupportsArray aResourceArray); + void deleteAddressBookPrefs(in nsIRDFDataSource db, in nsISupportsArray parentDir, in nsISupportsArray aResourceArray); The implementations for deleteAddressBooks and deleteAddressBookPrefs look very similar. What if you just changed deleteAddressBooks() to take a nsIRDFDataSource instead of a nsIRDFCompositeDataSource? You might have to add a QI in there. I think it's possible to not add this new method. About the new method, I don't think having prefs in the name is correct. That's an implementation detail. (that we use prefs as how we persist addressbooks.) That implementation detail should not be part of our interface. 7) It looks like you copied the existing bad style. But as a style issue, I like the aFoo notation for arguments. It makes code easier to read. If you are able to combine deleteAddressBooks() and deleteAddressBooksPrefs(), consider switching all the arguments to the aFoo style.

updated the patch based on Seth's comments. In this patch we are not using DeleteAddressbookPrefs but I changed the parameter of DeleteAddressBooks.

Comment on attachment 83125 [details] [diff] [review] patch addressing Seth's comments sr=sspitzer one comment: + catch(ex){ + dump("Failed to get RDF Service or addressbook \n"); + } consider: + catch(ex){ + dump("Failed to get RDF Service or addressbook: " + ex + "\n"); + } That way, we'll know what the exception was. No need for a new patch.

Comment on attachment 83125 [details] [diff] [review] patch addressing Seth's comments carrying over r=rdayal

Fix checked in on 5/10

20020517 trunk builds Works for MacOS X but Deleting Directory fails on WIn2k...please refer to regression Bug 145346.

20020529 trunk build on Win2K Verified it get removed directory pane.

Please have the fix checked in Branch build. Changing nsbeta1- to nsbeta1, nominate for Buffy.

Removed keyword, nsbeta1. The fix doesn't need to be checked into branch build.