Bug 1639750 - Overhaul sorting of address book contact list. r=mkmelin a=wsmwk
authorGeoff Lankow <geoff@darktrojan.net>
Mon, 22 Jun 2020 23:25:59 +1200
changeset 39462 040be9666a91bda0bd138a813c0aa6ee9ad6f15a
parent 39461 518cb51abae5a42223ac44e8f592330a75abbbd0
child 39463 e2b6205a866d52ec3641afd40df81008b80cda19
push id402
push userclokep@gmail.com
push dateMon, 29 Jun 2020 20:48:04 +0000
reviewersmkmelin, wsmwk
bugs1639750
Bug 1639750 - Overhaul sorting of address book contact list. r=mkmelin a=wsmwk
mail/components/addrbook/test/browser/browser_contact_tree.js
mailnews/addrbook/content/abView.js
--- a/mail/components/addrbook/test/browser/browser_contact_tree.js
+++ b/mail/components/addrbook/test/browser/browser_contact_tree.js
@@ -229,8 +229,111 @@ add_task(async () => {
   MailServices.ab.deleteAddressBook(bookA.URI);
   await deletePromise;
   deletePromise = observer.promiseNotification();
   MailServices.ab.deleteAddressBook(bookB.URI);
   await deletePromise;
 
   observer.cleanUp();
 });
+
+add_task(async () => {
+  function openDirectory(directory) {
+    for (let i = 0; i < abDirTree.view.rowCount; i++) {
+      abDirTree.changeOpenState(i, true);
+    }
+
+    let row = abWindow.gDirectoryTreeView.getIndexForId(directory.URI);
+    mailTestUtils.treeClick(EventUtils, abWindow, abDirTree, row, 0, {});
+  }
+
+  function checkRows(...expectedCards) {
+    Assert.equal(
+      abWindow.gAbView.rowCount,
+      expectedCards.length,
+      "rowCount correct"
+    );
+    for (let i = 0; i < expectedCards.length; i++) {
+      Assert.equal(
+        abWindow.gAbView.getCardFromRow(i).displayName,
+        expectedCards[i].displayName
+      );
+    }
+  }
+
+  let abWindow = await openAddressBookWindow();
+  let abDirTree = abWindow.GetDirTree();
+  let abContactTree = abWindow.document.getElementById("abResultsTree");
+
+  Assert.equal(abContactTree.columns[0].element.id, "GeneratedName");
+  Assert.equal(
+    abContactTree.columns[0].element.getAttribute("sortDirection"),
+    "ascending"
+  );
+  for (let i = 1; i < abContactTree.columns.length; i++) {
+    Assert.equal(
+      abContactTree.columns[i].element.getAttribute("sortDirection"),
+      ""
+    );
+  }
+
+  let bookA = createAddressBook("book A");
+  openDirectory(bookA);
+  checkRows();
+  let contactA2 = bookA.addCard(createContact("contact", "A2"));
+  let contactA1 = bookA.addCard(createContact("contact", "A1")); // Add first.
+  let contactA5 = bookA.addCard(createContact("contact", "A5")); // Add last.
+  checkRows(contactA1, contactA2, contactA5);
+  let contactA3 = bookA.addCard(createContact("contact", "A3")); // Add in the middle.
+  checkRows(contactA1, contactA2, contactA3, contactA5);
+
+  // Flip sort direction.
+  EventUtils.synthesizeMouseAtCenter(
+    abContactTree.columns.GeneratedName.element,
+    {},
+    abWindow
+  );
+  Assert.equal(
+    abContactTree.columns[0].element.getAttribute("sortDirection"),
+    "descending"
+  );
+  checkRows(contactA5, contactA3, contactA2, contactA1);
+  let contactA4 = bookA.addCard(createContact("contact", "A4")); // Add in the middle.
+  checkRows(contactA5, contactA4, contactA3, contactA2, contactA1);
+  let contactA7 = bookA.addCard(createContact("contact", "A7")); // Add first.
+  checkRows(contactA7, contactA5, contactA4, contactA3, contactA2, contactA1);
+  let contactA0 = bookA.addCard(createContact("contact", "A0")); // Add last.
+  checkRows(
+    contactA7,
+    contactA5,
+    contactA4,
+    contactA3,
+    contactA2,
+    contactA1,
+    contactA0
+  );
+
+  contactA3.displayName = "contact A6";
+  contactA3.lastName = "contact A3";
+  contactA3.primaryEmail = "contact.A6@invalid";
+  bookA.modifyCard(contactA3); // Rename, should change position.
+  checkRows(
+    contactA7,
+    contactA3, // Actually A6.
+    contactA5,
+    contactA4,
+    contactA2,
+    contactA1,
+    contactA0
+  );
+
+  // Restore original sort direction.
+  EventUtils.synthesizeMouseAtCenter(
+    abContactTree.columns.GeneratedName.element,
+    {},
+    abWindow
+  );
+  await closeAddressBookWindow(abWindow);
+
+  let deletePromise = promiseDirectoryRemoved();
+  MailServices.ab.deleteAddressBook(bookA.URI);
+  await deletePromise;
+});
--- a/mailnews/addrbook/content/abView.js
+++ b/mailnews/addrbook/content/abView.js
@@ -36,22 +36,25 @@ ABView.prototype = {
   ]),
 
   directory: null,
   listener: null,
   _notifications: [
     "addrbook-directory-invalidated",
     "addrbook-contact-created",
     "addrbook-contact-deleted",
+    "addrbook-contact-updated",
+    "addrbook-list-updated",
     "addrbook-list-member-added",
     "addrbook-list-member-removed",
   ],
 
   sortColumn: "",
   sortDirection: "",
