Bug 409608 Tidy up nsAbAddressCollecter and add some more tests. r/sr=Neil, also fixes bug 65635 Shouldn't collect people without email address

mailnews/addrbook/src/nsAbAddressCollecter.cpp

@@ -64,12 +64,6 @@ nsAbAddressCollecter::nsAbAddressCollecter()
 
 nsAbAddressCollecter::~nsAbAddressCollecter()
 {
-  if (m_database) {
-    m_database->Commit(nsAddrDBCommitType::kSessionCommit);
-    m_database->Close(PR_FALSE);
-    m_database = nsnull;
-  }
-
   nsresult rv;
   nsCOMPtr<nsIPrefBranch2> pPrefBranchInt(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));
   if(NS_SUCCEEDED(rv))
@@ -110,27 +104,41 @@ NS_IMETHODIMP nsAbAddressCollecter::CollectAddress(const nsACString &aAddresses,
   if (NS_FAILED(rv))
     return NS_OK;
 
-  char *curName = names;
-  char *curAddress = addresses;
+  char *curNamePtr = names;
+  char *curAddressPtr = addresses;
 
   for (PRUint32 i = 0; i < numAddresses; i++)
   {
+    nsDependentCString curAddress(curAddressPtr);
+    curAddressPtr += curAddress.Length() + 1;
+
     nsCString unquotedName;
-    rv = pHeader->UnquotePhraseOrAddr(curName, PR_FALSE, getter_Copies(unquotedName));
+    rv = pHeader->UnquotePhraseOrAddr(curNamePtr, PR_FALSE,
+                                      getter_Copies(unquotedName));
+    curNamePtr += strlen(curNamePtr) + 1;
     NS_ASSERTION(NS_SUCCEEDED(rv), "failed to unquote name");
     if (NS_FAILED(rv))
       continue;
 
+    // Don't allow collection of addresses with no email address, it makes
+    // no sense. Whilst we should never get here in most normal cases, we
+    // should still be careful.
+    if (curAddress.IsEmpty())
+      continue;
+
+
     nsCOMPtr <nsIAbCard> existingCard;
     nsCOMPtr <nsIAbCard> cardInstance;
     PRBool emailAddressIn2ndEmailColumn = PR_FALSE;
 
-    rv = GetCardFromAttribute(NS_LITERAL_CSTRING(kPriEmailColumn), nsDependentCString(curAddress), getter_AddRefs(existingCard));
+    rv = GetCardFromAttribute(NS_LITERAL_CSTRING(kPriEmailColumn), curAddress,
+                              getter_AddRefs(existingCard));
     // We've not found a card, but is this address actually in the additional
     // email column?
     if (!existingCard)
     {
-      rv = GetCardFromAttribute(NS_LITERAL_CSTRING(k2ndEmailColumn), nsDependentCString(curAddress), getter_AddRefs(existingCard));
+      rv = GetCardFromAttribute(NS_LITERAL_CSTRING(k2ndEmailColumn), curAddress,
+                                getter_AddRefs(existingCard));
       if (existingCard)
         emailAddressIn2ndEmailColumn = PR_TRUE;
     }
@@ -138,39 +146,33 @@ NS_IMETHODIMP nsAbAddressCollecter::CollectAddress(const nsACString &aAddresses,
     if (!existingCard && aCreateCard)
     {
       nsCOMPtr<nsIAbCard> senderCard = do_CreateInstance(NS_ABCARDPROPERTY_CONTRACTID, &rv);
-      if (NS_SUCCEEDED(rv) && senderCard)
+      if (NS_SUCCEEDED(rv) && senderCard && m_directory)
       {
-        PRBool modifiedCard;
-        rv = SetNamesForCard(senderCard, unquotedName, &modifiedCard);
-        NS_ASSERTION(NS_SUCCEEDED(rv), "failed to set names");
+        // Set up the fields for the new card.
+        SetNamesForCard(senderCard, unquotedName);
+        AutoCollectScreenName(senderCard, curAddress);
 
-        rv = AutoCollectScreenName(senderCard, nsCString(curAddress), &modifiedCard);
-        NS_ASSERTION(NS_SUCCEEDED(rv), "failed to set screenname");
-
-        rv = senderCard->SetPrimaryEmail(NS_ConvertUTF8toUTF16(curAddress));
-        NS_ASSERTION(NS_SUCCEEDED(rv), "failed to set email");
-
-        if (aSendFormat != nsIAbPreferMailFormat::unknown)
+        if (NS_SUCCEEDED(senderCard->SetPrimaryEmail(NS_ConvertUTF8toUTF16(curAddress))))
         {
-          rv = senderCard->SetPreferMailFormat(aSendFormat);
-          NS_ASSERTION(NS_SUCCEEDED(rv), "failed to remember preferred mail format");
-        }
+          if (aSendFormat != nsIAbPreferMailFormat::unknown)
+            senderCard->SetPreferMailFormat(aSendFormat);
 
-        rv = AddCardToAddressBook(senderCard);
-        NS_ASSERTION(NS_SUCCEEDED(rv), "failed to add card");
+          nsCOMPtr<nsIAbCard> addedCard;
+          rv = m_directory->AddCard(senderCard, getter_AddRefs(addedCard));
+          NS_ASSERTION(NS_SUCCEEDED(rv), "failed to add card");
+        }
       }
     }
     else if (existingCard && !emailAddressIn2ndEmailColumn) { 
       // address is already in the AB, so update the names
-      PRBool setNames = PR_FALSE;
-      
-      if (!unquotedName.IsEmpty())
-      {
-        rv = SetNamesForCard(existingCard, unquotedName, &setNames);
-        NS_ASSERTION(NS_SUCCEEDED(rv), "failed to set names");
-      }
+      PRBool modifiedCard = PR_FALSE;
+
+      nsString displayName;
+      existingCard->GetDisplayName(displayName);
+      // If we already have a display name, don't set the names on the card.
+      if (displayName.IsEmpty() && !unquotedName.IsEmpty())
+        modifiedCard = SetNamesForCard(existingCard, unquotedName);
 
-      PRBool setPreferMailFormat = PR_FALSE; 
       if (aSendFormat != nsIAbPreferMailFormat::unknown)
       {
         PRUint32 currentFormat;
@@ -178,20 +180,14 @@ NS_IMETHODIMP nsAbAddressCollecter::CollectAddress(const nsACString &aAddresses,
         NS_ASSERTION(NS_SUCCEEDED(rv), "failed to get preferred mail format");
 
         // we only want to update the AB if the current format is unknown
-        if (currentFormat == nsIAbPreferMailFormat::unknown) 
-        {
-          rv = existingCard->SetPreferMailFormat(aSendFormat);
-          NS_ASSERTION(NS_SUCCEEDED(rv), "failed to remember preferred mail format");
-          setPreferMailFormat = PR_TRUE;
-        }
+        if (currentFormat == nsIAbPreferMailFormat::unknown &&
+            NS_SUCCEEDED(existingCard->SetPreferMailFormat(aSendFormat)))
+          modifiedCard = PR_TRUE;
       }
 
-      if ((setNames || setPreferMailFormat) && m_directory)
+      if (modifiedCard && m_directory)
         m_directory->ModifyCard(existingCard);
     }
-
-    curName += strlen(curName) + 1;
-    curAddress += strlen(curAddress) + 1;
   } 
 
   PR_FREEIF(addresses);
@@ -199,130 +195,115 @@ NS_IMETHODIMP nsAbAddressCollecter::CollectAddress(const nsACString &aAddresses,
   return NS_OK;
 }
 
-nsresult nsAbAddressCollecter::AutoCollectScreenName(nsIAbCard *aCard, const nsACString &aEmail, PRBool *aModifiedCard)
+// Works out the screen name to put on the card for some well-known addresses
+void
+nsAbAddressCollecter::AutoCollectScreenName(nsIAbCard *aCard,
+                                            const nsCString &aEmail)
 {
-  NS_ENSURE_ARG_POINTER(aCard);
-  NS_ENSURE_ARG_POINTER(aModifiedCard);
+  if (!aCard)
+    return;
 
-  *aModifiedCard = PR_FALSE;
+  PRInt32 atPos = aEmail.FindChar('@');
+  if (atPos == -1)
+    return;
 
-  nsCString email(aEmail);
-  int atPos = aEmail.FindChar('@');
-  if (atPos == -1) 
-    return NS_OK;
-    
-  nsCString domain(Substring(email, atPos + 1));
-  if (domain.IsEmpty())
-    return NS_OK; 
+  const nsACString &domain(Substring(aEmail, atPos + 1));
 
   // username in 
   // username@aol.com (America Online)
   // username@cs.com (Compuserve)
   // username@netscape.net (Netscape webmail)
   // are all AIM screennames.  autocollect that info.
-  if (domain.Equals("aol.com") || 
-      domain.Equals("cs.com") || domain.Equals("netscape.net")) {
-    nsCString userName(Substring(email, 0, atPos));
-  
-    nsresult rv = aCard->SetAimScreenName(NS_ConvertUTF8toUTF16(userName));
-    NS_ENSURE_SUCCESS(rv,rv);
-
-    *aModifiedCard = PR_TRUE;
-    return rv;
-  }
-
-  return NS_OK;
+  if (!domain.IsEmpty() &&
+      (domain.Equals("aol.com") || domain.Equals("cs.com") ||
+       domain.Equals("netscape.net")))
+    aCard->SetAimScreenName(NS_ConvertUTF8toUTF16(Substring(aEmail, 0, atPos)));
 }
 
-
-nsresult 
-nsAbAddressCollecter::SetNamesForCard(nsIAbCard *aSenderCard, const nsACString &aFullName, PRBool *aModifiedCard)
+// Returns true if the card was modified successfully.
+PRBool
+nsAbAddressCollecter::SetNamesForCard(nsIAbCard *aSenderCard,
+                                      const nsCString &aFullName)
 {
   nsCString firstName;
   nsCString lastName;
-  *aModifiedCard = PR_FALSE;
+  PRBool modifiedCard = PR_FALSE;
 
-  nsString displayName;
-  nsresult rv = aSenderCard->GetDisplayName(displayName);
-  NS_ENSURE_SUCCESS(rv,rv);
+  if (NS_SUCCEEDED(aSenderCard->SetDisplayName(NS_ConvertUTF8toUTF16(aFullName))))
+    modifiedCard = PR_TRUE;
 
-  // we already have a display name, so don't do anything
-  if (!displayName.IsEmpty())
-    return NS_OK;
+  // Now split up the full name.
+  SplitFullName(aFullName, firstName, lastName);
 
-  aSenderCard->SetDisplayName(NS_ConvertUTF8toUTF16(aFullName));
-  *aModifiedCard = PR_TRUE;
+  if (!firstName.IsEmpty() &&
+      NS_SUCCEEDED(aSenderCard->SetFirstName(NS_ConvertUTF8toUTF16(firstName))))
+    modifiedCard = PR_TRUE;
 
-  rv = SplitFullName(aFullName, firstName, lastName);
-  if (NS_SUCCEEDED(rv))
-  {
-    aSenderCard->SetFirstName(NS_ConvertUTF8toUTF16(firstName));
-    
-    if (!lastName.IsEmpty())
-      aSenderCard->SetLastName(NS_ConvertUTF8toUTF16(lastName));
-  }
-  return rv;
+  if (!lastName.IsEmpty() &&
+      NS_SUCCEEDED(aSenderCard->SetLastName(NS_ConvertUTF8toUTF16(lastName))))
+    modifiedCard = PR_TRUE;
+
+  return modifiedCard;
 }
 
-nsresult nsAbAddressCollecter::SplitFullName(const nsACString &aFullName, nsACString &aFirstName, nsACString &aLastName)
+// Splits the first and last name based on the space between them.
+void
+nsAbAddressCollecter::SplitFullName(const nsCString &aFullName, nsCString &aFirstName,
+                                    nsCString &aLastName)
 {
-  int index = nsCString(aFullName).RFindChar(' ');
-  if (index != -1) 
+  int index = aFullName.RFindChar(' ');
+  if (index != -1)
   {
     aLastName = Substring(aFullName, index + 1);
     aFirstName = Substring(aFullName, 0, index);
   }
-  return NS_OK;
 }
 
+// Observes the collected address book pref in case it changes.
 NS_IMETHODIMP nsAbAddressCollecter::Observe(nsISupports *aSubject, const char *aTopic, const PRUnichar *aData)
 {
   nsCOMPtr<nsIPrefBranch2> pPrefBranchInt = do_QueryInterface(aSubject);
-  NS_ASSERTION(pPrefBranchInt, "failed to get prefs");
+  if (!pPrefBranchInt) {
+    NS_ASSERTION(pPrefBranchInt, "failed to get prefs");
+    return NS_OK;
+  }
 
   nsresult rv;
   nsCString prefVal;
-  pPrefBranchInt->GetCharPref(PREF_MAIL_COLLECT_ADDRESSBOOK, getter_Copies(prefVal));
-  rv = SetAbURI(prefVal.IsEmpty() ? nsDependentCString(kPersonalAddressbookUri) : prefVal);
+  pPrefBranchInt->GetCharPref(PREF_MAIL_COLLECT_ADDRESSBOOK,
+                              getter_Copies(prefVal));
+  rv = SetAbURI(prefVal);
   NS_ASSERTION(NS_SUCCEEDED(rv),"failed to change collected ab");
   return NS_OK;
 }
 
+// Initialises the collecter with the required items.
 nsresult nsAbAddressCollecter::Init(void)
 {
   nsresult rv;
   nsCOMPtr<nsIPrefBranch2> pPrefBranchInt(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));
-  NS_ENSURE_SUCCESS(rv,rv);
+  NS_ENSURE_SUCCESS(rv, rv);
 
   rv = pPrefBranchInt->AddObserver(PREF_MAIL_COLLECT_ADDRESSBOOK, this, PR_FALSE);
+  NS_ENSURE_SUCCESS(rv, rv);
 
   nsCString prefVal;
-  pPrefBranchInt->GetCharPref(PREF_MAIL_COLLECT_ADDRESSBOOK, getter_Copies(prefVal));
-  return SetAbURI(prefVal.IsEmpty() ? nsDependentCString(kPersonalAddressbookUri) : prefVal);
+  pPrefBranchInt->GetCharPref(PREF_MAIL_COLLECT_ADDRESSBOOK,
+                              getter_Copies(prefVal));
+  return SetAbURI(prefVal);
 }
 
-nsresult nsAbAddressCollecter::AddCardToAddressBook(nsIAbCard *card)
+// Performs the necessary changes to set up the collecter for the specified
+// collected address book.
+nsresult nsAbAddressCollecter::SetAbURI(nsCString &aURI)
 {
-  NS_ENSURE_ARG_POINTER(card);
+  if (aURI.IsEmpty())
+    aURI.AssignLiteral(kPersonalAddressbookUri);
 
-  nsCOMPtr <nsIAbCard> addedCard;
-  if (m_directory)
-    return m_directory->AddCard(card, getter_AddRefs(addedCard));
-
-  return NS_ERROR_FAILURE;
-}
-
-nsresult nsAbAddressCollecter::SetAbURI(const nsACString &aURI)
-{
   if (aURI == m_abURI)
     return NS_OK;
 
-  if (m_database) {
-    m_database->Commit(nsAddrDBCommitType::kSessionCommit);
-    m_database->Close(PR_FALSE);
-    m_database = nsnull;
-  }
-  
+  m_database = nsnull;
   m_directory = nsnull;
   m_abURI = aURI;
 

mailnews/addrbook/src/nsAbAddressCollecter.h

@@ -60,11 +60,11 @@ public:
   nsresult Init();
 
 private:
-  nsresult AddCardToAddressBook(nsIAbCard *card);
-  nsresult AutoCollectScreenName(nsIAbCard *aCard, const nsACString &aEmail, PRBool *aModifiedCard);
-  nsresult SetNamesForCard(nsIAbCard *aSenderCard, const nsACString &aFullName, PRBool *aModifiedCard);
-  nsresult SplitFullName (const nsACString &aFullName, nsACString &aFirstName, nsACString &aLastName);
-  nsresult SetAbURI(const nsACString &aURI);
+  void AutoCollectScreenName(nsIAbCard *aCard, const nsCString &aEmail);
+  PRBool SetNamesForCard(nsIAbCard *aSenderCard, const nsCString &aFullName);
+  void SplitFullName(const nsCString &aFullName, nsCString &aFirstName,
+                     nsCString &aLastName);
+  nsresult SetAbURI(nsCString &aURI);
   nsCOMPtr <nsIAddrDatabase> m_database;
   nsCOMPtr <nsIAbDirectory> m_directory;
   nsCString m_abURI;

mailnews/addrbook/test/unit/test_collection.js

@@ -2,45 +2,249 @@
 /*
  * Test suite for the Address Collecter Service.
  *
- * XXX todo:
- * - check addition of names with no email addresses (shouldn't allow)
- * - check addition of names and email addresses
- * - check addition of screenname
- * - check updates to existing cards
+ * XXX Todo: May still need to add test for
+ * nsIAbAddressCollecter::getCardFromAttribute if its kept.
  */
 
 const nsIAbPMF = Components.interfaces.nsIAbPreferMailFormat;
 
 var testnum = 0;
 
-// Emails and prefer mail format types to check for correct collection.
+// Source fields (emailHeader/mailFormat) and expected results for use for
+// testing the addition of new addresses to the database.
 //
-// Note: these emails should be different to allow collecting an address to
-// add a different card each time.
-var addEmailChecks = [ ["test0@invalid.com", nsIAbPMF.unknown],
-                       ["test1@invalid.com", nsIAbPMF.plaintext],
-                       ["test2@invalid.com", nsIAbPMF.html],
-                       // UTF-8 based addresses (bug 407564)
-                       ["test0@\u00D0.com", nsIAbPMF.unknown],
-                       ["test0\u00D0@invalid.com", nsIAbPMF.unknown],
-                     ];
+// Note: these email addressess should be different to allow collecting an
+// address to add a different card each time.
+var addEmailChecks =
+  // First 3 items aimed at basic collection and mail format.
+  [ { emailHeader: "test0@invalid.com",
+      primaryEmail: "test0@invalid.com",
+      mailFormat: nsIAbPMF.unknown,
+      displayName: "",
+      firstName: "",
+      lastName: "",
+      screenName: "" },
+    { emailHeader: "test1@invalid.com",
+      primaryEmail: "test1@invalid.com",
+      mailFormat: nsIAbPMF.plaintext,
+      displayName: "",
+      firstName: "",
+      lastName: "",
+      screenName: "" },
+    { emailHeader: "test2@invalid.com",
+      primaryEmail: "test2@invalid.com",
+      mailFormat: nsIAbPMF.html,
+      displayName: "",
+      firstName: "",
+      lastName: "",
+      screenName: "" },
+    // UTF-8 based addresses (bug 407564)
+    { emailHeader: "test0@\u00D0.com",
+      primaryEmail: "test0@\u00D0.com",
+      mailFormat: nsIAbPMF.unknown,
+      displayName: "",
+      firstName: "",
+      lastName: "",
+      screenName: "" },
+    { emailHeader: "test0\u00D0@invalid.com",
+      primaryEmail: "test0\u00D0@invalid.com",
+      mailFormat: nsIAbPMF.unknown,
+      displayName: "",
+      firstName: "",
+      lastName: "",
+      screenName: "" },
+    // Screen names
+    { emailHeader: "invalid\u00D00@aol.com",
+      primaryEmail: "invalid\u00D00@aol.com",
+      mailFormat: nsIAbPMF.unknown,
+      displayName: "",
+      firstName: "",
+      lastName: "",
+      screenName: "invalid\u00D00" },
+    { emailHeader: "invalid1\u00D00@cs.com",
+      primaryEmail: "invalid1\u00D00@cs.com",
+      mailFormat: nsIAbPMF.unknown,
+      displayName: "",
+      firstName: "",
+      lastName: "",
+      screenName: "invalid1\u00D00" },
+    { emailHeader: "invalid2\u00D00@netscape.net",
+      primaryEmail: "invalid2\u00D00@netscape.net",
+      mailFormat: nsIAbPMF.unknown,
+      displayName: "",
+      firstName: "",
+      lastName: "",
+      screenName: "invalid2\u00D00" },
+    // Collection of names
+    { emailHeader: "Test User <test3@invalid.com>",
+      primaryEmail: "test3@invalid.com",
+      mailFormat: nsIAbPMF.unknown,
+      displayName: "Test User",
+      firstName: "Test",
+      lastName: "User",
+      screenName: "" },
+    { emailHeader: "Test <test4@invalid.com>",
+      primaryEmail: "test4@invalid.com",
+      mailFormat: nsIAbPMF.unknown,
+      displayName: "Test",
+      firstName: "",
+      lastName: "",
+      screenName: "" },
+    // Collection of names with UTF-8 specific items
+    { emailHeader: "Test\u00D0 User <test5@invalid.com>",
+      primaryEmail: "test5@invalid.com",
+      mailFormat: nsIAbPMF.unknown,
+      displayName: "Test\u00D0 User",
+      firstName: "Test\u00D0",
+      lastName: "User",
+      screenName: "" },
+    { emailHeader: "Test\u00D0 <test6@invalid.com>",
+      primaryEmail: "test6@invalid.com",
+      mailFormat: nsIAbPMF.unknown,
+      displayName: "Test\u00D0",
+      firstName: "",
+      lastName: "",
+      screenName: "" },
+    ];
+
+// Source fields (emailHeader/mailFormat) and expected results for use for
+// testing the modification of cards in the database.
+//
+// Note: these sets re-use some of the ones for ease of definition.
+var modifyEmailChecks =
+  // No display name/other details. Add details and modify mail format.
+  [ { emailHeader: "Modify User\u00D0 <test0@\u00D0.com>",
+      primaryEmail: "test0@\u00D0.com",
+      mailFormat: nsIAbPMF.html,
+      displayName: "Modify User\u00D0",
+      firstName: "Modify",
+      lastName: "User\u00D0",
+      screenName: "" },
+    { emailHeader: "Modify <test0\u00D0@invalid.com>",
+      primaryEmail: "test0\u00D0@invalid.com",
+      mailFormat: nsIAbPMF.plaintext,
+      displayName: "Modify",
+      firstName: "",
+      lastName: "",
+      screenName: "" },
+    // No modification of existing cards with display names
+    { emailHeader: "Modify2 User\u00D02 <test0@\u00D0.com>",
+      primaryEmail: "test0@\u00D0.com",
+      mailFormat: nsIAbPMF.html,
+      displayName: "Modify User\u00D0",
+      firstName: "Modify",
+      lastName: "User\u00D0",
+      screenName: "" },
+    { emailHeader: "Modify3 <test0\u00D0@invalid.com>",
+      primaryEmail: "test0\u00D0@invalid.com",
+      mailFormat: nsIAbPMF.plaintext,
+      displayName: "Modify",
+      firstName: "",
+      lastName: "",
+      screenName: "" },
+    // Check no modification of cards for mail format where format is not
+    // "unknown".
+    { emailHeader: "Modify User\u00D0 <test0@\u00D0.com>",
+      primaryEmail: "test0@\u00D0.com",
+      mailFormat: nsIAbPMF.plaintext,
+      mailFormatOut: nsIAbPMF.html,
+      displayName: "Modify User\u00D0",
+      firstName: "Modify",
+      lastName: "User\u00D0",
+      screenName: "" },
+    { emailHeader: "Modify <test0\u00D0@invalid.com>",
+      primaryEmail: "test0\u00D0@invalid.com",
+      mailFormat: nsIAbPMF.html,
+      mailFormatOut: nsIAbPMF.plaintext,
+      displayName: "Modify",
+      firstName: "",
+      lastName: "",
+      screenName: "" },
+    // No modification of cards with email in second email address.
+    { emailHeader: "Modify Secondary <usersec\u00D0@invalid.com>",
+      primaryEmail: "userprim\u00D0@invalid.com",
+      secondEmail: "usersec\u00D0@invalid.com",
+      mailFormat: nsIAbPMF.unknown,
+      displayName: "",
+      firstName: "",
+      lastName: "",
+      screenName: "" },
+    { emailHeader: "Modify <usersec\u00D0@invalid.com>",
+      primaryEmail: "userprim\u00D0@invalid.com",
+      secondEmail: "usersec\u00D0@invalid.com",
+      mailFormat: nsIAbPMF.html,
+      mailFormatOut: nsIAbPMF.unknown,
+      displayName: "",
+      firstName: "",
+      lastName: "",
+      screenName: "" },
+   ];
 
 var collectChecker = {
   addressCollect: null,
   AB: null,
+  part: 0,
 
   checkAddress : function (aDetails) {
     try {
-      this.addressCollect.collectAddress(aDetails[0], true, aDetails[1]);
+      this.addressCollect.collectAddress(aDetails.emailHeader, true,
+                                         aDetails.mailFormat);
 
-      var card = this.AB.cardForEmailAddress(aDetails[0]);
+      this.checkCardResult(aDetails, false);
+    }
+    catch (e) {
+      throw "FAILED in checkAddress emailHeader: " + aDetails.emailHeader +
+      " part: " + this.part + " : " + e;
+    }
+    ++this.part;
+  },
+
+  checkAll : function (aDetailsArray) {
+    try {
+      // Formulate the string to add.
+      var emailHeader = "";
+      var i;
+
+      for (i = 0; i < aDetailsArray.length - 1; ++i)
+        emailHeader += aDetailsArray[i].emailHeader + ", ";
+
+      emailHeader += aDetailsArray[aDetailsArray.length - 1].emailHeader;
+
+      // Now add it. In this case we just set the Mail format Type to unknown.
+      this.addressCollect.collectAddress(emailHeader, true,
+                                         nsIAbPMF.unknown);
+
+      for (i = 0; i < aDetailsArray.length; ++i)
+        this.checkCardResult(aDetailsArray[i], true);
+    }
+    catch (e) {
+      throw "FAILED in checkAll item: " + i + " : " + e;
+    }
+  },
+
+  checkCardResult : function (aDetails, overrideMailFormat) {
+    try {
+      var card = this.AB.cardForEmailAddress(aDetails.primaryEmail);
 
       do_check_true(card != null);
 
-      do_check_eq(card.preferMailFormat, aDetails[1]);
+      if ("secondEmail" in aDetails)
+        do_check_eq(card.secondEmail, aDetails.secondEmail);
+
+      if (overrideMailFormat)
+        do_check_eq(card.preferMailFormat, nsIAbPMF.unknown);
+      else if ("mailFormatOut" in aDetails)
+        do_check_eq(card.preferMailFormat, aDetails.mailFormatOut);
+      else
+        do_check_eq(card.preferMailFormat, aDetails.mailFormat);
+
+      do_check_eq(card.displayName, aDetails.displayName);
+      do_check_eq(card.firstName, aDetails.firstName);
+      do_check_eq(card.lastName, aDetails.lastName);
+      do_check_eq(card.aimScreenName, aDetails.screenName);
     }
     catch (e) {
-      throw "FAILED in checkAddress(" + aDetails[0] + ", " + aDetails[1] + ") : " + e;
+      throw "FAILED in checkCardResult emailHeader: " + aDetails.emailHeader + " : " + e;
     }
   }
 };
@@ -81,24 +285,71 @@ function run_test()
 
     do_check_true(collectChecker.addressCollect != null);
 
-    ++testnum; // Test 2 - Email doesn't exist, but don't add it.
+    ++testnum; // Test 2 - Addition of header without email address.
+
+    collectChecker.addressCollect.collectAddress("MyTest <>", true,
+                                                 nsIAbPMF.unknown);
+
+    var CAB = collectChecker.AB.QueryInterface(Components.interfaces.nsIAbDirectory);
+
+    // Address book should have no cards present.
+    do_check_false(CAB.childCards.hasMoreElements());
+
+    ++testnum; // Test 3 - Email doesn't exist, but don't add it.
 
     // As we've just set everything up, we know we haven't got anything in the
     // AB, so just try and collect without adding.
-    collectChecker.addressCollect.collectAddress(addEmailChecks[0][0],
+    collectChecker.addressCollect.collectAddress(addEmailChecks[0].emailHeader,
                                                  false,
-                                                 addEmailChecks[0][1]);
+                                                 addEmailChecks[0].mailFormat);
 
-    var card = collectChecker.AB.cardForEmailAddress(addEmailChecks[0][0]);
+    var card = collectChecker.AB.cardForEmailAddress(addEmailChecks[0].emailHeader);
 
     do_check_true(card == null);
 
-    ++testnum; // Test 3 - Try and collect various emails and formats.
+    ++testnum; // Test 4 - Try and collect various emails and formats.
+
+    collectChecker.part = 0;
 
     addEmailChecks.forEach(collectChecker.checkAddress, collectChecker);
 
+    ++testnum; // Test 5 - Do all emails at the same time.
+
+    // First delete all existing cards
+    var childCards = CAB.childCards;
+    var cardsToDelete = Components.classes["@mozilla.org/supports-array;1"]
+                                  .createInstance(Components.interfaces.nsISupportsArray);
+    while (childCards.hasMoreElements()) {
+      cardsToDelete.AppendElement(childCards.getNext());
+    }
+
+    CAB.deleteCards(cardsToDelete);
+
+    // Address book should have no cards present.
+    do_check_false(CAB.childCards.hasMoreElements());
+
+    do_check_eq(collectChecker.AB.cardForEmailAddress(addEmailChecks[0].emailHeader), null);
+
+    // Now do all emails at the same time.
+    collectChecker.checkAll(addEmailChecks);
+
+    ++testnum; // Test 6 - Try and modify various emails and formats.
+
+    // Add a basic card with just primary and second email to allow testing
+    // of the case where we don't modify when second email is matching.
+    card = Components.classes["@mozilla.org/addressbook/cardproperty;1"]
+                     .createInstance(Components.interfaces.nsIAbCard);
+
+    card.primaryEmail = "userprim\u00D0@invalid.com";
+    card.secondEmail = "usersec\u00D0@invalid.com";
+
+    CAB.addCard(card);
+
+    collectChecker.part = 0;
+
+    modifyEmailChecks.forEach(collectChecker.checkAddress, collectChecker);
+
   } catch (e) {
     throw "FAILED in address collector tests in test #" + testnum + ": " + e;
   }
 };
-