Bug 451955 - Implement the password selection restoring in Password Manager.; r?MattN draft
authorSean Lee <selee@mozilla.com>
Fri, 09 Dec 2016 17:08:27 -1000
changeset 453237 c619df9f7e344498ee25257826f3b9ea7f0bb54c
parent 452993 8460203bc93b9667cea1bc00f9d9990a4b1a9474
child 453271 5b0c26479636c5991fc86f017b5b1dd5c089445c
push id39608
push userbmo:selee@mozilla.com
push dateFri, 23 Dec 2016 02:55:54 +0000
reviewersMattN
bugs451955
milestone53.0a1
Bug 451955 - Implement the password selection restoring in Password Manager.; r?MattN MozReview-Commit-ID: 9tVmOLd1APj
toolkit/components/passwordmgr/content/passwordManager.js
toolkit/components/passwordmgr/test/browser/browser_passwordmgr_sort.js
--- a/toolkit/components/passwordmgr/content/passwordManager.js
+++ b/toolkit/components/passwordmgr/content/passwordManager.js
@@ -120,17 +120,16 @@ let signonsTreeView = {
   // Keep track of which favicons we've fetched or started fetching.
   // Maps a login origin to a favicon URL.
   _faviconMap: new Map(),
   _filterSet: [],
   // Coalesce invalidations to avoid repeated flickering.
   _invalidateTask: new DeferredTask(() => {
     signonsTree.treeBoxObject.invalidateColumn(signonsTree.columns.siteCol);
   }, 10),
-  _lastSelectedRanges: [],
   selection: null,
 
   rowCount: 0,
   setTree(tree) {},
   getImageSrc(row, column) {
     if (column.element.getAttribute("id") !== "siteCol") {
       return "";
     }
@@ -223,19 +222,16 @@ let signonsTreeView = {
       }
       _editLogin("password");
     }
   },
 };
 
 function SortTree(column, ascending) {
   let table = GetVisibleLogins();
-  // remember which item was selected so we can restore it after the sort
-  let selections = GetTreeSelections();
-  let selectedNumber = selections.length ? table[selections[0]].number : -1;
 
   function compareFunc(a, b) {
     let valA, valB;
     switch (column) {
       case "hostname":
         let realmA = a.httpRealm;
         let realmB = b.httpRealm;
         realmA = realmA == null ? "" : realmA.toLowerCase();
@@ -263,36 +259,18 @@ function SortTree(column, ascending) {
   }
 
   // do the sort
   table.sort(compareFunc);
   if (!ascending) {
     table.reverse();
   }
 
-  // restore the selection
-  let selectedRow = -1;
-  if (selectedNumber >= 0 && false) {
-    for (let s = 0; s < table.length; s++) {
-      if (table[s].number == selectedNumber) {
-        // update selection
-        // note: we need to deselect before reselecting in order to trigger ...Selected()
-        signonsTree.view.selection.select(-1);
-        signonsTree.view.selection.select(s);
-        selectedRow = s;
-        break;
-      }
-    }
-  }
-
   // display the results
   signonsTree.treeBoxObject.invalidate();
-  if (selectedRow >= 0) {
-    signonsTree.treeBoxObject.ensureRowIsVisible(selectedRow);
-  }
 }
 
 function LoadSignons() {
   // loads signons into table
   try {
     signons = Services.logins.getAllLogins();
   } catch (e) {
     signons = [];
@@ -338,16 +316,37 @@ function GetTreeSelections() {
           selections[selections.length] = k;
         }
       }
     }
   }
   return selections;
 }
 
+function StoreSelections(selections) {
+  let selectedSignons = [];
+  let filterSet = signonsTreeView._filterSet;
+  let table = filterSet.length ? filterSet : signons;
+  for (let index of selections) {
+    selectedSignons.push(table[index].guid);
+  }
+  return selectedSignons;
+}
+
+function RestoreSelections(selectedSignons) {
+  let filterSet = signonsTreeView._filterSet;
+  let table = filterSet.length ? filterSet : signons;
+  signonsTreeView.selection.clearSelection();
+  for (let i = 0; i < table.length; i++) {
+    if (selectedSignons.includes(table[i].guid)) {
+      signonsTreeView.selection.rangedSelect(i, i, true);
+    }
+  }
+}
+
 function SignonSelected() {
   let selections = GetTreeSelections();
   if (selections.length) {
     removeButton.removeAttribute("disabled");
   } else {
     removeButton.setAttribute("disabled", true);
   }
 }