+  collator: new Intl.Collator(undefined, { numeric: true }),
 
   deleteSelectedCards() {
     let directoryMap = new Map();
     for (let i = 0; i < this.selection.getRangeCount(); i++) {
       let start = {};
       let finish = {};
       this.selection.getRangeAt(i, start, finish);
       for (let j = start.value; j <= finish.value; j++) {
@@ -90,20 +93,20 @@ ABView.prototype = {
       if (sortDirection == this.sortDirection) {
         return;
       }
       this._rowMap.reverse();
     } else {
       this._rowMap.sort((a, b) => {
         let aText = a.getText(sortColumn);
         let bText = b.getText(sortColumn);
-        if (aText == bText) {
-          return 0;
+        if (sortDirection == "descending") {
+          return this.collator.compare(bText, aText);
         }
-        return aText < bText ? -1 : 1;
+        return this.collator.compare(aText, bText);
       });
     }
 
     if (this.tree) {
       this.tree.invalidate();
     }
     this.sortColumn = sortColumn;
     this.sortDirection = sortDirection;
@@ -160,21 +163,65 @@ ABView.prototype = {
         }
         break;
       case "addrbook-list-member-added":
         if (!this.directory) {
           break;
         }
       // Falls through.
       case "addrbook-contact-created":
-        this._rowMap.push(new abViewCard(subject));
+        let viewCard = new abViewCard(subject);
+        let sortText = viewCard.getText(this.sortColumn);
+        let added = false;
+        for (let i = this._rowMap.length - 1; !added && i >= 0; i--) {
+          if (
+            this.collator.compare(
+              sortText,
+              this._rowMap[i].getText(this.sortColumn)
+            ) < 0
+          ) {
+            this._rowMap.splice(
+              this.sortDirection == "ascending" ? i : i + 1,
+              0,
+              viewCard
+            );
+            added = true;
+          }
+        }
+        if (!added) {
+          if (this.sortDirection == "ascending") {
+            this._rowMap.push(viewCard);
+          } else {
+            this._rowMap.unshift(viewCard);
+          }
+        }
         if (this.listener) {
           this.listener.onCountChanged(this.rowCount);
         }
         break;
+
+      case "addrbook-list-updated":
+        if (!this.directory) {
+          break;
+        }
+      // Falls through.
+      case "addrbook-contact-updated": {
+        let needsSort = false;
+        for (let i = this._rowMap.length - 1; i >= 0; i--) {
+          if (this._rowMap[i].card.equals(subject)) {
+            this._rowMap[i]._getTextCache = {};
+            needsSort = true;
+          }
+        }
+        if (needsSort) {
+          this.sortBy(this.sortColumn, this.sortDirection, true);
+        }
+        break;
+      }
+
       case "addrbook-list-member-removed":
         if (!this.directory) {
           break;
         }
       // Falls through.
       case "addrbook-contact-deleted":
         for (let i = this._rowMap.length - 1; i >= 0; i--) {
           if (this._rowMap[i].card.equals(subject)) {
@@ -186,19 +233,20 @@ ABView.prototype = {
         }
         break;
     }
   },
 };
 
 function abViewCard(card) {
   this.card = card;
+  this._getTextCache = {};
 }
 abViewCard.prototype = {
-  getText(columnID) {
+  _getText(columnID) {
     try {
       switch (columnID) {
         case "addrbook": {
           let { directoryId } = this.card;
           return directoryId.substring(directoryId.indexOf("&") + 1);
         }
         case "GeneratedName":
           return this.card.generateName(
@@ -212,16 +260,22 @@ abViewCard.prototype = {
           return this.card.isMailList
             ? ""
             : this.card.getPropertyAsAString(columnID);
       }
     } catch (ex) {
       return "";
     }
   },
+  getText(columnID) {
+    if (!(columnID in this._getTextCache)) {
+      this._getTextCache[columnID] = this._getText(columnID);
+    }
+    return this._getTextCache[columnID];
+  },
   get id() {
     return this.card.UID;
   },
   get open() {
     return false;
   },
   get level() {
     return 0;