Spun out from Bug 357321. Currently HasCardForEmailAddress/CardForEmailAddress are methods on nsIAbMDBCard We should make these more generic and add them to nsIAbCard. For the trunk, I think we can also remove HasCardForEmailAddress and just use CardForEmailAddress. Callers can test for the existence of the returned card. I didn't do this in my patch for Bug 357321 because that patch is going into 1.8.1 where we can't remove public interface methods easily.

(In reply to comment #0) > Currently > HasCardForEmailAddress/CardForEmailAddress are methods on nsIAbMDBCard Actually, you meant nsIAbMDBDirectory, and should be on nsIAbDirectory.

I am splitting the work into multiple parts, the first being the removal of HasCardForEmailAddress.

Comment on attachment 271555 [details] [diff] [review] Part 1 - Remove HasCardForEmailAddress - patch v0.2 r=me via code inspection. The only thing I'd be tempted to say is with this section: - PRBool cardExists = PR_FALSE; + nsCOMPtr<nsIAbCard> cardForAddress; // don't want to abort the rest of the scoring. if (!authorEmailAddress.IsEmpty()) { - for (PRInt32 index = 0; index < whiteListDirArray.Count() && !cardExists; index++) - rv = whiteListDirArray[index]->HasCardForEmailAddress(authorEmailAddress.get(), &cardExists); + for (PRInt32 index = 0; index < whiteListDirArray.Count() && !cardForAddress; index++) + rv = whiteListDirArray[index]->CardForEmailAddress(authorEmailAddress.get(), getter_AddRefs(cardForAddress)); if (NS_FAILED(rv)) - cardExists = PR_FALSE; + cardForAddress = nsnull; } - if (cardExists) + if (cardForAddress) Would it be faster to use nsIAbCard* cardForAddress; ... ->CardForEmailAddress(..., &cardForAddress) etc, thus avoiding the overhead of nsCOMPtr and getter_AddRef functions in this case (we'd obviously need a NS_RELEASE in the cardForAddress existing case). With the patch should be faster than it is at the moment anyway, so I'm not too worried. It is just a thought that came to mind.

Is this patch correct as far as NS_RELEASE, etc?

Comment on attachment 271752 [details] [diff] [review] Part 1 - Remove HasCardForEmailAddress with NS_RELEASE - patch v0.2a r=me if you change the nsMsgSearchTerm.cpp instance to NS_IF_RELEASE as we're not guaranteed to have a valid pointer/card in that case (the nsMsgDBFolder.cpp is ok though).

Comment on attachment 271752 [details] [diff] [review] Part 1 - Remove HasCardForEmailAddress with NS_RELEASE - patch v0.2a actually set the flag this time...

Changes since v0.2a: * NS_RELEASE -> NS_IF_RELEASE in nsMsgSearchTerm.cpp Carrying forward r= and requesting sr=

Comment on attachment 272068 [details] [diff] [review] Part 1 - Remove HasCardForEmailAddress with NS_IF_RELEASE - patch v0.2b Checking in (trunk) mail/base/content/mailCommands.js; new revision: 1.34; previous revision: 1.33 mail/base/content/msgHdrViewOverlay.js; new revision: 1.92; previous revision: 1.91 mailnews/addrbook/public/nsIAbMDBDirectory.idl; new revision: 1.10; previous revision: 1.9 mailnews/addrbook/src/nsAbMDBDirProperty.cpp; new revision: 1.16; previous revision: 1.15 mailnews/addrbook/src/nsAbMDBDirectory.cpp; new revision: 1.64; previous revision: 1.63 mailnews/addrbook/src/nsAbMDBDirectory.h; new revision: 1.25; previous revision: 1.24 mailnews/base/resources/content/mailCommands.js; new revision: 1.105; previous revision: 1.104 mailnews/base/search/src/nsMsgSearchTerm.cpp; new revision: 1.144; previous revision: 1.143 mailnews/base/util/nsMsgDBFolder.cpp; new revision: 1.319; previous revision: 1.318 done

Hey people... the change on nsMsgDBFolder.cpp makes on of my accounts always segpipe just after getting the last message. Some accounts works fine, but that one doesn't.

Downloading messages from a POP3 server crashes in nsMsgDBFolder::CallFilterPlugins() because cardForAddress is uninitialized.

Comment on attachment 272323 [details] [diff] [review] fix crash This looks the right fix so I'm jumping in and giving r=me, requesting sr.

That patch works fine here. It crashed before because the code tried to release cardForAddress while it was a 0x01.

Could someone please check this in?

(In reply to comment #13) > Could someone please check this in? > Checked in: /cvsroot/mozilla/mailnews/base/util/nsMsgDBFolder.cpp,v <-- nsMsgDBFolder.cpp new revision: 1.320; previous revision: 1.319 Thanks for the patch.

Ian, is there any ongoing work for your second part here? It's a feature we need in bug 243631 to support a search in LDAP/OSX/Outlook address books.

These two bugs need to be linked together, but I think the bigger issue here is that we need to add implementations for non-MDB address books. Hence, bug 449618 (bumping it up) is "blocking" this bug (adding the impls).