Bug 1522453 - Prevent mailing list member iteration from failing if a member has no email address. r=mkmelin a=jorgk
authorGeoff Lankow <geoff@darktrojan.net>
Fri, 25 Jan 2019 15:51:47 +1300
changeset 34345 f3b778e7c52ffb688f3e3a574fa0ee9347ba2f4a
parent 34344 85ae1ae9eaa022dd07414a4749e7eaa47f3101bd
child 34346 89ebd1b371c673db7506a7ab80df43549629f6c2
push id389
push userclokep@gmail.com
push dateMon, 18 Mar 2019 19:01:53 +0000
reviewersmkmelin, jorgk
bugs1522453
Bug 1522453 - Prevent mailing list member iteration from failing if a member has no email address. r=mkmelin a=jorgk
mail/components/extensions/parent/ext-addressBook.js
mail/components/extensions/test/xpcshell/test_ext_addressBook.js
mailnews/addrbook/src/nsAddrDatabase.cpp
mailnews/addrbook/test/unit/test_bug1522453.js
mailnews/addrbook/test/unit/xpcshell.ini
--- a/mail/components/extensions/parent/ext-addressBook.js
+++ b/mail/components/extensions/parent/ext-addressBook.js
@@ -43,23 +43,22 @@ var addressBookCache = new class extends
   _makeDirectoryNode(directory, parent = null) {
     directory.QueryInterface(Ci.nsIAbDirectory);
     let node = {
       id: directory.UID,
       type: directory.isMailList ? "mailingList" : "addressBook",
       item: directory,
       get contacts() {
         delete this.contacts;
-        if (directory.isMailList) {
-          this.contacts = [...directory.addressLists.enumerate()];
-        } else {
-          this.contacts = [...directory.childCards]
-            .filter(c => !c.isMailList);
+        this.contacts = [];
+        for (let card of directory.childCards) {
+          if (!card.isMailList) {
+            this.contacts.push(addressBookCache._makeContactNode(card, directory));
+          }
         }
-        this.contacts = this.contacts.map(c => addressBookCache._makeContactNode(c, directory));
         return this.contacts;
       },
       get mailingLists() {
         delete this.mailingLists;
         if (directory.isMailList) {
           return undefined;
         }
         this.mailingLists = [];
@@ -455,18 +454,17 @@ this.addressBook = class extends Extensi
         list(parentId) {
           let parentNode = addressBookCache.findAddressBookById(parentId);
           return addressBookCache.convert(parentNode.mailingLists, false);
         },
         get(id) {
           return addressBookCache.convert(addressBookCache.findMailingListById(id), false);
         },
         create(parentId, { name, nickName, description }) {
-          let mailList = Cc["@mozilla.org/addressbook/directoryproperty;1"].createInstance();
-          mailList.QueryInterface(Ci.nsIAbDirectory);
+          let mailList = Cc["@mozilla.org/addressbook/directoryproperty;1"].createInstance(Ci.nsIAbDirectory);
           mailList.isMailList = true;
           mailList.dirName = name;
           mailList.listNickName = (nickName === null) ? "" : nickName;
           mailList.description = (description === null) ? "" : description;
 
           let parentNode = addressBookCache.findAddressBookById(parentId);
           let newMailList = parentNode.item.addMailList(mailList);
           return newMailList.UID;
--- a/mail/components/extensions/test/xpcshell/test_ext_addressBook.js
+++ b/mail/components/extensions/test/xpcshell/test_ext_addressBook.js
@@ -446,27 +446,28 @@ add_task(async function test_addressBook
         let book = MailServices.ab.getDirectoryFromId(args[0]);
         MailServices.ab.deleteAddressBook(book.URI);
         extension.sendMessage();
         return;
       }
 
       case "createContact": {
         let contact = Cc["@mozilla.org/addressbook/cardproperty;1"].createInstance(Ci.nsIAbCard);
-        contact.setProperty("FirstName", "external");
-        contact.setProperty("LastName", "add");
+        contact.firstName = "external";
+        contact.lastName = "add";
+        contact.primaryEmail = "test@invalid";
         let newContact = parent.addCard(contact);
         extension.sendMessage(parent.UID, newContact.UID);
         return;
       }
       case "updateContact": {
         let contact = findContact(args[0]);
         if (contact) {
-          contact.setProperty("FirstName", "external");
-          contact.setProperty("LastName", "edit");
+          contact.firstName = "external";
+          contact.lastName = "edit";
           parent.modifyCard(contact);
           extension.sendMessage();
           return;
         }
         break;
       }
       case "deleteContact": {
         let contact = findContact(args[0]);
@@ -476,18 +477,17 @@ add_task(async function test_addressBook
           parent.deleteCards(cardArray);
           extension.sendMessage();
           return;
         }
         break;
       }
 
       case "createMailingList": {
-        let list = Cc["@mozilla.org/addressbook/directoryproperty;1"].createInstance();
-        list.QueryInterface(Ci.nsIAbDirectory);
+        let list = Cc["@mozilla.org/addressbook/directoryproperty;1"].createInstance(Ci.nsIAbDirectory);
         list.isMailList = true;
         list.dirName = "external add";
 
         let newList = parent.addMailList(list);
         extension.sendMessage(parent.UID, newList.UID);
         return;
       }
       case "updateMailingList": {
--- a/mailnews/addrbook/src/nsAddrDatabase.cpp
+++ b/mailnews/addrbook/src/nsAddrDatabase.cpp
@@ -2556,19 +2556,18 @@ nsListAddressEnumerator::HasMoreElements
   // correctly. Therefore, whilst processing lists ensure that we don't return
   // false if the only thing stopping us is a blank row, just skip it and try
   // the next one.
   while (mAddressPos < mAddressTotal)
   {
     nsCOMPtr<nsIMdbRow> currentRow;
     nsresult rv = mDb->GetAddressRowByPos(mListRow, mAddressPos + 1,
                                           getter_AddRefs(currentRow));
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    if (currentRow)
+
+    if (NS_SUCCEEDED(rv) && currentRow)
     {
       *aResult = true;
       break;
     }
 
     ++mAddressPos;
   }
 
@@ -2582,29 +2581,29 @@ nsListAddressEnumerator::GetNext(nsISupp
 
   *aResult = nullptr;
 
   if (!mDbTable || !mDb->GetEnv())
   {
       return NS_ERROR_NULL_POINTER;
   }
 
-  if (++mAddressPos <= mAddressTotal)
+  while (++mAddressPos <= mAddressTotal)
   {
     nsCOMPtr<nsIMdbRow> currentRow;
     nsresult rv = mDb->GetAddressRowByPos(mListRow, mAddressPos,
                                           getter_AddRefs(currentRow));
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    nsCOMPtr<nsIAbCard> resultCard;
-    rv = mDb->CreateABCard(currentRow, mListRowID,
-                           getter_AddRefs(resultCard));
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    return CallQueryInterface(resultCard, aResult);
+    if (NS_SUCCEEDED(rv)) {
+      nsCOMPtr<nsIAbCard> resultCard;
+      rv = mDb->CreateABCard(currentRow, mListRowID,
+                             getter_AddRefs(resultCard));
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      return CallQueryInterface(resultCard, aResult);
+    }
   }
 
   return NS_ERROR_FAILURE;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 
 NS_IMETHODIMP nsAddrDatabase::EnumerateCards(nsIAbDirectory *directory, nsISimpleEnumerator **result)
new file mode 100644
--- /dev/null
+++ b/mailnews/addrbook/test/unit/test_bug1522453.js
@@ -0,0 +1,51 @@
+function run_test() {
+  do_get_profile();
+  MailServices.ab.directories;
+  let book = MailServices.ab.getDirectory(kPABData.URI);
+
+  let list = Cc["@mozilla.org/addressbook/directoryproperty;1"].createInstance(Ci.nsIAbDirectory);
+  list.isMailList = true;
+  list.dirName = "list";
+  list = book.addMailList(list);
+
+  let contact1 = Cc["@mozilla.org/addressbook/cardproperty;1"].createInstance(Ci.nsIAbCard);
+  contact1.firstName = "contact";
+  contact1.lastName = "1";
+  contact1.primaryEmail = "contact1@invalid";
+  contact1 = book.addCard(contact1);
+  list.addCard(contact1);
+
+  let contact2 = Cc["@mozilla.org/addressbook/cardproperty;1"].createInstance(Ci.nsIAbCard);
+  contact2.firstName = "contact";
+  contact2.lastName = "2";
+  // No email address!
+  contact2 = book.addCard(contact2);
+  list.addCard(contact2);
+
+  let contact3 = Cc["@mozilla.org/addressbook/cardproperty;1"].createInstance(Ci.nsIAbCard);
+  contact3.firstName = "contact";
+  contact3.lastName = "3";
+  contact3.primaryEmail = "contact3@invalid";
+  contact3 = book.addCard(contact3);
+  list.addCard(contact3);
+
+  // book.childCards should contain the list and all three contacts.
+  let bookCards = book.childCards;
+  ok(bookCards.hasMoreElements());
+  equal(list.UID, bookCards.getNext().QueryInterface(Ci.nsIAbCard).UID);
+  ok(bookCards.hasMoreElements());
+  equal(contact1.UID, bookCards.getNext().QueryInterface(Ci.nsIAbCard).UID);
+  ok(bookCards.hasMoreElements());
+  equal(contact2.UID, bookCards.getNext().QueryInterface(Ci.nsIAbCard).UID);
+  ok(bookCards.hasMoreElements());
+  equal(contact3.UID, bookCards.getNext().QueryInterface(Ci.nsIAbCard).UID);
+  ok(!bookCards.hasMoreElements());
+
+  // list.childCards should contain contacts 1 and 3, and crucially, not die at 2.
+  let listCards = list.childCards;
+  ok(listCards.hasMoreElements());
+  equal(contact1.UID, listCards.getNext().QueryInterface(Ci.nsIAbCard).UID);
+  ok(listCards.hasMoreElements());
+  equal(contact3.UID, listCards.getNext().QueryInterface(Ci.nsIAbCard).UID);
+  ok(!listCards.hasMoreElements());
+}
--- a/mailnews/addrbook/test/unit/xpcshell.ini
+++ b/mailnews/addrbook/test/unit/xpcshell.ini
@@ -3,16 +3,17 @@ head = head_addrbook.js
 tail =
 support-files = data/*
 
 [test_basic_nsIAbCard.js]
 [test_basic_nsIAbDirectory.js]
 [test_bug387403.js]
 [test_bug534822.js]
 [test_bug_448165.js]
+[test_bug1522453.js]
 [test_cardForEmail.js]
 [test_collection.js]
 [test_collection_2.js]
 [test_db_enumerator.js]
 [test_ldap1.js]
 [test_ldap2.js]
 [test_ldapOffline.js]
 [test_mailList1.js]