From b9661755d7f97dbc6c9b339d17f2692ce21ec6bf Mon Sep 17 00:00:00 2001 From: Tim Giles Date: Fri, 11 Mar 2022 15:19:53 +0000 Subject: [PATCH] Bug 1756387 - Fix keyboard navigation issues on about:logins. r=sgalich,dimi Differential Revision: https://phabricator.services.mozilla.com/D139848 --- .../aboutlogins/content/aboutLogins.css | 2 +- .../aboutlogins/content/aboutLogins.html | 16 +- .../tests/browser/browser_tabKeyNav.js | 177 ++++++++++++------ 3 files changed, 132 insertions(+), 63 deletions(-) diff --git a/browser/components/aboutlogins/content/aboutLogins.css b/browser/components/aboutlogins/content/aboutLogins.css index 0c96440ec996..a28df926b6a0 100644 --- a/browser/components/aboutlogins/content/aboutLogins.css +++ b/browser/components/aboutlogins/content/aboutLogins.css @@ -15,7 +15,7 @@ body { --sidebar-width: 320px; display: grid; grid-template-columns: var(--sidebar-width) 1fr; - grid-template-rows: 1fr; + grid-template-rows: auto 1fr; } @media (max-width: 830px) { diff --git a/browser/components/aboutlogins/content/aboutLogins.html b/browser/components/aboutlogins/content/aboutLogins.html index 3f8a99410ecd..0ff238689962 100644 --- a/browser/components/aboutlogins/content/aboutLogins.html +++ b/browser/components/aboutlogins/content/aboutLogins.html @@ -33,16 +33,14 @@ +
+ + + +
-
-
- - - -
- - -
+ + diff --git a/browser/components/aboutlogins/tests/browser/browser_tabKeyNav.js b/browser/components/aboutlogins/tests/browser/browser_tabKeyNav.js index fa49996281b7..a9f73bb6002e 100644 --- a/browser/components/aboutlogins/tests/browser/browser_tabKeyNav.js +++ b/browser/components/aboutlogins/tests/browser/browser_tabKeyNav.js @@ -30,70 +30,137 @@ add_task(async function test_tab_key_nav() { let browser = gBrowser.selectedBrowser; await SpecialPowers.spawn(browser, [], async () => { const EventUtils = ContentTaskUtils.getEventUtils(content); + // list [selector, shadow root selector] for each element + // in the order we expect them to be navigated. + let expectedElementsInOrder = [ + ["login-filter", "input"], + ["fxaccounts-button", "button"], + ["menu-button", "button"], + ["login-list", "select"], + ["login-list", "ol"], + ["login-list", "button"], + ["login-item", "button.edit-button"], + ["login-item", "button.delete-button"], + ["login-item", "a.origin-input"], + ["login-item", "button.copy-username-button"], + ["login-item", "input.reveal-password-checkbox"], + ["login-item", "button.copy-password-button"], + ]; + + let firstElement = content.document + .querySelector(expectedElementsInOrder.at(0).at(0)) + .shadowRoot.querySelector(expectedElementsInOrder.at(0).at(1)); + let lastElement = content.document + .querySelector(expectedElementsInOrder.at(-1).at(0)) + .shadowRoot.querySelector(expectedElementsInOrder.at(-1).at(1)); async function tab() { EventUtils.synthesizeKey("KEY_Tab", {}, content); await new Promise(resolve => content.requestAnimationFrame(resolve)); // The following line can help with focus trap debugging: - // await new Promise(resolve => content.window.setTimeout(resolve, 100)); + // await new Promise(resolve => content.window.setTimeout(resolve, 2000)); + } + async function shiftTab() { + EventUtils.synthesizeKey("KEY_Tab", { shiftKey: true }, content); + await new Promise(resolve => content.requestAnimationFrame(resolve)); } // Getting focused shadow DOM element itself instead of shadowRoot, // using recursion for any component-nesting level, as in: // document.activeElement.shadowRoot.activeElement.shadowRoot.activeElement - function getFocusedEl() { - let el = content.document.activeElement; + function getFocusedElement() { + let element = content.document.activeElement; const getShadowRootFocus = e => { if (e.shadowRoot) { return getShadowRootFocus(e.shadowRoot.activeElement); } return e; }; - return getShadowRootFocus(el); + return getShadowRootFocus(element); } - - // We’re making two passes with focused and focusedAgain sets of DOM elements, - // rather than just checking we get back to the first focused element, - // so we can cover more edge-cases involving issues with focus tree. - const focused = new Set(); - const focusedAgain = new Set(); - - // Initializing: some element is supposed to be automatically focused - const firstEl = getFocusedEl(); - focused.add(firstEl); - - // Setting a maximum number of keypresses to escape the loop, - // should we fall into a focus trap. - // Fixed amount of keypresses also helps assess consistency with keyboard navigation. - const maxKeypresses = content.document.getElementsByTagName("*").length * 2; - let keypresses = 1; - - while (focusedAgain.size < focused.size && keypresses <= maxKeypresses) { + // Helper function for getting the DOM element given an entry in the ordered list + function getElementFromOrderedArray(combinedSelectors) { + let [selector, shadowRootSelector] = combinedSelectors; + return content.document + .querySelector(selector) + .shadowRoot.querySelector(shadowRootSelector); + } + // Ensure the test starts in a valid state + firstElement.focus(); + info(`what is our navigator platform ${content.window.navigator.platform}`); + // Assert that we tab navigate correctly + for (let expectedSelector of expectedElementsInOrder) { + let expectedElement = getElementFromOrderedArray(expectedSelector); + // By default, MacOS will skip over certain text controls, such as links. + if ( + content.window.navigator.platform.toLowerCase().includes("mac") && + expectedElement.tagName === "A" + ) { + continue; + } + expectedElement = Cu.waiveXrays(expectedElement); + info(`expectedElement className ${expectedElement.className}`); + let actualElem = getFocusedElement(); + actualElem = Cu.waiveXrays(actualElem); + info(`actualElement className ${actualElem.className}`); + is( + actualElem, + expectedElement, + "Actual focused element should equal the expected focused element" + ); await tab(); - keypresses++; - let el = getFocusedEl(); - - // The following block is needed to get back to document - // after tabbing to browser chrome. - // This focus trap in browser chrome is a testing artifact we’re fixing below. - if (el.tagName === "BODY" && el !== firstEl) { - firstEl.focus(); - await new Promise(resolve => content.requestAnimationFrame(resolve)); - el = getFocusedEl(); - } - - if (focused.has(el)) { - focusedAgain.add(el); - } else { - focused.add(el); - } } - is( - focusedAgain.size, - focused.size, - "All focusable elements should be kept accessible with TAB key (no focus trap)." - ); + // The following block is needed to get back to document + // after tabbing to browser chrome. + // This focus trap in browser chrome is a testing artifact we’re fixing below. + if (getFocusedElement().tagName === "BODY") { + firstElement.focus(); + } + + // Assert that we tab navigate correctly after looping through chrome elements + for (let expectedSelector of expectedElementsInOrder) { + let expectedElement = getElementFromOrderedArray(expectedSelector); + // By default, MacOS will skip over certain text controls, such as links. + if ( + content.window.navigator.platform.toLowerCase().includes("mac") && + expectedElement.tagName === "A" + ) { + continue; + } + let actualElement = getFocusedElement(); + is( + actualElement, + expectedElement, + "Actual focused element should equal the expected focused element" + ); + await tab(); + } + // The following block is needed to get back to document + // after tabbing to browser chrome. + // This focus trap in browser chrome is a testing artifact we’re fixing below. + if (getFocusedElement().tagName === "BODY") { + lastElement.focus(); + } + + // Assert that we shift + tab navigate correctly starting from the last ordered element + for (let expectedSelector of expectedElementsInOrder.reverse()) { + let expectedElement = getElementFromOrderedArray(expectedSelector); + // By default, MacOS will skip over certain text controls, such as links. + if ( + content.window.navigator.platform.toLowerCase().includes("mac") && + expectedElement.tagName === "A" + ) { + continue; + } + let actualElement = getFocusedElement(); + is( + actualElement, + expectedElement, + "Actual focused element should equal the expected focused element" + ); + await shiftTab(); + } }); }); @@ -117,22 +184,22 @@ add_task(async function testTabToCreateButton() { let createButton = loginList.shadowRoot.querySelector( ".create-login-button" ); - let getFocusedEl = () => loginList.shadowRoot.activeElement; + let getFocusedElement = () => loginList.shadowRoot.activeElement; - is(getFocusedEl(), null, "login-list isn't focused"); + is(getFocusedElement(), null, "login-list isn't focused"); loginSort.focus(); await waitForAnimationFrame(); - is(getFocusedEl(), loginSort, "login sort is focused"); + is(getFocusedElement(), loginSort, "login sort is focused"); await tab(); - is(getFocusedEl(), loginListbox, "listbox is focused next"); + is(getFocusedElement(), loginListbox, "listbox is focused next"); await tab(); - is(getFocusedEl(), createButton, "create button is after"); + is(getFocusedElement(), createButton, "create button is after"); await tab(); - is(getFocusedEl(), null, "login-list isn't focused again"); + is(getFocusedElement(), null, "login-list isn't focused again"); }); }); @@ -156,18 +223,22 @@ add_task(async function testTabToEditButton() { let loginList = content.document.querySelector("login-list"); let loginItem = content.document.querySelector("login-item"); + let loginFilter = content.document.querySelector("login-filter"); let createButton = loginList.shadowRoot.querySelector( ".create-login-button" ); let editButton = loginItem.shadowRoot.querySelector(".edit-button"); let breachAlert = loginItem.shadowRoot.querySelector(".breach-alert"); - let getFocusedEl = () => { + let getFocusedElement = () => { if (content.document.activeElement == loginList) { return loginList.shadowRoot.activeElement; } if (content.document.activeElement == loginItem) { return loginItem.shadowRoot.activeElement; } + if (content.document.activeElement == loginFilter) { + return loginFilter.shadowRoot.activeElement; + } ok( false, "not expecting a different element to get focused in this test: " + @@ -198,11 +269,11 @@ add_task(async function testTabToEditButton() { createButton.focus(); await waitForAnimationFrame(); - is(getFocusedEl(), createButton, "create button is focused"); + is(getFocusedElement(), createButton, "create button is focused"); await tab(); await waitForAnimationFrame(); - is(getFocusedEl(), editButton, "edit button is focused"); + is(getFocusedElement(), editButton, "edit button is focused"); } } );