From 4faa8c2b596bc521984a62bbc58e10c8448e508d Mon Sep 17 00:00:00 2001 From: Yura Zenevich Date: Tue, 1 Aug 2017 12:55:21 -0400 Subject: [PATCH] Bug 1377276 - add modal dialog semantics and better accessibility for onboarding overlay dialog. r=mossop, gasolin, rexboy MozReview-Commit-ID: 9xyhn7jLJqD --- .../onboarding/content/onboarding.js | 110 ++++++++++++++++-- .../browser_onboarding_accessibility.js | 27 +++++ .../browser/browser_onboarding_keyboard.js | 102 +++++++++++++--- .../onboarding/test/browser/head.js | 73 +++++++++--- 4 files changed, 274 insertions(+), 38 deletions(-) diff --git a/browser/extensions/onboarding/content/onboarding.js b/browser/extensions/onboarding/content/onboarding.js index 8405ed411b13..00381e0341a6 100644 --- a/browser/extensions/onboarding/content/onboarding.js +++ b/browser/extensions/onboarding/content/onboarding.js @@ -20,6 +20,7 @@ const BRAND_SHORT_NAME = Services.strings .createBundle("chrome://branding/locale/brand.properties") .GetStringFromName("brandShortName"); const PROMPT_COUNT_PREF = "browser.onboarding.notification.prompt-count"; +const ONBOARDING_DIALOG_ID = "onboarding-overlay-dialog"; /** * Add any number of tours, key is the tourId, value should follow the format below @@ -368,6 +369,7 @@ class Onboarding { let { body } = this._window.document; this._overlayIcon = this._renderOverlayButton(); this._overlayIcon.addEventListener("click", this); + this._overlayIcon.addEventListener("keypress", this); body.insertBefore(this._overlayIcon, body.firstChild); this._overlay = this._renderOverlay(); @@ -436,6 +438,15 @@ class Onboarding { } } + /** + * Find a tour that should be selected. It is either a first tour that was not + * yet complete or the first one in the tab list. + */ + get selectedTour() { + return this._tours.find(tour => !this.isTourCompleted(tour.id)) || + this._tours[0]; + } + handleClick(target) { let { id, classList } = target; // Only containers receive pointer events in onboarding tour tab list, @@ -452,8 +463,7 @@ class Onboarding { // Let's toggle the overlay. case "onboarding-overlay": this.toggleOverlay(); - let selectedTour = this._tours.find(tour => !this.isTourCompleted(tour.id)) || this._tours[0]; - this.gotoPage(selectedTour.id); + this.gotoPage(this.selectedTour.id); break; case "onboarding-notification-close-btn": this.hideNotification(); @@ -477,8 +487,46 @@ class Onboarding { } } + /** + * Wrap keyboard focus within the dialog and focus on first element after last + * when moving forward or last element after first when moving backwards. Do + * nothing if focus is moving in the middle of the list of dialog's focusable + * elements. + * + * @param {DOMNode} current currently focused element + * @param {Boolean} back direction + * @return {DOMNode} newly focused element if any + */ + wrapMoveFocus(current, back) { + let elms = [...this._dialog.querySelectorAll( + `button, input[type="checkbox"], input[type="email"], [tabindex="0"]`)]; + let next; + if (back) { + if (elms.indexOf(current) === 0) { + next = elms[elms.length - 1]; + next.focus(); + } + } else if (elms.indexOf(current) === elms.length - 1) { + next = elms[0]; + next.focus(); + } + return next; + } + handleKeypress(event) { - let { target, key } = event; + let { target, key, shiftKey } = event; + + if (target === this._overlayIcon) { + if ([" ", "Enter"].includes(key)) { + // Remember that the dialog was opened with a keyboard. + this._overlayIcon.dataset.keyboardFocus = true; + this.handleClick(target); + event.preventDefault(); + } + + return; + } + // Current focused item can be tab container if previous navigation was done // via mouse. if (target.classList.contains("onboarding-tour-item-container")) { @@ -515,6 +563,16 @@ class Onboarding { } event.preventDefault(); break; + case "Escape": + this.toggleOverlay(); + break; + case "Tab": + let next = this.wrapMoveFocus(target, shiftKey); + // If focus was wrapped, prevent Tab key default action. + if (next) { + event.preventDefault(); + } + break; default: break; } @@ -564,6 +622,7 @@ class Onboarding { this.hideNotification(); this._overlay.classList.toggle("onboarding-opened"); + this.toggleModal(this._overlay.classList.contains("onboarding-opened")); let hiddenCheckbox = this._window.document.getElementById("onboarding-tour-hidden-checkbox"); if (hiddenCheckbox.checked) { @@ -571,6 +630,41 @@ class Onboarding { } } + /** + * Set modal dialog state and properties for accessibility purposes. + * @param {Boolean} opened whether the dialog is opened or closed. + */ + toggleModal(opened) { + let { document: doc } = this._window; + if (opened) { + // Set aria-hidden to true for the rest of the document. + [...doc.body.children].forEach( + child => child.id !== "onboarding-overlay" && + child.setAttribute("aria-hidden", true)); + // When dialog is opened with the keyboard, focus on the selected or + // first tour item. + if (this._overlayIcon.dataset.keyboardFocus) { + doc.getElementById(this.selectedTour.id).focus(); + } else { + // When dialog is opened with mouse, focus on the dialog itself to avoid + // visible keyboard focus styling. + this._dialog.focus(); + } + } else { + // Remove all set aria-hidden attributes. + [...doc.body.children].forEach( + child => child.removeAttribute("aria-hidden")); + // If dialog was opened with a keyboard, set the focus back on the overlay + // button. + if (this._overlayIcon.dataset.keyboardFocus) { + delete this._overlayIcon.dataset.keyboardFocus; + this._overlayIcon.focus(); + } else { + this._window.document.activeElement.blur(); + } + } + } + gotoPage(tourId) { let targetPageId = `${tourId}-page`; for (let page of this._tourPages) { @@ -868,7 +962,7 @@ class Onboarding { // We use `innerHTML` for more friendly reading. // The security should be fine because this is not from an external input. div.innerHTML = ` -
+
`; + this._dialog = div.querySelector(`[role="dialog"]`); + this._dialog.id = ONBOARDING_DIALOG_ID; + div.querySelector("label[for='onboarding-tour-hidden-checkbox']").textContent = this._bundle.GetStringFromName("onboarding.hidden-checkbox-label-text"); div.querySelector("#onboarding-header").textContent = @@ -898,7 +995,7 @@ class Onboarding { button.setAttribute("aria-label", tooltip); button.id = "onboarding-overlay-button"; button.setAttribute("aria-haspopup", true); - button.setAttribute("aria-controls", "onboarding-overlay-dialog"); + button.setAttribute("aria-controls", `${ONBOARDING_DIALOG_ID}`); let img = this._window.document.createElement("img"); img.id = "onboarding-overlay-button-icon"; img.setAttribute("role", "presentation"); @@ -959,11 +1056,10 @@ class Onboarding { this.markTourCompletionState(tour.id); } - let dialog = this._window.document.getElementById("onboarding-overlay-dialog"); let ul = this._window.document.getElementById("onboarding-tour-list"); ul.appendChild(itemsFrag); let footer = this._window.document.getElementById("onboarding-footer"); - dialog.insertBefore(pagesFrag, footer); + this._dialog.insertBefore(pagesFrag, footer); } _loadCSS() { diff --git a/browser/extensions/onboarding/test/browser/browser_onboarding_accessibility.js b/browser/extensions/onboarding/test/browser/browser_onboarding_accessibility.js index b9539616b5c9..8711b0410a5e 100644 --- a/browser/extensions/onboarding/test/browser/browser_onboarding_accessibility.js +++ b/browser/extensions/onboarding/test/browser/browser_onboarding_accessibility.js @@ -63,3 +63,30 @@ add_task(async function test_onboarding_notification_bar() { await BrowserTestUtils.removeTab(tab); }); + +add_task(async function test_onboarding_overlay_dialog() { + resetOnboardingDefaultState(); + + info("Wait for onboarding overlay loaded"); + let tab = await openTab(ABOUT_HOME_URL); + let browser = tab.linkedBrowser; + await promiseOnboardingOverlayLoaded(browser); + + info("Test accessibility and semantics of the dialog overlay"); + await assertModalDialog(browser, { visible: false }); + + info("Click on overlay button and check modal dialog state"); + await BrowserTestUtils.synthesizeMouseAtCenter("#onboarding-overlay-button", + {}, browser); + await promiseOnboardingOverlayOpened(browser); + await assertModalDialog(browser, + { visible: true, focusedId: "onboarding-overlay-dialog" }); + + info("Close the dialog and check modal dialog state"); + await BrowserTestUtils.synthesizeMouseAtCenter("#onboarding-overlay-close-btn", + {}, browser); + await promiseOnboardingOverlayClosed(browser); + await assertModalDialog(browser, { visible: false }); + + await BrowserTestUtils.removeTab(tab); +}); diff --git a/browser/extensions/onboarding/test/browser/browser_onboarding_keyboard.js b/browser/extensions/onboarding/test/browser/browser_onboarding_keyboard.js index def353ca297e..2e65cb59795a 100644 --- a/browser/extensions/onboarding/test/browser/browser_onboarding_keyboard.js +++ b/browser/extensions/onboarding/test/browser/browser_onboarding_keyboard.js @@ -4,19 +4,29 @@ "use strict"; -function assertTourList(browser, args) { - return ContentTask.spawn(browser, args, ({ tourId, focusedId }) => { - let doc = content.document; - let items = [...doc.querySelectorAll(".onboarding-tour-item")]; - items.forEach(item => is(item.getAttribute("aria-selected"), - item.id === tourId ? "true" : "false", - "Active item should have aria-selected set to true and inactive to false")); - let focused = doc.getElementById(focusedId); - is(focused, doc.activeElement, `Focus should be set on ${focusedId}`); +function assertOverlayState(browser, args) { + return ContentTask.spawn(browser, args, ({ tourId, focusedId, visible }) => { + let { document: doc, window} = content; + if (tourId) { + let items = [...doc.querySelectorAll(".onboarding-tour-item")]; + items.forEach(item => is(item.getAttribute("aria-selected"), + item.id === tourId ? "true" : "false", + "Active item should have aria-selected set to true and inactive to false")); + } + if (focusedId) { + let focused = doc.getElementById(focusedId); + is(focused, doc.activeElement, `Focus should be set on ${focusedId}`); + } + if (visible !== undefined) { + let overlay = doc.getElementById("onboarding-overlay"); + is(window.getComputedStyle(overlay).getPropertyValue("display"), + visible ? "block" : "none", + `Onboarding overlay should be ${visible ? "visible" : "invisible"}`); + } }); } -const TEST_DATA = [ +const TOUR_LIST_TEST_DATA = [ { key: "VK_DOWN", expected: { tourId: TOUR_IDs[1], focusedId: TOUR_IDs[1] }}, { key: "VK_DOWN", expected: { tourId: TOUR_IDs[2], focusedId: TOUR_IDs[2] }}, { key: "VK_DOWN", expected: { tourId: TOUR_IDs[3], focusedId: TOUR_IDs[3] }}, @@ -32,6 +42,21 @@ const TEST_DATA = [ { key: " ", expected: { tourId: TOUR_IDs[2], focusedId: TOUR_IDs[2] }} ]; +const BUTTONS_TEST_DATA = [ + { key: " ", expected: { focusedId: TOUR_IDs[0], visible: true }}, + { key: "VK_ESCAPE", expected: { focusedId: "onboarding-overlay-button", visible: false }}, + { key: "VK_RETURN", expected: { focusedId: TOUR_IDs[1], visible: true }}, + { key: "VK_TAB", options: { shiftKey: true }, expected: { focusedId: TOUR_IDs[0], visible: true }}, + { key: "VK_TAB", options: { shiftKey: true }, expected: { focusedId: "onboarding-overlay-close-btn", visible: true }}, + { key: " ", expected: { focusedId: "onboarding-overlay-button", visible: false }}, + { key: "VK_RETURN", expected: { focusedId: TOUR_IDs[1], visible: true }}, + { key: "VK_TAB", options: { shiftKey: true }, expected: { focusedId: TOUR_IDs[0], visible: true }}, + { key: "VK_TAB", options: { shiftKey: true }, expected: { focusedId: "onboarding-overlay-close-btn", visible: true }}, + { key: "VK_TAB", expected: { focusedId: TOUR_IDs[0], visible: true }}, + { key: "VK_TAB", options: { shiftKey: true }, expected: { focusedId: "onboarding-overlay-close-btn", visible: true }}, + { key: "VK_RETURN", expected: { focusedId: "onboarding-overlay-button", visible: false }} +]; + add_task(async function test_tour_list_keyboard_navigation() { resetOnboardingDefaultState(); @@ -48,14 +73,65 @@ add_task(async function test_tour_list_keyboard_navigation() { info("Set initial focus on the currently active tab"); await ContentTask.spawn(tab.linkedBrowser, {}, () => content.document.querySelector(".onboarding-active").focus()); - await assertTourList(tab.linkedBrowser, + await assertOverlayState(tab.linkedBrowser, { tourId: TOUR_IDs[0], focusedId: TOUR_IDs[0] }); - for (let { key, options = {}, expected } of TEST_DATA) { + for (let { key, options = {}, expected } of TOUR_LIST_TEST_DATA) { info(`Pressing ${key} to select ${expected.tourId} and have focus on ${expected.focusedId}`); await BrowserTestUtils.synthesizeKey(key, options, tab.linkedBrowser); - await assertTourList(tab.linkedBrowser, expected); + await assertOverlayState(tab.linkedBrowser, expected); } await BrowserTestUtils.removeTab(tab); }); + +add_task(async function test_buttons_keyboard_navigation() { + resetOnboardingDefaultState(); + + info("Wait for onboarding overlay loaded"); + let tab = await openTab(ABOUT_HOME_URL); + await promiseOnboardingOverlayLoaded(tab.linkedBrowser); + + info("Set keyboard focus on the onboarding overlay button"); + await ContentTask.spawn(tab.linkedBrowser, {}, () => + content.document.getElementById("onboarding-overlay-button").focus()); + await assertOverlayState(tab.linkedBrowser, + { focusedId: "onboarding-overlay-button", visible: false }); + + for (let { key, options = {}, expected } of BUTTONS_TEST_DATA) { + info(`Pressing ${key} to have ${expected.visible ? "visible" : "invisible"} overlay and have focus on ${expected.focusedId}`); + await BrowserTestUtils.synthesizeKey(key, options, tab.linkedBrowser); + await assertOverlayState(tab.linkedBrowser, expected); + } + + await BrowserTestUtils.removeTab(tab); +}); + +add_task(async function test_overlay_dialog_keyboard_navigation() { + resetOnboardingDefaultState(); + + info("Wait for onboarding overlay loaded"); + let tab = await openTab(ABOUT_HOME_URL); + let browser = tab.linkedBrowser; + await promiseOnboardingOverlayLoaded(browser); + + info("Test accessibility and semantics of the dialog overlay"); + await assertModalDialog(browser, { visible: false }); + + info("Set keyboard focus on the onboarding overlay button"); + await ContentTask.spawn(browser, {}, () => + content.document.getElementById("onboarding-overlay-button").focus()); + info("Open dialog with keyboard and check the dialog state"); + await BrowserTestUtils.synthesizeKey(" ", {}, browser); + await promiseOnboardingOverlayOpened(browser); + await assertModalDialog(browser, + { visible: true, keyboardFocus: true, focusedId: TOUR_IDs[0] }); + + info("Close the dialog and check modal dialog state"); + await BrowserTestUtils.synthesizeKey("VK_ESCAPE", {}, browser); + await promiseOnboardingOverlayClosed(browser); + await assertModalDialog(browser, + { visible: false, keyboardFocus: true, focusedId: "onboarding-overlay-button" }); + + await BrowserTestUtils.removeTab(tab); +}); diff --git a/browser/extensions/onboarding/test/browser/head.js b/browser/extensions/onboarding/test/browser/head.js index b3704feafa55..87b36a46f9e4 100644 --- a/browser/extensions/onboarding/test/browser/head.js +++ b/browser/extensions/onboarding/test/browser/head.js @@ -87,21 +87,22 @@ function promiseOnboardingOverlayLoaded(browser) { } function promiseOnboardingOverlayOpened(browser) { - let condition = () => { - return ContentTask.spawn(browser, {}, function() { - return new Promise(resolve => { - let overlay = content.document.querySelector("#onboarding-overlay"); - if (overlay.classList.contains("onboarding-opened")) { - resolve(true); - return; - } - resolve(false); - }); - }) - }; - return BrowserTestUtils.waitForCondition( - condition, - "Should open onboarding overlay", + return BrowserTestUtils.waitForCondition(() => + ContentTask.spawn(browser, {}, () => + content.document.querySelector("#onboarding-overlay").classList.contains( + "onboarding-opened")), + "Should close onboarding overlay", + 100, + 30 + ); +} + +function promiseOnboardingOverlayClosed(browser) { + return BrowserTestUtils.waitForCondition(() => + ContentTask.spawn(browser, {}, () => + !content.document.querySelector("#onboarding-overlay").classList.contains( + "onboarding-opened")), + "Should close onboarding overlay", 100, 30 ); @@ -209,9 +210,17 @@ function assertOverlaySemantics(browser) { return ContentTask.spawn(browser, {}, function() { let doc = content.document; + info("Checking dialog"); + let dialog = doc.getElementById("onboarding-overlay-dialog"); + is(dialog.getAttribute("role"), "dialog", + "Dialog should have a dialog role attribute set"); + is(dialog.tabIndex, "-1", "Dialog should be focusable but not in tab order"); + is(dialog.getAttribute("aria-labelledby"), "onboarding-header", + "Dialog should be labaled by its header"); + info("Checking the tablist container"); is(doc.getElementById("onboarding-tour-list").getAttribute("role"), "tablist", - "Tour list should have a tablist role argument set"); + "Tour list should have a tablist role attribute set"); info("Checking each tour item that represents the tab"); let items = [...doc.querySelectorAll(".onboarding-tour-item")]; @@ -222,15 +231,43 @@ function assertOverlaySemantics(browser) { item.classList.contains("onboarding-active") ? "true" : "false", "Active item should have aria-selected set to true and inactive to false"); is(item.tabIndex, "0", "Item tab index must be set for keyboard accessibility"); - is(item.getAttribute("role"), "tab", "Item should have a tab role argument set"); + is(item.getAttribute("role"), "tab", "Item should have a tab role attribute set"); let tourPanelId = `${item.id}-page`; is(item.getAttribute("aria-controls"), tourPanelId, "Item should have aria-controls attribute point to its tabpanel"); let panel = doc.getElementById(tourPanelId); is(panel.getAttribute("role"), "tabpanel", - "Tour panel should have a tabpanel role argument set"); + "Tour panel should have a tabpanel role attribute set"); is(panel.getAttribute("aria-labelledby"), item.id, "Tour panel should have aria-labelledby attribute point to its tab"); }); }); } + +function assertModalDialog(browser, args) { + return ContentTask.spawn(browser, args, ({ keyboardFocus, visible, focusedId }) => { + let doc = content.document; + let overlayButton = doc.getElementById("onboarding-overlay-button"); + if (visible) { + [...doc.body.children].forEach(child => + child.id !== "onboarding-overlay" && + is(child.getAttribute("aria-hidden"), "true", + "Content should not be visible to screen reader")); + is(focusedId ? doc.getElementById(focusedId) : doc.body, + doc.activeElement, `Focus should be on ${focusedId || "body"}`); + is(keyboardFocus ? "true" : undefined, + overlayButton.dataset.keyboardFocus, + "Overlay button focus state is saved correctly"); + } else { + [...doc.body.children].forEach( + child => ok(!child.hasAttribute("aria-hidden"), + "Content should be visible to screen reader")); + if (keyboardFocus) { + is(overlayButton, doc.activeElement, + "Focus should be set on overlay button"); + } + ok(!overlayButton.dataset.keyboardFocus, + "Overlay button focus state should be cleared"); + } + }); +}