Bug 1560359 - Add keyboard support for login-list on about:logins. r=fluent-reviewers,sfoster,flod,yzen
☠☠ backed out by 7575aaa9db33 ☠ ☠
authorJared Wein <jwein@mozilla.com>
Tue, 02 Jul 2019 18:32:18 +0000
changeset 540647 ba5bdea1b6082f84171d5ad31209764b369e54f2
parent 540646 019ec2921d00611da2d2d99a37919f66f64a2c54
child 540648 2c0e25c99a726d63508c43a431546a85671d76c6
push id11529
push userarchaeopteryx@coole-files.de
push dateThu, 04 Jul 2019 15:22:33 +0000
treeherdermozilla-beta@ebb510a784b8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfluent-reviewers, sfoster, flod, yzen
bugs1560359
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1560359 - Add keyboard support for login-list on about:logins. r=fluent-reviewers,sfoster,flod,yzen Differential Revision: https://phabricator.services.mozilla.com/D35828
browser/components/aboutlogins/content/aboutLogins.css
browser/components/aboutlogins/content/aboutLogins.ftl
browser/components/aboutlogins/content/aboutLogins.html
browser/components/aboutlogins/content/components/login-item.css
browser/components/aboutlogins/content/components/login-item.js
browser/components/aboutlogins/content/components/login-list-item.css
browser/components/aboutlogins/content/components/login-list-item.js
browser/components/aboutlogins/content/components/login-list.css
browser/components/aboutlogins/content/components/login-list.js
browser/components/aboutlogins/tests/chrome/test_login_list.html
--- a/browser/components/aboutlogins/content/aboutLogins.css
+++ b/browser/components/aboutlogins/content/aboutLogins.css
@@ -22,17 +22,16 @@ header {
 
 login-filter {
   flex: auto;
   align-self: center;
 }
 
 login-list {
   grid-area: logins;
-  overflow: hidden auto;
 }
 
 login-item {
   grid-area: login;
   max-width: 800px;
 }
 
 #branding-logo {
--- a/browser/components/aboutlogins/content/aboutLogins.ftl
+++ b/browser/components/aboutlogins/content/aboutLogins.ftl
@@ -13,16 +13,17 @@
 about-logins-page-title = Logins & Passwords
 
 create-login-button = New Login
 
 login-filter =
   .placeholder = Search Logins
 
 login-list =
+  .aria-label = Logins matching search query
   .count =
     { $count ->
         [one] { $count } login
        *[other] { $count } logins
     }
   .last-changed-option = Last Changed
   .last-used-option = Last Used
   .missing-username = (no username)
--- a/browser/components/aboutlogins/content/aboutLogins.html
+++ b/browser/components/aboutlogins/content/aboutLogins.html
@@ -29,17 +29,18 @@
       <menu-button data-l10n-id="menu-button"
                    data-l10n-attrs="button-title,
                                     menuitem-feedback,
                                     menuitem-import,
                                     menuitem-preferences"></menu-button>
     </header>
     <login-list data-l10n-id="login-list"
                 data-l10n-args='{"count": 0}'
-                data-l10n-attrs="count,
+                data-l10n-attrs="aria-label,
+                                 count,
                                  last-changed-option,
                                  last-used-option,
                                  missing-username,
                                  name-option,
                                  new-login-subtitle,
                                  new-login-title,
                                  sort-label-text"></login-list>
     <login-item data-l10n-id="login-item"
@@ -74,17 +75,17 @@
           <select id="login-sort">
             <option class="name-option" value="name"/>
             <option class="last-used-option" value="last-used"/>
             <option class="last-changed-option" value="last-changed"/>
           </select>
         </label>
         <span class="count"></span>
       </div>
-      <ol>
+      <ol role="listbox" tabindex="0">
       </ol>
     </template>
 
     <template id="login-list-item-template">
       <link rel="stylesheet" href="chrome://global/skin/in-content/common.css">
       <link rel="stylesheet" href="chrome://browser/content/aboutlogins/components/login-list-item.css">
       <span class="title"></span>
       <span class="username"></span>
--- a/browser/components/aboutlogins/content/components/login-item.css
+++ b/browser/components/aboutlogins/content/components/login-item.css
@@ -125,16 +125,26 @@
 .reveal-password-checkbox:hover:active {
   opacity: var(--reveal-checkbox-opacity-active);
 }
 
 .reveal-password-checkbox:checked {
   background-image: url("chrome://browser/content/aboutlogins/icons/hide-password.svg") !important;
 }
 
+.reveal-password-checkbox:-moz-focusring {
+  outline: 2px solid var(--in-content-border-active);
+  /* offset outline to align with 1px border-width set for buttons/menulists above. */
+  outline-offset: -1px;
+  /* Make outline-radius slightly bigger than the border-radius set above,
+   * to make the thicker outline corners look smooth */
+  -moz-outline-radius: 3px;
+  box-shadow: 0 0 0 4px var(--in-content-border-active-shadow);
+}
+
 @supports -moz-bool-pref("browser.in-content.dark-mode") {
 @media (prefers-color-scheme: dark) {
   :host {
     --reveal-checkbox-opacity: .8;
     --reveal-checkbox-opacity-hover: 1;
     --reveal-checkbox-opacity-active: .6;
   }
 }
--- a/browser/components/aboutlogins/content/components/login-item.js
+++ b/browser/components/aboutlogins/content/components/login-item.js
@@ -248,16 +248,17 @@ export default class LoginItem extends R
       delete this.dataset.isNewLogin;
     } else {
       this.dataset.isNewLogin = true;
     }
     this._toggleEditing(!login.guid);
 
     this._revealCheckbox.checked = false;
 
+    this._editButton.focus();
     this.render();
   }
 
   /**
    * Updates the view if the login argument matches the login currently
    * displayed.
    *
    * @param {login} login The login that was added to storage. The login object is
@@ -359,21 +360,26 @@ export default class LoginItem extends R
     } else {
       // Need to set a shorter width than -moz-available so the reveal checkbox
       // will still appear next to the password.
       this._passwordInput.style.width = (this._login.password || "").length + "ch";
     }
 
     this._deleteButton.disabled = this.dataset.isNewLogin;
     this._editButton.disabled = shouldEdit;
-    this._originInput.readOnly = !this.dataset.isNewLogin;
+    let inputTabIndex = !shouldEdit ? -1 : 0;
+    this._originInput.readOnly = !shouldEdit;
+    this._originInput.tabIndex = inputTabIndex;
     this._usernameInput.readOnly = !shouldEdit;
+    this._usernameInput.tabIndex = inputTabIndex;
     this._passwordInput.readOnly = !shouldEdit;
+    this._passwordInput.tabIndex = inputTabIndex;
     if (shouldEdit) {
       this.dataset.editing = true;
+      this._originInput.focus();
     } else {
       delete this.dataset.editing;
     }
   }
 
   _updatePasswordRevealState() {
     let labelAttr = this._revealCheckbox.checked ? "password-show-title"
                                                  : "password-hide-title";
--- a/browser/components/aboutlogins/content/components/login-list-item.css
+++ b/browser/components/aboutlogins/content/components/login-list-item.css
@@ -19,16 +19,21 @@
 :host(:hover) {
   background-color: var(--in-content-box-background-hover);
 }
 
 :host(:hover:active) {
   background-color: var(--in-content-box-background-active);
 }
 
+:host(.keyboard-selected) {
+  border-inline-start-color: var(--in-content-border-active-shadow);
+  background-color: var(--in-content-box-background-odd);
+}
+
 :host(.selected) {
   border-inline-start-color: var(--in-content-border-highlight);
   background-color: var(--in-content-box-background-hover);
 }
 
 .title {
   font-weight: bold;
 }
--- a/browser/components/aboutlogins/content/components/login-list-item.js
+++ b/browser/components/aboutlogins/content/components/login-list-item.js
@@ -3,30 +3,35 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 import {recordTelemetryEvent} from "../aboutLoginsUtils.js";
 
 export default class LoginListItem extends HTMLElement {
   constructor(login) {
     super();
     this._login = login;
+    this.id = login.guid ?
+      // Prepend the ID with a string since IDs must not begin with a number.
+      "lli-" + this._login.guid :
+      "new-login-list-item";
   }
 
   connectedCallback() {
     if (this.shadowRoot) {
       this.render();
       return;
     }
 
     let loginListItemTemplate = document.querySelector("#login-list-item-template");
     this.attachShadow({mode: "open"})
         .appendChild(loginListItemTemplate.content.cloneNode(true));
 
     this._title = this.shadowRoot.querySelector(".title");
     this._username = this.shadowRoot.querySelector(".username");
+    this.setAttribute("role", "option");
 
     this.render();
 
     this.addEventListener("click", this);
   }
 
   render() {
     if (!this._login.guid) {
--- a/browser/components/aboutlogins/content/components/login-list.css
+++ b/browser/components/aboutlogins/content/components/login-list.css
@@ -11,18 +11,16 @@
 }
 
 .meta {
   display: flex;
   align-items: center;
   padding: 10px 18px;
   border-bottom: 1px solid var(--in-content-box-border-color);
   background-color: var(--in-content-box-info-background);
-  position: sticky;
-  top: 0;
 }
 
 .count {
   flex: auto;
   text-align: end;
   font-size: smaller;
   margin-inline-start: 18px;
 }
--- a/browser/components/aboutlogins/content/components/login-list.js
+++ b/browser/components/aboutlogins/content/components/login-list.js
@@ -32,38 +32,43 @@ export default class LoginList extends R
     this._list = this.shadowRoot.querySelector("ol");
 
     this.render();
 
     this.shadowRoot.getElementById("login-sort")
                    .addEventListener("change", this);
     window.addEventListener("AboutLoginsLoginSelected", this);
     window.addEventListener("AboutLoginsFilterLogins", this);
+    this.addEventListener("keydown", this);
 
     super.connectedCallback();
   }
 
   render() {
     this._list.textContent = "";
 
     if (!this._logins.length) {
       document.l10n.setAttributes(this, "login-list", {count: 0});
       return;
     }
 
     if (!this._selectedGuid) {
       this._blankLoginListItem.classList.add("selected");
+      this._blankLoginListItem.setAttribute("aria-selected", "true");
+      this._list.setAttribute("aria-activedescendant", this._blankLoginListItem.id);
       this._list.append(this._blankLoginListItem);
     }
 
     for (let login of this._logins) {
       let listItem = new LoginListItem(login);
       listItem.setAttribute("missing-username", this.getAttribute("missing-username"));
       if (login.guid == this._selectedGuid) {
         listItem.classList.add("selected");
+        listItem.setAttribute("aria-selected", "true");
+        this._list.setAttribute("aria-activedescendant", listItem.id);
       }
       this._list.append(listItem);
     }
 
     let visibleLoginCount = this._applyFilter();
     document.l10n.setAttributes(this, "login-list", {count: visibleLoginCount});
   }
 
@@ -84,36 +89,45 @@ export default class LoginList extends R
         if (this._selectedGuid == event.detail.guid) {
           return;
         }
 
         this._selectedGuid = event.detail.guid || null;
         this.render();
         break;
       }
+      case "keydown": {
+        this._handleKeyboardNav(event);
+        break;
+      }
     }
   }
 
   static get reflectedFluentIDs() {
-    return ["count",
+    return ["aria-label",
+            "count",
             "last-used-option",
             "last-changed-option",
             "missing-username",
             "name-option",
             "new-login-subtitle",
             "new-login-title",
             "sort-label-text"];
   }
 
   static get observedAttributes() {
     return this.reflectedFluentIDs;
   }
 
   handleSpecialCaseFluentString(attrName) {
     switch (attrName) {
+      case "aria-label": {
+        this._list.setAttribute("aria-label", this.getAttribute(attrName));
+        break;
+      }
       case "missing-username": {
         break;
       }
       case "new-login-subtitle":
       case "new-login-title": {
         this._blankLoginListItem.setAttribute(attrName, this.getAttribute(attrName));
         break;
       }
@@ -189,10 +203,89 @@ export default class LoginList extends R
         }
       } else if (!listItem.hidden) {
         listItem.hidden = true;
       }
     }
 
     return matchingLoginGuids.length;
   }
+
+  _handleKeyboardNav(event) {
+    if (this._list != this.shadowRoot.activeElement) {
+      return;
+    }
+
+    let isLTR = document.dir == "ltr";
+    let activeDescendantId = this._list.getAttribute("aria-activedescendant");
+    let activeDescendant = activeDescendantId ?
+      this.shadowRoot.getElementById(activeDescendantId) :
+      this._list.firstElementChild;
+    let newlyFocusedItem = null;
+    switch (event.key) {
+      case "ArrowDown": {
+        let nextItem = activeDescendant.nextElementSibling;
+        if (!nextItem) {
+          return;
+        }
+        newlyFocusedItem = nextItem;
+        break;
+      }
+      case "ArrowLeft": {
+        let item = isLTR ?
+          activeDescendant.previousElementSibling :
+          activeDescendant.nextElementSibling;
+        if (!item) {
+          return;
+        }
+        newlyFocusedItem = item;
+        break;
+      }
+      case "ArrowRight": {
+        let item = isLTR ?
+          activeDescendant.nextElementSibling :
+          activeDescendant.previousElementSibling;
+        if (!item) {
+          return;
+        }
+        newlyFocusedItem = item;
+        break;
+      }
+      case "ArrowUp": {
+        let previousItem = activeDescendant.previousElementSibling;
+        if (!previousItem) {
+          return;
+        }
+        newlyFocusedItem = previousItem;
+        break;
+      }
+      case "Tab": {
+        // Bug 1562716: Pressing Tab from the login-list cycles back to the
+        // login-sort dropdown due to the login-list having `overflow`
+        // CSS property set. Explicitly forward focus here until
+        // this keyboard trap is fixed.
+        if (event.shiftKey) {
+          return;
+        }
+        let loginItem = document.querySelector("login-item");
+        if (loginItem) {
+          event.preventDefault();
+          loginItem.shadowRoot.querySelector(".edit-button").focus();
+        }
+        return;
+      }
+      case " ":
+      case "Enter": {
+        event.preventDefault();
+        activeDescendant.click();
+        return;
+      }
+      default:
+        return;
+    }
+    event.preventDefault();
+    this._list.setAttribute("aria-activedescendant", newlyFocusedItem.id);
+    activeDescendant.classList.remove("keyboard-selected");
+    newlyFocusedItem.classList.add("keyboard-selected");
+    newlyFocusedItem.scrollIntoView(false);
+  }
 }
 customElements.define("login-list", LoginList);
--- a/browser/components/aboutlogins/tests/chrome/test_login_list.html
+++ b/browser/components/aboutlogins/tests/chrome/test_login_list.html
@@ -2,16 +2,17 @@
 <html>
 <!--
 Test the login-list component
 -->
 <head>
   <meta charset="utf-8">
   <title>Test the login-list component</title>
   <script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <script src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"></script>
   <script type="module" src="chrome://browser/content/aboutlogins/components/login-list.js"></script>
   <script src="aboutlogins_common.js"></script>
 
   <link rel="stylesheet" href="/tests/SimpleTest/test.css"/>
 </head>
 <body>
   <p id="display">
   </p>
@@ -69,17 +70,69 @@ add_task(async function setup() {
   displayEl.appendChild(gLoginList);
 });
 
 add_task(async function test_empty_list() {
   ok(gLoginList, "loginList exists");
   is(gLoginList.textContent, "", "Initially empty");
 });
 
+add_task(async function test_keyboard_navigation() {
+  gLoginList.setLogins([TEST_LOGIN_1, TEST_LOGIN_2, TEST_LOGIN_3]);
+
+  while (document.activeElement != gLoginList) {
+    sendKey("TAB");
+    await new Promise(resolve => requestAnimationFrame(resolve));
+  }
+
+  sendKey("TAB");
+  sendKey("TAB");
+  let loginSort = gLoginList.shadowRoot.querySelector("#login-sort");
+  await SimpleTest.promiseWaitForCondition(() => loginSort == gLoginList.shadowRoot.activeElement,
+    "waiting for login-sort to get focus");
+  ok(loginSort == gLoginList.shadowRoot.activeElement, "#login-sort should be focused after tabbing to it");
+
+  sendKey("TAB");
+  let ol = gLoginList.shadowRoot.querySelector("ol");
+  await SimpleTest.promiseWaitForCondition(() => ol.matches(":focus"),
+    "waiting for 'ol' to get focus");
+  ok(ol.matches(":focus"), "'ol' should be focused after tabbing to it");
+
+  for (let [keyFwd, keyRev] of [["LEFT", "RIGHT"], ["DOWN", "UP"]]) {
+    sendKey(keyFwd);
+    await SimpleTest.promiseWaitForCondition(() => ol.getAttribute("aria-activedescendant") == ol.children[1].id,
+      `waiting for second item in list to get focused (${keyFwd})`);
+    ok(ol.children[1].classList.contains("keyboard-selected"), `second item should be marked as keyboard-selected (${keyFwd})`);
+
+    sendKey(keyRev);
+    await SimpleTest.promiseWaitForCondition(() => ol.getAttribute("aria-activedescendant") == ol.children[0].id,
+      `waiting for first item in list to get focused (${keyRev})`);
+    ok(ol.children[0].classList.contains("keyboard-selected"), `first item should be marked as keyboard-selected (${keyRev})`);
+  }
+
+  sendKey("DOWN");
+  await SimpleTest.promiseWaitForCondition(() => ol.getAttribute("aria-activedescendant") == ol.children[1].id,
+    `waiting for second item in list to get focused (DOWN)`);
+  ok(ol.children[1].classList.contains("keyboard-selected"), `second item should be marked as keyboard-selected (DOWN)`);
+  let selectedGuid = ol.children[1].dataset.guid;
+
+  let loginSelectedEvent = null;
+  gLoginList.addEventListener("AboutLoginsLoginSelected", event => loginSelectedEvent = event, {once: true});
+  sendKey("RETURN");
+  is(ol.querySelector(".selected").dataset.guid, selectedGuid, "item should be marked as selected");
+  ok(loginSelectedEvent, "AboutLoginsLoginSelected event should be dispatched on pressing Enter");
+  is(loginSelectedEvent.detail.guid, selectedGuid, "event should have expected login attached");
+});
+
 add_task(async function test_empty_login_username_in_list() {
+  // Clear the selection so the 'new' login will be in the list too.
+  window.dispatchEvent(new CustomEvent("AboutLoginsLoginSelected", {
+    detail: {},
+  }));
+
   gLoginList.setLogins([TEST_LOGIN_3]);
   let loginListItems = gLoginList.shadowRoot.querySelectorAll("login-list-item");
   is(loginListItems.length, 2, "A blank login and the one stored login should be displayed");
   ok(!loginListItems[0].dataset.guid, "first login-list-item should be the 'new' item");
   is(loginListItems[1].dataset.guid, TEST_LOGIN_3.guid, "login-list-item should have correct guid attribute");
 
   loginListItems[1].setAttribute("missing-username", "(no username)");
   loginListItems[1].render();