@@ -508,16 +507,17 @@ function getColumnByName(column) {
       return document.getElementById("timePasswordChangedCol");
     case "timesUsed":
       return document.getElementById("timesUsedCol");
   }
   return undefined;
 }
 
 function SignonColumnSort(column) {
+  let selectedSignons = StoreSelections(GetTreeSelections());
   let sortedCol = getColumnByName(column);
   let lastSortedCol = getColumnByName(lastSignonSortColumn);
 
   // clear out the sortDirection attribute on the old column
   lastSortedCol.removeAttribute("sortDirection");
 
   // determine if sort is to be ascending or descending
   lastSignonSortAscending = (column == lastSignonSortColumn) ? !lastSignonSortAscending : true;
@@ -525,41 +525,28 @@ function SignonColumnSort(column) {
   // sort
   lastSignonSortColumn = column;
   SortTree(lastSignonSortColumn, lastSignonSortAscending);
 
   // set the sortDirection attribute to get the styling going
   // first we need to get the right element
   sortedCol.setAttribute("sortDirection", lastSignonSortAscending ?
                                           "ascending" : "descending");
+  RestoreSelections(selectedSignons);
 }
 
 function SignonClearFilter() {
-  let singleSelection = (signonsTreeView.selection.count == 1);
-
   // Clear the Tree Display
   signonsTreeView.rowCount = 0;
   signonsTree.treeBoxObject.rowCountChanged(0, -signonsTreeView._filterSet.length);
   signonsTreeView._filterSet = [];
 
   // Just reload the list to make sure deletions are respected
   LoadSignons();
 
-  // Restore selection
-  if (singleSelection) {
-    signonsTreeView.selection.clearSelection();
-    for (let i = 0; i < signonsTreeView._lastSelectedRanges.length; ++i) {
-      let range = signonsTreeView._lastSelectedRanges[i];
-      signonsTreeView.selection.rangedSelect(range.min, range.max, true);
-    }
-  } else {
-    signonsTreeView.selection.select(0);
-  }
-  signonsTreeView._lastSelectedRanges = [];
-
   signonsIntro.textContent = kSignonBundle.getString("loginsDescriptionAll");
 }
 
 function FocusFilterBox() {
   if (filterField.getAttribute("focused") != "true") {
     filterField.focus();
   }
 }
@@ -580,55 +567,41 @@ function SignonMatchesFilter(aSignon, aF
   return false;
 }
 
 function _filterPasswords(aFilterValue, view) {
   aFilterValue = aFilterValue.toLowerCase();
   return signons.filter(s => SignonMatchesFilter(s, aFilterValue));
 }
 
-function SignonSaveState() {
-  // Save selection
-  let seln = signonsTreeView.selection;
-  signonsTreeView._lastSelectedRanges = [];
-  let rangeCount = seln.getRangeCount();
-  for (let i = 0; i < rangeCount; ++i) {
-    let min = {}; let max = {};
-    seln.getRangeAt(i, min, max);
-    signonsTreeView._lastSelectedRanges.push({ min: min.value, max: max.value });
-  }
-}
-
 function FilterPasswords() {
+  let selectedSignons = StoreSelections(GetTreeSelections());
   if (filterField.value == "") {
     SignonClearFilter();
+    RestoreSelections(selectedSignons);
     return;
   }
 
   let newFilterSet = _filterPasswords(filterField.value, signonsTreeView);
-  if (!signonsTreeView._filterSet.length) {
-    // Save Display Info for the Non-Filtered mode when we first
-    // enter Filtered mode.
-    SignonSaveState();
-  }
   signonsTreeView._filterSet = newFilterSet;
 
   // Clear the display
   let oldRowCount = signonsTreeView.rowCount;
   signonsTreeView.rowCount = 0;
   signonsTree.treeBoxObject.rowCountChanged(0, -oldRowCount);
   // Set up the filtered display
   signonsTreeView.rowCount = signonsTreeView._filterSet.length;
   signonsTree.treeBoxObject.rowCountChanged(0, signonsTreeView.rowCount);
 
   // if the view is not empty then select the first item
   if (signonsTreeView.rowCount > 0)
     signonsTreeView.selection.select(0);
 
   signonsIntro.textContent = kSignonBundle.getString("loginsDescriptionFiltered");
+  RestoreSelections(selectedSignons);
 }
 
 function CopyPassword() {
   // Don't copy passwords if we aren't already showing the passwords & a master
   // password hasn't been entered.
   if (!showingPasswords && !masterPasswordLogin())
     return;
   // Copy selected signon's password to clipboard
--- a/toolkit/components/passwordmgr/test/browser/browser_passwordmgr_sort.js
+++ b/toolkit/components/passwordmgr/test/browser/browser_passwordmgr_sort.js
@@ -41,16 +41,19 @@ add_task(function* test() {
         "mypass",
         "smith",
         "very secret",
         "super secret",
         "absolutely",
         "mozilla",
         "mozilla.com",
     ];
+
+    const usernameSelections = ["username", "my username", "my user name"];
+
     let nsLoginInfo = new Components.Constructor("@mozilla.org/login-manager/loginInfo;1",
                                                  Ci.nsILoginInfo, "init");
     for (let i = 0; i < 10; i++)
         pwmgr.addLogin(new nsLoginInfo(urls[i], urls[i], null, users[i], pwds[i],
                                        "u" + (i + 1), "p" + (i + 1)));
 
     // Open the password manager dialog
     const PWMGR_DLG = "chrome://passwordmgr/content/passwordManager.xul";
@@ -102,16 +105,34 @@ add_task(function* test() {
         }
 
         function setFilter(string) {
             filter.value = string;
             filter.doCommand();
             setTimeout(runNextTest, 0);
         }
 
+        function selectItemsByUsername(usernames) {
+          let entries = getColumnEntries(1);
+          for (let username of usernames) {
+            let index = entries.indexOf(username);
+            ok(index != -1, "Username is able to be selected.");
+            sTree.view.selection.rangedSelect(index, index, true);
+          }
+        }
+
+        function checkItemSelections(usernames) {
+          let entries = getColumnEntries(1);
+          for (let username of usernames) {
+            let index = entries.indexOf(username);
+            ok(sTree.view.selection.isSelected(index), "Username must be a selected one.");
+          }
+          sTree.view.selection.clearSelection();
+        }
+
         function checkSortMarkers(activeCol) {
             let isOk = true;
             let col = null;
             let hasAttr = false;
             let treecols = activeCol.parentNode;
             for (let i = 0; i < treecols.childNodes.length; i++) {
                 col = treecols.childNodes[i];
                 if (col.nodeName != "treecol")
@@ -145,56 +166,69 @@ add_task(function* test() {
             return entries;
         }
 
         let testCounter = 0;
         let expectedValues;
         function runNextTest() {
             switch (testCounter++) {
             case 0:
+                selectItemsByUsername(usernameSelections);
                 expectedValues = urls.slice().sort();
                 checkColumnEntries(0, expectedValues);
                 checkSortDirection(siteCol, true);
+                checkItemSelections(usernameSelections);
                 // Toggle sort direction on Host column
                 clickCol(siteCol);
                 break;
             case 1:
+                selectItemsByUsername(usernameSelections);
                 expectedValues.reverse();
                 checkColumnEntries(0, expectedValues);
                 checkSortDirection(siteCol, false);
+                checkItemSelections(usernameSelections);
                 // Sort by Username
                 clickCol(userCol);
                 break;
             case 2:
+                selectItemsByUsername(usernameSelections);
                 expectedValues = users.slice().sort();
                 checkColumnEntries(1, expectedValues);
                 checkSortDirection(userCol, true);
+                checkItemSelections(usernameSelections);
                 // Sort by Password
                 clickCol(passwordCol);
                 break;
             case 3:
+                selectItemsByUsername(usernameSelections);
                 expectedValues = pwds.slice().sort();
                 checkColumnEntries(2, expectedValues);
                 checkSortDirection(passwordCol, true);
+                checkItemSelections(usernameSelections);
                 // Set filter
                 setFilter("moz");
                 break;
             case 4:
+                // Customize the username list due to filter here.
+                selectItemsByUsername(["my username", "my user name"]);
                 expectedValues = [ "absolutely", "mozilla", "mozilla.com",
                                    "mypass", "mypass", "super secret",
                                    "very secret" ];
                 checkColumnEntries(2, expectedValues);
                 checkSortDirection(passwordCol, true);
+                checkItemSelections(["my username", "my user name"]);
                 // Reset filter
                 setFilter("");
                 break;
             case 5:
+                selectItemsByUsername(usernameSelections);
                 expectedValues = pwds.slice().sort();
                 checkColumnEntries(2, expectedValues);
                 checkSortDirection(passwordCol, true);
+                checkItemSelections(usernameSelections);
                 // cleanup
                 Services.ww.registerNotification(function(aSubject, aTopic, aData) {
                     // unregister ourself
                     Services.ww.unregisterNotification(arguments.callee);
 
                     pwmgr.removeAllLogins();
                     resolve();
                 });