Bug 1577849 - Move to the next visible login when the selected login is removed, respecting the applied search filter. r=MattN

Differential Revision: https://phabricator.services.mozilla.com/D44723

--HG--
extra : moz-landing-system : lando

browser/components/aboutlogins/content/components/login-list.js

@@ -338,17 +338,20 @@ export default class LoginList extends HTMLElement {
    *                      nsILoginInfo/nsILoginMetaInfo.
    */
   loginRemoved(login) {
-    this._logins[login.guid].listItem.remove();
-
     // Update the selected list item to the previous item in the list
     // if one exists, otherwise the next item. If no logins remain
-    // the login-intro text will be shown instead of the login-list.
+    // the login-intro or empty-search text will be shown instead of the login-list.
     if (this._selectedGuid == login.guid) {
-      let index = this._loginGuidsSortedOrder.indexOf(login.guid);
-      if (this._loginGuidsSortedOrder.length > 1) {
+      let visibleListItems = this._list.querySelectorAll(
+        ".login-list-item[data-guid]:not([hidden])"
+      );
+      if (visibleListItems.length > 1) {
+        let index = [...visibleListItems].findIndex(listItem => {
+          return listItem.dataset.guid == login.guid;
+        });
         let newlySelectedIndex = index > 0 ? index - 1 : index + 1;
         let newlySelectedLogin = this._logins[
-          this._loginGuidsSortedOrder[newlySelectedIndex]
+          visibleListItems[newlySelectedIndex].dataset.guid
         ].login;
         window.dispatchEvent(
           new CustomEvent("AboutLoginsLoginSelected", {
@@ -359,16 +362,15 @@ export default class LoginList extends HTMLElement {
       }
     }
 
+    this._logins[login.guid].listItem.remove();
     delete this._logins[login.guid];
     this._loginGuidsSortedOrder = this._loginGuidsSortedOrder.filter(guid => {
       return guid != login.guid;
     });
 
-    let visibleLoginGuids = this._applyFilter();
-    this._updateVisibleLoginCount(visibleLoginGuids.size);
-
-    // Since the login has been removed, we don't need to call render
-    // as nothing related to the login needs updating.
+    // Render the login-list to update the search result count and show the
+    // empty-search message if needed.
+    this.render();
   }
 
   /**

browser/components/aboutlogins/tests/browser/browser_deleteLogin.js

@@ -16,25 +16,31 @@ add_task(async function setup() {
 add_task(async function test_show_logins() {
   let browser = gBrowser.selectedBrowser;
 
-  await ContentTask.spawn(browser, [TEST_LOGIN1, TEST_LOGIN2], async logins => {
-    let loginList = Cu.waiveXrays(content.document.querySelector("login-list"));
-    let loginFound = await ContentTaskUtils.waitForCondition(() => {
-      return (
-        loginList._loginGuidsSortedOrder.length == 2 &&
-        loginList._loginGuidsSortedOrder.includes(logins[0].guid) &&
-        loginList._loginGuidsSortedOrder.includes(logins[1].guid)
+  await ContentTask.spawn(
+    browser,
+    [TEST_LOGIN1.guid, TEST_LOGIN2.guid],
+    async loginGuids => {
+      let loginList = Cu.waiveXrays(
+        content.document.querySelector("login-list")
       );
-    }, "Waiting for logins to be displayed");
-    ok(
-      !content.document.documentElement.classList.contains("no-logins"),
-      "Should no longer be in no logins view"
-    );
-    ok(
-      !loginList.classList.contains("no-logins"),
-      "login-list should no longer be in no logins view"
-    );
-    ok(loginFound, "Newly added logins should be added to the page");
-  });
+      let loginFound = await ContentTaskUtils.waitForCondition(() => {
+        return (
+          loginList._loginGuidsSortedOrder.length == 2 &&
+          loginList._loginGuidsSortedOrder.includes(loginGuids[0]) &&
+          loginList._loginGuidsSortedOrder.includes(loginGuids[1])
+        );
+      }, "Waiting for logins to be displayed");
+      ok(
+        !content.document.documentElement.classList.contains("no-logins"),
+        "Should no longer be in no logins view"
+      );
+      ok(
+        !loginList.classList.contains("no-logins"),
+        "login-list should no longer be in no logins view"
+      );
+      ok(loginFound, "Newly added logins should be added to the page");
+    }
+  );
 });
 
 add_task(async function test_login_item() {

browser/components/aboutlogins/tests/chrome/test_login_list.html

@@ -438,6 +438,122 @@ add_task(async function test_sorted_list() {
   is(title1.localeCompare(title2), -1, "Logins should be sorted alphabetically by hostname");
 });
 
+add_task(async function test_login_list_item_removed_next_selected() {
+  let logins = [];
+  for (let i = 0; i < 12; i++) {
+    let group = i % 2 ? "BB" : "AA";
+    // Create logins of the form `jared0AAa@example.com`,
+    // `jared1BBb@example.com`, `jared2AAc@example.com`, etc.
+    logins.push({
+      guid: `${i}`,
+      username: `jared${i}${group}${String.fromCharCode(97 + i)}@example.com`,
+      origin: "https://www.example.com",
+    });
+  }
+
+  gLoginList.setLogins(logins);
+  let visibleLogins = gLoginList.shadowRoot.querySelectorAll(".login-list-item[data-guid]:not([hidden])");
+  await SimpleTest.promiseWaitForCondition(() => {
+    return visibleLogins.length == 12;
+  }, "Waiting for all logins to be visible");
+  is(gLoginList._selectedGuid, logins[0].guid, "login0 should be selected by default");
+
+  window.dispatchEvent(
+    new CustomEvent("AboutLoginsFilterLogins", {
+      bubbles: true,
+      detail: "BB",
+    })
+  );
+
+  await SimpleTest.promiseWaitForCondition(() => {
+    visibleLogins = gLoginList.shadowRoot.querySelectorAll(".login-list-item[data-guid]:not([hidden])");
+    return visibleLogins.length == 6;
+  }, "Only logins with BB in the username should be visible, visible count: " + visibleLogins.length);
+
+  is(gLoginList._selectedGuid, logins[0].guid, "login0 should still be selected after filtering");
+
+  gLoginList.loginRemoved({guid: logins[0].guid});
+
+  await SimpleTest.promiseWaitForCondition(() => {
+    return gLoginList._loginGuidsSortedOrder.length == 11;
+  }, "Waiting for login to get removed");
+
+  await SimpleTest.promiseWaitForCondition(() => {
+    visibleLogins = gLoginList.shadowRoot.querySelectorAll(".login-list-item[data-guid]:not([hidden])");
+    return visibleLogins.length == 6;
+  }, "the number of visible logins should not change, got " + visibleLogins.length);
+  is(gLoginList._selectedGuid, logins[1].guid,
+     "login1 should be selected after delete since the deleted login was not visible and login1 was the first in the list");
+
+  let loginToSwitchTo = gLoginList._logins[visibleLogins[1].dataset.guid].login;
+  window.dispatchEvent(
+    new CustomEvent("AboutLoginsLoginSelected", {
+      bubbles: true,
+      detail: loginToSwitchTo,
+    })
+  );
+  is(gLoginList._selectedGuid, loginToSwitchTo.guid, "login3 should be selected");
+
+  gLoginList.loginRemoved({guid: logins[3].guid});
+
+  await SimpleTest.promiseWaitForCondition(() => {
+    return gLoginList._loginGuidsSortedOrder.length == 10;
+  }, "Waiting for login to get removed");
+
+  await SimpleTest.promiseWaitForCondition(() => {
+    visibleLogins = gLoginList.shadowRoot.querySelectorAll(
+      ".login-list-item[data-guid]:not([hidden])"
+    );
+    return visibleLogins.length == 5;
+  }, "the number of filtered logins should decrease by 1");
+  is(gLoginList._selectedGuid, visibleLogins[0].dataset.guid, "the first login should now be selected");
+
+  gLoginList.loginRemoved({guid: logins[1].guid});
+
+  await SimpleTest.promiseWaitForCondition(() => {
+    return gLoginList._loginGuidsSortedOrder.length == 9;
+  }, "Waiting for login to get removed");
+
+  await SimpleTest.promiseWaitForCondition(() => {
+    visibleLogins = gLoginList.shadowRoot.querySelectorAll(
+      ".login-list-item[data-guid]:not([hidden])"
+    );
+    return visibleLogins.length == 4;
+  }, "the number of filtered logins should decrease by 1");
+  is(gLoginList._selectedGuid, visibleLogins[0].dataset.guid, "the first login should now still be selected");
+
+  loginToSwitchTo = gLoginList._logins[visibleLogins[3].dataset.guid].login;
+  window.dispatchEvent(
+    new CustomEvent("AboutLoginsLoginSelected", {
+      bubbles: true,
+      detail: loginToSwitchTo,
+    })
+  );
+  is(gLoginList._selectedGuid, visibleLogins[3].dataset.guid, "the last login should now still be selected");
+
+  gLoginList.loginRemoved({guid: logins[10].guid});
+
+  await SimpleTest.promiseWaitForCondition(() => {
+    return gLoginList._loginGuidsSortedOrder.length == 8;
+  }, "Waiting for login to get removed");
+
+  await SimpleTest.promiseWaitForCondition(() => {
+    visibleLogins = gLoginList.shadowRoot.querySelectorAll(
+      ".login-list-item[data-guid]:not([hidden])"
+    );
+    return visibleLogins.length == 4;
+  }, "the number of filtered logins should decrease by 1");
+  is(gLoginList._selectedGuid, visibleLogins[3].dataset.guid, "the last login should now be selected");
+
+  loginToSwitchTo = gLoginList._logins[visibleLogins[2].dataset.guid].login;
+  window.dispatchEvent(
+    new CustomEvent("AboutLoginsLoginSelected", {
+      bubbles: true,
+      detail: loginToSwitchTo,
+    })
+  );
+  is(gLoginList._selectedGuid, visibleLogins[2].dataset.guid, "the last login should now still be selected");
+});
 </script>
 
 </body>