From 07581454e527de2f5b67e705b445551b25643979 Mon Sep 17 00:00:00 2001 From: Yura Zenevich Date: Wed, 28 Aug 2019 12:12:59 +0000 Subject: [PATCH] Bug 1564968 - adding keyboard audit type serverside support. r=nchevobbe Differential Revision: https://phabricator.services.mozilla.com/D43442 --HG-- extra : moz-landing-system : lando --- .../client/accessibility/reducers/audit.js | 9 +- .../server/actors/accessibility/accessible.js | 9 + .../actors/accessibility/audit/keyboard.js | 416 ++++++++++++++++++ .../actors/accessibility/audit/moz.build | 1 + devtools/server/tests/browser/browser.ini | 5 + .../browser_accessibility_keyboard_audit.js | 111 +++++ .../browser_accessibility_walker_audit.js | 5 + .../doc_accessibility_keyboard_audit.html | 33 ++ devtools/shared/constants.js | 13 + 9 files changed, 597 insertions(+), 5 deletions(-) create mode 100644 devtools/server/actors/accessibility/audit/keyboard.js create mode 100644 devtools/server/tests/browser/browser_accessibility_keyboard_audit.js create mode 100644 devtools/server/tests/browser/doc_accessibility_keyboard_audit.html diff --git a/devtools/client/accessibility/reducers/audit.js b/devtools/client/accessibility/reducers/audit.js index ea933052ea06..861af668fd85 100644 --- a/devtools/client/accessibility/reducers/audit.js +++ b/devtools/client/accessibility/reducers/audit.js @@ -62,11 +62,10 @@ function audit(state = getInitialState(), action) { [filter]: isToggledToActive, }; - if ( - isToggledToActive && - !filters[FILTERS.ALL] && - Object.values(AUDIT_TYPE).every(filterKey => filters[filterKey]) - ) { + const allAuditTypesActive = Object.values(AUDIT_TYPE) + .filter(filterKey => filters.hasOwnProperty(filterKey)) + .every(filterKey => filters[filterKey]); + if (isToggledToActive && !filters[FILTERS.ALL] && allAuditTypesActive) { filters[FILTERS.ALL] = true; } else if (!isToggledToActive && filters[FILTERS.ALL]) { filters[FILTERS.ALL] = false; diff --git a/devtools/server/actors/accessibility/accessible.js b/devtools/server/actors/accessibility/accessible.js index 37f6f8b141ba..763f34dce601 100644 --- a/devtools/server/actors/accessibility/accessible.js +++ b/devtools/server/actors/accessibility/accessible.js @@ -17,6 +17,12 @@ loader.lazyRequireGetter( "devtools/server/actors/accessibility/audit/contrast", true ); +loader.lazyRequireGetter( + this, + "auditKeyboard", + "devtools/server/actors/accessibility/audit/keyboard", + true +); loader.lazyRequireGetter( this, "auditTextLabel", @@ -486,6 +492,9 @@ const AccessibleActor = ActorClassWithSpec(accessibleSpec, { switch (type) { case AUDIT_TYPE.CONTRAST: return this._getContrastRatio(); + case AUDIT_TYPE.KEYBOARD: + // Determine if keyboard accessibility is lacking where it is necessary. + return auditKeyboard(this.rawAccessible); case AUDIT_TYPE.TEXT_LABEL: // Determine if text alternative is missing for an accessible where it // is necessary. diff --git a/devtools/server/actors/accessibility/audit/keyboard.js b/devtools/server/actors/accessibility/audit/keyboard.js new file mode 100644 index 000000000000..ddfa72f1bd1f --- /dev/null +++ b/devtools/server/actors/accessibility/audit/keyboard.js @@ -0,0 +1,416 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +"use strict"; + +const { Ci, Cu } = require("chrome"); +loader.lazyRequireGetter( + this, + "CssLogic", + "devtools/server/actors/inspector/css-logic", + true +); +loader.lazyRequireGetter( + this, + "getCSSStyleRules", + "devtools/shared/inspector/css-logic", + true +); +loader.lazyRequireGetter(this, "InspectorUtils", "InspectorUtils"); +loader.lazyRequireGetter( + this, + "nodeConstants", + "devtools/shared/dom-node-constants" +); + +const { + accessibility: { + AUDIT_TYPE: { KEYBOARD }, + ISSUE_TYPE: { + [KEYBOARD]: { + FOCUSABLE_NO_SEMANTICS, + FOCUSABLE_POSITIVE_TABINDEX, + INTERACTIVE_NO_ACTION, + INTERACTIVE_NOT_FOCUSABLE, + NO_FOCUS_VISIBLE, + }, + }, + SCORES: { FAIL, WARNING }, + }, +} = require("devtools/shared/constants"); + +// Specified by the author CSS rule type. +const STYLE_RULE = 1; + +/** + * Focus specific pseudo classes that the keyboard audit simulates to determine + * focus styling. + */ +const FOCUS_PSEUDO_CLASS = ":focus"; +const MOZ_FOCUSRING_PSEUDO_CLASS = ":-moz-focusring"; + +const INTERACTIVE_ROLES = new Set([ + Ci.nsIAccessibleRole.ROLE_BUTTONMENU, + Ci.nsIAccessibleRole.ROLE_CHECKBUTTON, + Ci.nsIAccessibleRole.ROLE_CHECK_MENU_ITEM, + Ci.nsIAccessibleRole.ROLE_CHECK_RICH_OPTION, + Ci.nsIAccessibleRole.ROLE_COMBOBOX, + Ci.nsIAccessibleRole.ROLE_COMBOBOX_OPTION, + Ci.nsIAccessibleRole.ROLE_EDITCOMBOBOX, + Ci.nsIAccessibleRole.ROLE_ENTRY, + Ci.nsIAccessibleRole.ROLE_LINK, + Ci.nsIAccessibleRole.ROLE_LISTBOX, + Ci.nsIAccessibleRole.ROLE_MENUITEM, + Ci.nsIAccessibleRole.ROLE_OPTION, + Ci.nsIAccessibleRole.ROLE_PAGETAB, + Ci.nsIAccessibleRole.ROLE_PASSWORD_TEXT, + Ci.nsIAccessibleRole.ROLE_PUSHBUTTON, + Ci.nsIAccessibleRole.ROLE_RADIOBUTTON, + Ci.nsIAccessibleRole.ROLE_RADIO_MENU_ITEM, + Ci.nsIAccessibleRole.ROLE_RICH_OPTION, + Ci.nsIAccessibleRole.ROLE_SLIDER, + Ci.nsIAccessibleRole.ROLE_SPINBUTTON, + Ci.nsIAccessibleRole.ROLE_SWITCH, + Ci.nsIAccessibleRole.ROLE_TOGGLE_BUTTON, +]); + +/** + * Determine if a node is dead or is not an element node. + * + * @param {DOMNode} node + * Node to be tested for validity. + * + * @returns {Boolean} + * True if the node is either dead or is not an element node. + */ +function isInvalidNode(node) { + return ( + !node || + Cu.isDeadWrapper(node) || + node.nodeType !== nodeConstants.ELEMENT_NODE || + !node.ownerGlobal + ); +} + +/** + * Determine if a current node has focus specific styling by applying a + * focus-related pseudo class (such as :focus or :-moz-focusring) to a focusable + * node. + * + * @param {DOMNode} focusableNode + * Node to apply focus-related pseudo class to. + * @param {DOMNode} currentNode + * Node to be checked for having focus specific styling. + * @param {String} pseudoClass + * A focus related pseudo-class to be simulated for style comparison. + * + * @returns {Boolean} + * True if the currentNode has focus specific styling. + */ +function hasStylesForFocusRelatedPseudoClass( + focusableNode, + currentNode, + pseudoClass +) { + const defaultRules = getCSSStyleRules(currentNode); + + InspectorUtils.addPseudoClassLock(focusableNode, pseudoClass); + + // Determine a set of properties that are specific to CSS rules that are only + // present when a focus related pseudo-class is locked in. + const tempRules = getCSSStyleRules(currentNode); + const properties = new Set(); + for (const rule of tempRules) { + if (rule.type !== STYLE_RULE) { + continue; + } + + if (!defaultRules.includes(rule)) { + for (let index = 0; index < rule.style.length; index++) { + properties.add(rule.style.item(index)); + } + } + } + + // If there are no focus specific CSS rules or properties, currentNode does + // node have any focus specific styling, we are done. + if (properties.size === 0) { + InspectorUtils.removePseudoClassLock(focusableNode, pseudoClass); + return false; + } + + // Determine values for properties that are focus specific. + const tempStyle = CssLogic.getComputedStyle(currentNode); + const focusStyle = {}; + for (const name of properties.values()) { + focusStyle[name] = tempStyle.getPropertyValue(name); + } + + InspectorUtils.removePseudoClassLock(focusableNode, pseudoClass); + + // If values for focus specific properties are different from default style + // values, assume we have focus spefic styles for the currentNode. + const defaultStyle = CssLogic.getComputedStyle(currentNode); + for (const name of properties.values()) { + if (defaultStyle.getPropertyValue(name) !== focusStyle[name]) { + return true; + } + } + + return false; +} + +/** + * Check if an element node (currentNode) has distinct focus styling. This + * function also takes into account a case when focus styling is applied to a + * descendant too. + * + * @param {DOMNode} focusableNode + * Node to apply focus-related pseudo class to. + * @param {DOMNode} currentNode + * Node to be checked for having focus specific styling. + * + * @returns {Boolean} + * True if the node or its descendant has distinct focus styling. + */ +function hasFocusStyling(focusableNode, currentNode) { + if (isInvalidNode(currentNode)) { + return false; + } + + // Check if an element node has distinct :-moz-focusring styling. + const hasStylesForMozFocusring = hasStylesForFocusRelatedPseudoClass( + focusableNode, + currentNode, + MOZ_FOCUSRING_PSEUDO_CLASS + ); + if (hasStylesForMozFocusring) { + return true; + } + + // Check if an element node has distinct :focus styling. + const hasStylesForFocus = hasStylesForFocusRelatedPseudoClass( + focusableNode, + currentNode, + FOCUS_PSEUDO_CLASS + ); + if (hasStylesForFocus) { + return true; + } + + // If no element specific focus styles where found, check if its element + // children have them. + for ( + let child = currentNode.firstElementChild; + child; + child = currentNode.nextnextElementSibling + ) { + if (hasFocusStyling(focusableNode, child)) { + return true; + } + } + + return false; +} + +/** + * A rule that determines if a focusable accessible object has appropriate focus + * styling. + * + * @param {nsIAccessible} accessible + * Accessible to be checked for being focusable and having focus + * styling. + * + * @return {null|Object} + * Null if accessible has keyboard focus styling, audit report object + * otherwise. + */ +function focusStyleRule(accessible) { + const { DOMNode } = accessible; + if (isInvalidNode(DOMNode)) { + return null; + } + + // Ignore non-focusable elements. + const state = {}; + accessible.getState(state, {}); + if (!(state.value & Ci.nsIAccessibleStates.STATE_FOCUSABLE)) { + return null; + } + + if (hasFocusStyling(DOMNode, DOMNode)) { + return null; + } + + // If no browser or author focus styling was found, check if the node is a + // widget that is themed by platform native theme. + if (InspectorUtils.isElementThemed(DOMNode)) { + return null; + } + + return { score: WARNING, issue: NO_FOCUS_VISIBLE }; +} + +/** + * A rule that determines if an interactive accessible has any associated + * accessible actions with it. If the element is interactive but and has no + * actions, assistive technology users will not be able to interact with it. + * + * @param {nsIAccessible} accessible + * Accessible to be checked for being interactive and having accessible + * actions. + * + * @return {null|Object} + * Null if accessible is not interactive or if it is and it has + * accessible action associated with it, audit report object otherwise. + */ +function interactiveRule(accessible) { + if (!INTERACTIVE_ROLES.has(accessible.role)) { + return null; + } + + if (accessible.actionCount > 0) { + return null; + } + + return { score: FAIL, issue: INTERACTIVE_NO_ACTION }; +} + +/** + * A rule that determines if an interactive accessible is also focusable when + * not disabled. + * + * @param {nsIAccessible} accessible + * Accessible to be checked for being interactive and being focusable + * when enabled. + * + * @return {null|Object} + * Null if accessible is not interactive or if it is and it is focusable + * when enabled, audit report object otherwise. + */ +function focusableRule(accessible) { + if (!INTERACTIVE_ROLES.has(accessible.role)) { + return null; + } + + const state = {}; + accessible.getState(state, {}); + // We only expect in interactive accessible object to be focusable if it is + // not disabled. + if (state.value & Ci.nsIAccessibleStates.STATE_UNAVAILABLE) { + return null; + } + + // State will be focusable even if the tabindex is negative. + if ( + state.value & Ci.nsIAccessibleStates.STATE_FOCUSABLE && + accessible.DOMNode.tabIndex > -1 + ) { + return null; + } + + return { score: FAIL, issue: INTERACTIVE_NOT_FOCUSABLE }; +} + +/** + * A rule that determines if a focusable accessible has an associated + * interactive role. + * + * @param {nsIAccessible} accessible + * Accessible to be checked for having an interactive role if it is + * focusable. + * + * @return {null|Object} + * Null if accessible is not interactive or if it is and it has an + * interactive role, audit report object otherwise. + */ +function semanticsRule(accessible) { + if (INTERACTIVE_ROLES.has(accessible.role)) { + return null; + } + + const state = {}; + accessible.getState(state, {}); + if (state.value & Ci.nsIAccessibleStates.STATE_FOCUSABLE) { + return { score: WARNING, issue: FOCUSABLE_NO_SEMANTICS }; + } + + return null; +} + +/** + * A rule that determines if an element associated with a focusable accessible + * has a positive tabindex. + * + * @param {nsIAccessible} accessible + * Accessible to be checked for having an element with positive tabindex + * attribute. + * + * @return {null|Object} + * Null if accessible is not focusable or if it is and its element's + * tabindex attribute is less than 1, audit report object otherwise. + */ +function tabIndexRule(accessible) { + const { DOMNode } = accessible; + if (isInvalidNode(DOMNode)) { + return null; + } + + const state = {}; + accessible.getState(state, {}); + if (!(state.value & Ci.nsIAccessibleStates.STATE_FOCUSABLE)) { + return null; + } + + if (DOMNode.tabIndex > 0) { + return { score: WARNING, issue: FOCUSABLE_POSITIVE_TABINDEX }; + } + + return null; +} + +function auditKeyboard(accessible) { + // Do not test anything on accessible objects for documents or frames. + if ( + accessible.role === Ci.nsIAccessibleRole.ROLE_DOCUMENT || + accessible.role === Ci.nsIAccessibleRole.ROLE_INTERNAL_FRAME + ) { + return null; + } + + // Check if interactive accessible can be used by the assistive + // technology. + let issue = interactiveRule(accessible); + if (issue) { + return issue; + } + + // Check if interactive accessible is also focusable when enabled. + issue = focusableRule(accessible); + if (issue) { + return issue; + } + + // Check if accessible object has an element with a positive tabindex. + issue = tabIndexRule(accessible); + if (issue) { + return issue; + } + + // Check if a focusable accessible has interactive semantics. + issue = semanticsRule(accessible); + if (issue) { + return issue; + } + + // Check if focusable accessible has associated focus styling. + issue = focusStyleRule(accessible); + if (issue) { + return issue; + } + + return issue; +} + +module.exports.auditKeyboard = auditKeyboard; diff --git a/devtools/server/actors/accessibility/audit/moz.build b/devtools/server/actors/accessibility/audit/moz.build index 4a834f29b257..0600b8a8e29f 100644 --- a/devtools/server/actors/accessibility/audit/moz.build +++ b/devtools/server/actors/accessibility/audit/moz.build @@ -4,6 +4,7 @@ DevToolsModules( 'contrast.js', + 'keyboard.js', 'text-label.js', ) diff --git a/devtools/server/tests/browser/browser.ini b/devtools/server/tests/browser/browser.ini index 9216389fd667..b8a753a7e085 100644 --- a/devtools/server/tests/browser/browser.ini +++ b/devtools/server/tests/browser/browser.ini @@ -12,6 +12,7 @@ support-files = application-manifest-warnings.html doc_accessibility_audit.html doc_accessibility_infobar.html + doc_accessibility_keyboard_audit.html doc_accessibility_text_label_audit_frame.html doc_accessibility_text_label_audit.html doc_accessibility.html @@ -49,6 +50,10 @@ support-files = [browser_accessibility_highlighter_infobar.js] skip-if = (os == 'win' && processor == 'aarch64') # bug 1533184 [browser_accessibility_infobar_show.js] +[browser_accessibility_keyboard_audit.js] +skip-if = + (os == 'win' && processor == 'aarch64') || # bug 1533184 + fission # Fails intermittently under Fission. [browser_accessibility_infobar_audit_text_label.js] [browser_accessibility_node.js] skip-if = (os == 'win' && processor == 'aarch64') # bug 1533184 diff --git a/devtools/server/tests/browser/browser_accessibility_keyboard_audit.js b/devtools/server/tests/browser/browser_accessibility_keyboard_audit.js new file mode 100644 index 000000000000..de024af7cea6 --- /dev/null +++ b/devtools/server/tests/browser/browser_accessibility_keyboard_audit.js @@ -0,0 +1,111 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +"use strict"; + +/** + * Checks functionality around text label audit for the AccessibleActor. + */ + +const { + accessibility: { + AUDIT_TYPE: { KEYBOARD }, + SCORES: { FAIL, WARNING }, + ISSUE_TYPE: { + [KEYBOARD]: { + FOCUSABLE_NO_SEMANTICS, + FOCUSABLE_POSITIVE_TABINDEX, + INTERACTIVE_NO_ACTION, + INTERACTIVE_NOT_FOCUSABLE, + NO_FOCUS_VISIBLE, + }, + }, + }, +} = require("devtools/shared/constants"); + +add_task(async function() { + const { target, walker, accessibility } = await initAccessibilityFrontForUrl( + `${MAIN_DOMAIN}doc_accessibility_keyboard_audit.html` + ); + + const a11yWalker = await accessibility.getWalker(); + await accessibility.enable(); + + const tests = [ + [ + "Focusable element (styled button) with no semantics.", + "#button-1", + { score: WARNING, issue: FOCUSABLE_NO_SEMANTICS }, + ], + ["Element (styled button) with no semantics.", "#button-2", null], + [ + "Container element for out of order focusable element.", + "#input-container", + null, + ], + [ + "Interactive element with focus out of order (-1).", + "#input-1", + { + score: FAIL, + issue: INTERACTIVE_NOT_FOCUSABLE, + }, + ], + [ + "Interactive element with focus out of order (-1) when disabled.", + "#input-2", + null, + ], + ["Interactive element when disabled.", "#input-3", null], + ["Focusable interactive element.", "#input-4", null], + [ + "Interactive accesible (link with no attributes) with no accessible actions.", + "#link-1", + { + score: FAIL, + issue: INTERACTIVE_NO_ACTION, + }, + ], + ["Interactive accessible (link with valid hred).", "#link-2", null], + ["Interactive accessible with no tabindex.", "#button-3", null], + [ + "Interactive accessible with -1 tabindex.", + "#button-4", + { + score: FAIL, + issue: INTERACTIVE_NOT_FOCUSABLE, + }, + ], + ["Interactive accessible with 0 tabindex.", "#button-5", null], + [ + "Interactive accessible with 1 tabindex.", + "#button-6", + { score: WARNING, issue: FOCUSABLE_POSITIVE_TABINDEX }, + ], + [ + "Focusable ARIA button with no focus styling.", + "#focusable-1", + { score: WARNING, issue: NO_FOCUS_VISIBLE }, + ], + ["Focusable ARIA button with focus styling.", "#focusable-2", null], + ["Focusable ARIA button with browser styling.", "#focusable-3", null], + ]; + + for (const [description, selector, expected] of tests) { + info(description); + const node = await walker.querySelector(walker.rootNode, selector); + const front = await a11yWalker.getAccessibleFor(node); + const audit = await front.audit({ types: [KEYBOARD] }); + Assert.deepEqual( + audit[KEYBOARD], + expected, + `Audit result for ${selector} is correct.` + ); + } + + await accessibility.disable(); + await waitForA11yShutdown(); + await target.destroy(); + gBrowser.removeCurrentTab(); +}); diff --git a/devtools/server/tests/browser/browser_accessibility_walker_audit.js b/devtools/server/tests/browser/browser_accessibility_walker_audit.js index 9969a014c647..5e47a701b5d8 100644 --- a/devtools/server/tests/browser/browser_accessibility_walker_audit.js +++ b/devtools/server/tests/browser/browser_accessibility_walker_audit.js @@ -21,6 +21,7 @@ add_task(async function() { childCount: 2, checks: { [AUDIT_TYPE.CONTRAST]: null, + [AUDIT_TYPE.KEYBOARD]: null, [AUDIT_TYPE.TEXT_LABEL]: { score: SCORES.FAIL, issue: ISSUE_TYPE.DOCUMENT_NO_TITLE, @@ -35,6 +36,7 @@ add_task(async function() { childCount: 1, checks: { [AUDIT_TYPE.CONTRAST]: null, + [AUDIT_TYPE.KEYBOARD]: null, [AUDIT_TYPE.TEXT_LABEL]: null, }, }, @@ -52,6 +54,7 @@ add_task(async function() { isLargeText: false, score: "fail", }, + [AUDIT_TYPE.KEYBOARD]: null, [AUDIT_TYPE.TEXT_LABEL]: null, }, }, @@ -61,6 +64,7 @@ add_task(async function() { childCount: 1, checks: { [AUDIT_TYPE.CONTRAST]: null, + [AUDIT_TYPE.KEYBOARD]: null, [AUDIT_TYPE.TEXT_LABEL]: null, }, }, @@ -76,6 +80,7 @@ add_task(async function() { isLargeText: false, score: "fail", }, + [AUDIT_TYPE.KEYBOARD]: null, [AUDIT_TYPE.TEXT_LABEL]: null, }, }, diff --git a/devtools/server/tests/browser/doc_accessibility_keyboard_audit.html b/devtools/server/tests/browser/doc_accessibility_keyboard_audit.html new file mode 100644 index 000000000000..5b1a7cfa4779 --- /dev/null +++ b/devtools/server/tests/browser/doc_accessibility_keyboard_audit.html @@ -0,0 +1,33 @@ + + + + + + + +
I should really be a button
+
I should really be a button
+
+ + + + Though a link, I'm not interactive. + I'm a proper link. + + + + +
Focusable with no focus style.
+
Focusable with focus style.
+
Focusable with focus style.
+ + diff --git a/devtools/shared/constants.js b/devtools/shared/constants.js index 59eca5d72453..b141d75acff9 100644 --- a/devtools/shared/constants.js +++ b/devtools/shared/constants.js @@ -13,11 +13,24 @@ // List of audit types. const AUDIT_TYPE = { CONTRAST: "CONTRAST", + KEYBOARD: "KEYBOARD", TEXT_LABEL: "TEXT_LABEL", }; // Types of issues grouped by audit types. const ISSUE_TYPE = { + [AUDIT_TYPE.KEYBOARD]: { + // Focusable accessible objects have no semantics. + FOCUSABLE_NO_SEMANTICS: "FOCUSABLE_NO_SEMANTICS", + // Tab index greater than 0 is provided. + FOCUSABLE_POSITIVE_TABINDEX: "FOCUSABLE_POSITIVE_TABINDEX", + // Interactive accesible objects do not have an associated action. + INTERACTIVE_NO_ACTION: "INTERACTIVE_NO_ACTION", + // Interative accessible objcets are not focusable. + INTERACTIVE_NOT_FOCUSABLE: "INTERACTIVE_NOT_FOCUSABLE", + // Focusable accessible objects have no focus styling. + NO_FOCUS_VISIBLE: "NO_FOCUS_VISIBLE", + }, [AUDIT_TYPE.TEXT_LABEL]: { // name is provided via "alt" attribute. AREA_NO_NAME_FROM_ALT: "AREA_NO_NAME_FROM_ALT",