Bug 1588795 - Fix broken mailing list dialog. r=pmorris
authorGeoff Lankow <geoff@darktrojan.net>
Wed, 16 Oct 2019 14:42:47 +1300
changeset 37176 2a06340a320b1497372f2a05977d79f625ef5f9c
parent 37175 c4c283656a88f263c19d95ad17e816f72ff4ccba
child 37177 e2e9a71dc663501ae30b40c308f4ef006a844960
push id395
push userclokep@gmail.com
push dateMon, 02 Dec 2019 19:38:57 +0000
reviewerspmorris
bugs1588795
Bug 1588795 - Fix broken mailing list dialog. r=pmorris
mailnews/addrbook/content/abMailListDialog.js
mailnews/addrbook/jsaddrbook/AddrBookMailingList.jsm
mailnews/addrbook/test/unit/test_jsaddrbook.js
--- a/mailnews/addrbook/content/abMailListDialog.js
+++ b/mailnews/addrbook/content/abMailListDialog.js
@@ -7,16 +7,19 @@
 
 var { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 var { MailServices } = ChromeUtils.import(
   "resource:///modules/MailServices.jsm"
 );
 var { AppConstants } = ChromeUtils.import(
   "resource://gre/modules/AppConstants.jsm"
 );
+var { fixIterator, toXPCOMArray } = ChromeUtils.import(
+  "resource:///modules/iteratorUtils.jsm"
+);
 
 top.MAX_RECIPIENTS = 1;
 var inputElementType = "";
 
 var gListCard;
 var gEditList;
 var gOldListName = "";
 var gLoadListeners = [];
@@ -44,18 +47,18 @@ function mailingListExists(listname) {
   }
   return false;
 }
 
 /**
  * Get the new inputs from the create/edit mailing list dialog and use them to
  * update the mailing list that was passed in as an argument.
  *
- * @param {XPCWrappedNative_NoHelper} mailList - The mailing list object to
- *   update. When creating a new list it will be newly created and empty.
+ * @param {nsIAbDirectory} mailList - The mailing list object to update. When
+ *   creating a new list it will be newly created and empty.
  * @param {boolean} isNewList - Whether we are populating a new list.
  * @return {boolean} - Whether the operation succeeded or not.
  */
 function updateMailList(mailList, isNewList) {
   let listname = document.getElementById("ListName").value.trim();
 
   if (listname.length == 0) {
     alert(gAddressBookBundle.getString("emptyListName"));
@@ -82,58 +85,69 @@ function updateMailList(mailList, isNewL
     }
   }
 
   mailList.isMailList = true;
   mailList.dirName = listname;
   mailList.listNickName = document.getElementById("ListNickName").value;
   mailList.description = document.getElementById("ListDescription").value;
 
+  return true;
+}
+
+/**
+ * Updates the members of the mailing list.
+ *
+ * @param {nsIAbDirectory} mailList - The mailing list object to
+ *   update. When creating a new list it will be newly created and empty.
+ * @param {nsIAbDirectory} parentDirectory - The address book containing the
+ *   mailing list.
+ */
+function updateMailListMembers(mailList, parentDirectory) {
   // Gather email address inputs into a single string (comma-separated).
   let addresses = Array.from(
     document.querySelectorAll(".textbox-addressingWidget"),
     element => element.value
   )
     .filter(value => value.trim())
     .join();
 
   // Convert the addresses string into address objects.
   let addressObjects = MailServices.headerParser.makeFromDisplayAddress(
     addresses
   );
+  let existingCards = [...fixIterator(mailList.addressLists, Ci.nsIAbCard)];
 
-  // Update the list by updating existing entries/cards or adding new ones.
-  let oldAddressCount = isNewList ? 0 : mailList.addressLists.length;
-  let addressCounter = 0;
-
-  for (let { email, name } of addressObjects) {
-    let addNewCard = addressCounter >= oldAddressCount;
+  // Work out which addresses need to be added...
+  let existingCardAddresses = existingCards.map(card => card.primaryEmail);
+  let addressObjectsToAdd = addressObjects.filter(
+    aObj => !existingCardAddresses.includes(aObj.email)
+  );
 
-    let card = addNewCard
-      ? Cc["@mozilla.org/addressbook/cardproperty;1"].createInstance()
-      : mailList.addressLists.queryElementAt(addressCounter, Ci.nsIAbCard);
+  // ... and which need to be removed.
+  let addressObjectAddresses = addressObjects.map(aObj => aObj.email);
+  let cardsToRemove = existingCards.filter(
+    card => !addressObjectAddresses.includes(card.primaryEmail)
+  );
 
-    card = card && card.QueryInterface(Ci.nsIAbCard);
-
-    if (card) {
+  for (let { email, name } of addressObjectsToAdd) {
+    let card = parentDirectory.cardForEmailAddress(email);
+    if (!card) {
+      card = Cc["@mozilla.org/addressbook/cardproperty;1"].createInstance(
+        Ci.nsIAbCard
+      );
       card.primaryEmail = email;
       card.displayName = name || email;
-
-      if (addNewCard) {
-        mailList.addressLists.appendElement(card);
-      }
-      addressCounter += 1;
     }
+    mailList.addCard(card);
   }
 
-  // Remove any remaining unneeded entries.
-  while (mailList.addressLists.length > addressCounter) {
-    mailList.addressLists.removeElementAt(addressCounter);
+  if (cardsToRemove.length > 0) {
+    mailList.deleteCards(toXPCOMArray(cardsToRemove, Ci.nsIMutableArray));
   }
-  return true;
 }
 
 function MailListOKButton(event) {
   var popup = document.getElementById("abPopup");
   if (popup) {
     var uri = popup.getAttribute("value");
 
     // FIX ME - hack to avoid crashing if no ab selected because of blank option bug from template
@@ -148,16 +162,17 @@ function MailListOKButton(event) {
     var mailList = Cc[
       "@mozilla.org/addressbook/directoryproperty;1"
     ].createInstance();
     mailList = mailList.QueryInterface(Ci.nsIAbDirectory);
 
     if (updateMailList(mailList, true)) {
       var parentDirectory = GetDirectoryFromURI(uri);
       mailList = parentDirectory.addMailList(mailList);
+      updateMailListMembers(mailList, parentDirectory);
       NotifySaveListeners(mailList);
     } else {
       event.preventDefault();
     }
   }
 }
 
 function OnLoadNewMailList() {
@@ -216,16 +231,19 @@ function OnLoadNewMailList() {
   input.popup.addEventListener("click", () => {
     awReturnHit(input);
   });
 }
 
 function EditListOKButton(event) {
   // edit mailing list in database
   if (updateMailList(gEditList, false)) {
+    let parentURI = GetParentDirectoryFromMailingListURI(gEditList.URI);
+    let parentDirectory = GetDirectoryFromURI(parentURI);
+    updateMailListMembers(gEditList, parentDirectory);
     if (gListCard) {
       // modify the list card (for the results pane) from the mailing list
       gListCard.displayName = gEditList.dirName;
       gListCard.lastName = gEditList.dirName;
       gListCard.setProperty("NickName", gEditList.listNickName);
       gListCard.setProperty("Notes", gEditList.description);
     }
 
--- a/mailnews/addrbook/jsaddrbook/AddrBookMailingList.jsm
+++ b/mailnews/addrbook/jsaddrbook/AddrBookMailingList.jsm
@@ -217,22 +217,31 @@ AddrBookMailingList.prototype = {
       },
 
       get directoryId() {
         return self._parent.uuid;
       },
       get localId() {
         return self._localId;
       },
+      get firstName() {
+        return "";
+      },
+      get lastName() {
+        return self._name;
+      },
       get displayName() {
         return self._name;
       },
       set displayName(value) {
         self._name = value;
       },
+      get primaryEmail() {
+        return "";
+      },
 
       generateName(generateFormat) {
         return self._name;
       },
       getProperty(name, defaultValue) {
         switch (name) {
           case "NickName":
             return self._nickName;
--- a/mailnews/addrbook/test/unit/test_jsaddrbook.js
+++ b/mailnews/addrbook/test/unit/test_jsaddrbook.js
@@ -282,16 +282,22 @@ add_task(async function createMailingLis
   equal(list.isMailList, true);
   equal(list.isQuery, false);
   equal(list.supportsMailingLists, false);
 
   // Check list enumerations.
   equal(Array.from(list.addressLists.enumerate()).length, 0);
   equal(Array.from(list.childNodes).length, 0);
   equal(Array.from(list.childCards).length, 0);
+
+  // Check nsIAbCard properties.
+  equal(listCard.firstName, "");
+  equal(listCard.lastName, "new list");
+  equal(listCard.primaryEmail, "");
+  equal(listCard.displayName, "new list");
 });
 
 add_task(async function editMailingList() {
   list.dirName = "updated list";
   list.editMailListToDatabase(null);
   observer.checkEvents(
     ["onItemPropertyChanged", list, "DirName", undefined, "updated list"],
     ["onItemPropertyChanged", list, "DirName", undefined, "updated list"], // Seriously?