From 1fd4b004b2543727d4a3775f355934327765baa1 Mon Sep 17 00:00:00 2001 From: Gabe <41127686+Zidious@users.noreply.github.com> Date: Wed, 24 Nov 2021 13:55:35 +0000 Subject: [PATCH 1/4] fix(aria-allowed-role): landmark roles banner on header and contentinfo on footer to only report on top-level rule (#3142) Co-authored-by: Wilco Fiers Co-authored-by: Wilco Fiers --- axe.d.ts | 7 +- doc/frame-messenger.md | 2 +- doc/run-partial.md | 16 +-- lib/checks/aria/deprecatedrole-evaluate.js | 2 +- .../aria/get-element-unallowed-roles.js | 71 ++++----- .../aria/is-aria-role-allowed-on-element.js | 10 +- lib/core/base/audit.js | 11 +- lib/core/utils/send-command-to-frame.js | 5 +- lib/rules/aria-roles.json | 8 +- test/checks/aria/aria-allowed-role.js | 6 +- test/checks/aria/deprecatedrole.js | 24 ++-- .../aria/get-element-unallowed-roles.js | 135 ++++++++++++++++-- .../aria/is-aria-role-allowed-on-element.js | 4 +- test/core/base/audit.js | 18 +-- test/core/utils/get-frame-contexts.js | 8 +- test/core/utils/send-command-to-frame.js | 16 ++- .../aria-allowed-role/aria-allowed-role.html | 20 ++- .../aria-allowed-role/aria-allowed-role.json | 2 + .../aria-required-children.html | 2 +- 19 files changed, 236 insertions(+), 131 deletions(-) diff --git a/axe.d.ts b/axe.d.ts index bde2cb393f..675d9bd595 100644 --- a/axe.d.ts +++ b/axe.d.ts @@ -264,13 +264,16 @@ declare namespace axe { results: PartialRuleResult[]; environmentData?: EnvironmentData; } - type PartialResults = Array + type PartialResults = Array; interface FrameContext { frameSelector: CrossTreeSelector; frameContext: ContextObject; } interface Utils { - getFrameContexts: (context?: ElementContext, options?: RunOptions) => FrameContext[]; + getFrameContexts: ( + context?: ElementContext, + options?: RunOptions + ) => FrameContext[]; shadowSelect: (selector: CrossTreeSelector) => Element | null; } interface EnvironmentData { diff --git a/doc/frame-messenger.md b/doc/frame-messenger.md index 3ac8e3545d..53dd3a9b2b 100644 --- a/doc/frame-messenger.md +++ b/doc/frame-messenger.md @@ -110,7 +110,7 @@ The `message` passed to responder may be an `Error`. If axe-core passes an `Erro When axe-core tests frames, it first sends a ping to that frame, to check that the frame has a compatible version of axe-core in it that can respond to the message. If it gets no response, that frame will be skipped in the test. Axe-core does this to avoid a situation where it waits the full frame timeout, just to find out the frame didn't have axe-core in it in the first place. -In situations where communication between frames can be slow, it may be necessary to increase the ping timeout. This can be done with the `pingWaitTime` option. By default, this is 500ms. This can be configured in the following way: +In situations where communication between frames can be slow, it may be necessary to increase the ping timeout. This can be done with the `pingWaitTime` option. By default, this is 500ms. This can be configured in the following way: ```js const results = await axe.run(context, { pingWaitTime: 1000 })); diff --git a/doc/run-partial.md b/doc/run-partial.md index e0b09838df..ae7114d8ea 100644 --- a/doc/run-partial.md +++ b/doc/run-partial.md @@ -7,9 +7,7 @@ To use these methods, call `axe.runPartial()` in the top window, and in all nest This results in code that looks something like the following. The `context` and `options` arguments used are the same as would be passed to `axe.run`. See [API.md](api.md) for details. ```js -const partialResults = await Promise.all( - runPartialRecursive(context, options) -) +const partialResults = await Promise.all(runPartialRecursive(context, options)); const axeResults = await axe.finishRun(partialResults, options); ``` @@ -25,10 +23,10 @@ function runPartialRecursive(context, options = {}, win = window) { // Find all frames in context, and determine what context object to use in that frame const frameContexts = axe.utils.getFrameContexts(context, options); // Run the current context, in the current window. - const promiseResults = [ axe.runPartial(context, options) ]; + const promiseResults = [axe.runPartial(context, options)]; // Loop over all frames in context - frameContexts.forEach(({ frameSelector, frameContext }) => { + frameContexts.forEach(({ frameSelector, frameContext }) => { // Find the window of the frame const frame = axe.utils.shadowSelect(frameSelector); const frameWin = frame.contentWindow; @@ -37,7 +35,7 @@ function runPartialRecursive(context, options = {}, win = window) { promiseResults.push(...frameResults); }); return promiseResults; -}; +} ``` **important**: The order in which these methods are called matters for performance. Internally, axe-core constructs a flattened tree when `axe.utils.getFrameContexts` is called. This is fairly slow, and so should not happen more than once per frame. When `axe.runPartial` is called, that tree will be used if it still exists. Since this tree can get out of sync with the actual DOM, it is important to call `axe.runPartial` immediately after `axe.utils.getFrameContexts`. @@ -55,7 +53,7 @@ The `axe.finishRun` method does two things: It calls the `after` methods of chec // - frame_1a // - frame_2 // The partial results are passed in the following order: -axe.finishRun([ top, frame_1, frame_1a, frame_2 ]) +axe.finishRun([top, frame_1, frame_1a, frame_2]); ``` If for some reason `axe.runPartial` fails to run, the integration API **must** include `null` in the data in place of the results object, so that axe-core knows to skip over it. If a frame fails to run, results from any descending frames **must be omitted**. To illustrate this, consider the following: @@ -68,9 +66,7 @@ If for some reason `axe.runPartial` fails to run, the integration API **must** i // - frame_2 // If axe.runPartial throws an error, the results must be passed to finishRun like this: -axe.finishRun([ - top, null, /* nothing for frame 1a, */ frame_2 -]) +axe.finishRun([top, null, /* nothing for frame 1a, */ frame_2]); ``` **important**: Since `axe.finishRun` may have access to cross-origin information, it should only be called in an environment that is known not to have third-party scripts. When using a browser driver, this can for example by done in a blank page. diff --git a/lib/checks/aria/deprecatedrole-evaluate.js b/lib/checks/aria/deprecatedrole-evaluate.js index 149d501556..c2349df4c5 100644 --- a/lib/checks/aria/deprecatedrole-evaluate.js +++ b/lib/checks/aria/deprecatedrole-evaluate.js @@ -15,7 +15,7 @@ export default function deprecatedroleEvaluate(node, options, virtualNode) { if (!roleDefinition?.deprecated) { return false; } - + this.data(role); return true; } diff --git a/lib/commons/aria/get-element-unallowed-roles.js b/lib/commons/aria/get-element-unallowed-roles.js index 5b285857db..cbf7ede0ff 100644 --- a/lib/commons/aria/get-element-unallowed-roles.js +++ b/lib/commons/aria/get-element-unallowed-roles.js @@ -2,12 +2,7 @@ import isValidRole from './is-valid-role'; import getImplicitRole from './implicit-role'; import getRoleType from './get-role-type'; import isAriaRoleAllowedOnElement from './is-aria-role-allowed-on-element'; -import { - tokenList, - isHtmlElement, - matchesSelector, - getNodeFromTree -} from '../../core/utils'; +import { tokenList, isHtmlElement, getNodeFromTree } from '../../core/utils'; import AbstractVirtuaNode from '../../core/base/virtual-node/abstract-virtual-node'; // dpub roles which are subclassing roles that are implicit on some native @@ -22,6 +17,11 @@ const dpubRoles = [ 'doc-noteref' ]; +const landmarkRoles = { + header: 'banner', + footer: 'contentinfo' +}; + /** * Returns all roles applicable to element in a list * @@ -33,7 +33,6 @@ const dpubRoles = [ function getRoleSegments(vNode) { let roles = []; - if (!vNode) { return roles; } @@ -44,9 +43,7 @@ function getRoleSegments(vNode) { } // filter invalid roles - roles = roles.filter(role => isValidRole(role)); - - return roles; + return roles.filter(role => isValidRole(role)); } /** @@ -59,50 +56,32 @@ function getRoleSegments(vNode) { function getElementUnallowedRoles(node, allowImplicit = true) { const vNode = node instanceof AbstractVirtuaNode ? node : getNodeFromTree(node); - const { nodeName } = vNode.props; - // by pass custom elements if (!isHtmlElement(vNode)) { return []; } + // allow landmark roles to use their implicit role inside another landmark + // @see https://github.com/dequelabs/axe-core/pull/3142 + const { nodeName } = vNode.props; + const implicitRole = getImplicitRole(vNode) || landmarkRoles[nodeName]; const roleSegments = getRoleSegments(vNode); - const implicitRole = getImplicitRole(vNode); - - // stores all roles that are not allowed for a specific element most often an element only has one explicit role - const unallowedRoles = roleSegments.filter(role => { - // if role and implicit role are same, when allowImplicit: true - // ignore as it is a redundant role - if (allowImplicit && role === implicitRole) { - return false; - } - - // if role is a dpub role make sure it's used on an element with a valid - // implicit role fallback - if (allowImplicit && dpubRoles.includes(role)) { - const roleType = getRoleType(role); - if (implicitRole !== roleType) { - return true; - } - } - - // Edge case: - // setting implicit role row on tr element is allowed when child of table[role='grid'] - if ( - !allowImplicit && - !( - role === 'row' && - nodeName === 'tr' && - matchesSelector(vNode, 'table[role="grid"] > tr') - ) - ) { - return true; - } - // check if role is allowed on element - return !isAriaRoleAllowedOnElement(vNode, role); + return roleSegments.filter(role => { + return !roleIsAllowed(role, vNode, allowImplicit, implicitRole); }); +} - return unallowedRoles; +function roleIsAllowed(role, vNode, allowImplicit, implicitRole) { + if (allowImplicit && role === implicitRole) { + return true; + } + // if role is a dpub role make sure it's used on an element with a valid + // implicit role fallback + if (dpubRoles.includes(role) && getRoleType(role) !== implicitRole) { + return false; + } + // check if role is allowed on element + return isAriaRoleAllowedOnElement(vNode, role); } export default getElementUnallowedRoles; diff --git a/lib/commons/aria/is-aria-role-allowed-on-element.js b/lib/commons/aria/is-aria-role-allowed-on-element.js index 0051c7ac42..b7c3bb9191 100644 --- a/lib/commons/aria/is-aria-role-allowed-on-element.js +++ b/lib/commons/aria/is-aria-role-allowed-on-element.js @@ -15,17 +15,17 @@ function isAriaRoleAllowedOnElement(node, role) { node instanceof AbstractVirtuaNode ? node : getNodeFromTree(node); const implicitRole = getImplicitRole(vNode); - // always allow the explicit role to match the implicit role - if (role === implicitRole) { - return true; - } - const spec = getElementSpec(vNode); if (Array.isArray(spec.allowedRoles)) { return spec.allowedRoles.includes(role); } + // By default, ARIA in HTML does not allow implicit roles to be the same as explicit ones + // aria-allowed-roles has an `allowedImplicit` option to bypass this. + if (role === implicitRole) { + return false; + } return !!spec.allowedRoles; } diff --git a/lib/core/base/audit.js b/lib/core/base/audit.js index 035b30a753..a56eb51b45 100644 --- a/lib/core/base/audit.js +++ b/lib/core/base/audit.js @@ -592,11 +592,10 @@ class Audit { } else if (['tag', 'tags', undefined].includes(only.type)) { only.type = 'tag'; - const unmatchedTags = only.values.filter(tag => ( - !tags.includes(tag) && - !/wcag2[1-3]a{1,3}/.test(tag) - )); - if (unmatchedTags.length !== 0) { + const unmatchedTags = only.values.filter( + tag => !tags.includes(tag) && !/wcag2[1-3]a{1,3}/.test(tag) + ); + if (unmatchedTags.length !== 0) { axe.log('Could not find tags `' + unmatchedTags.join('`, `') + '`'); } } else { @@ -622,7 +621,7 @@ class Audit { application: this.application }; if (typeof branding === 'string') { - this.application = branding + this.application = branding; } if ( branding && diff --git a/lib/core/utils/send-command-to-frame.js b/lib/core/utils/send-command-to-frame.js index dd61ec069e..7d7f5b57ea 100644 --- a/lib/core/utils/send-command-to-frame.js +++ b/lib/core/utils/send-command-to-frame.js @@ -2,16 +2,15 @@ import getSelector from './get-selector'; import respondable from './respondable'; import log from '../log'; - /** * Sends a command to an instance of axe in the specified frame * @param {Element} node The frame element to send the message to * @param {Object} parameters Parameters to pass to the frame * @param {Function} callback Function to call when results from the frame has returned */ - export default function sendCommandToFrame(node, parameters, resolve, reject) { +export default function sendCommandToFrame(node, parameters, resolve, reject) { const win = node.contentWindow; - const pingWaitTime = parameters.options?.pingWaitTime ?? 500 + const pingWaitTime = parameters.options?.pingWaitTime ?? 500; if (!win) { log('Frame does not have a content window', node); resolve(null); diff --git a/lib/rules/aria-roles.json b/lib/rules/aria-roles.json index a8ac21dc4c..ac36928643 100644 --- a/lib/rules/aria-roles.json +++ b/lib/rules/aria-roles.json @@ -10,10 +10,10 @@ "all": [], "any": [], "none": [ - "fallbackrole", - "invalidrole", - "abstractrole", - "unsupportedrole", + "fallbackrole", + "invalidrole", + "abstractrole", + "unsupportedrole", "deprecatedrole" ] } diff --git a/test/checks/aria/aria-allowed-role.js b/test/checks/aria/aria-allowed-role.js index 0c78f505f7..605d595ebb 100644 --- a/test/checks/aria/aria-allowed-role.js +++ b/test/checks/aria/aria-allowed-role.js @@ -30,11 +30,11 @@ describe('aria-allowed-role', function() { var options = { allowImplicit: false }; - var actual = axe.testUtils + var outcome = axe.testUtils .getCheckEvaluate('aria-allowed-role') .call(checkContext, null, options, vNode); - var expected = false; - assert.equal(actual, expected); + + assert.isFalse(outcome); assert.deepEqual(checkContext._data, ['row']); }); diff --git a/test/checks/aria/deprecatedrole.js b/test/checks/aria/deprecatedrole.js index 566594d2ca..d57a348369 100644 --- a/test/checks/aria/deprecatedrole.js +++ b/test/checks/aria/deprecatedrole.js @@ -4,10 +4,10 @@ describe('deprecatedrole', function() { var checkContext = axe.testUtils.MockCheckContext(); var checkSetup = axe.testUtils.checkSetup; var checkEvaluate = axe.testUtils.getCheckEvaluate('deprecatedrole'); - afterEach(function () { + afterEach(function() { checkContext.reset(); axe.reset(); - }) + }); it('returns true if applied to a deprecated role', function() { axe.configure({ @@ -36,7 +36,9 @@ describe('deprecatedrole', function() { } } }); - var params = checkSetup('
Contents
'); + var params = checkSetup( + '
Contents
' + ); assert.isTrue(checkEvaluate.apply(checkContext, params)); assert.deepEqual(checkContext._data, 'doc-fizzbuzz'); }); @@ -57,8 +59,8 @@ describe('deprecatedrole', function() { assert.isNull(checkContext._data); }); - describe('with fallback roles', function () { - it('returns true if the deprecated role is the first valid role', function () { + describe('with fallback roles', function() { + it('returns true if the deprecated role is the first valid role', function() { axe.configure({ standards: { ariaRoles: { @@ -69,12 +71,14 @@ describe('deprecatedrole', function() { } } }); - var params = checkSetup('
Contents
'); + var params = checkSetup( + '
Contents
' + ); assert.isTrue(checkEvaluate.apply(checkContext, params)); assert.deepEqual(checkContext._data, 'melon'); - }) + }); - it('returns false if the deprecated role is not the first valid role', function () { + it('returns false if the deprecated role is not the first valid role', function() { axe.configure({ standards: { ariaRoles: { @@ -85,7 +89,9 @@ describe('deprecatedrole', function() { } } }); - var params = checkSetup('
Contents
'); + var params = checkSetup( + '
Contents
' + ); assert.isFalse(checkEvaluate.apply(checkContext, params)); assert.isNull(checkContext._data); }); diff --git a/test/commons/aria/get-element-unallowed-roles.js b/test/commons/aria/get-element-unallowed-roles.js index 20b6f1f74e..a899e37056 100644 --- a/test/commons/aria/get-element-unallowed-roles.js +++ b/test/commons/aria/get-element-unallowed-roles.js @@ -2,7 +2,7 @@ describe('aria.getElementUnallowedRoles', function() { var flatTreeSetup = axe.testUtils.flatTreeSetup; var getElementUnallowedRoles = axe.commons.aria.getElementUnallowedRoles; - it('returns false for INPUT with role application', function() { + it('returns unallowed role=application when used on a input elm', function() { var node = document.createElement('input'); var role = 'application'; node.setAttribute('type', ''); @@ -14,7 +14,7 @@ describe('aria.getElementUnallowedRoles', function() { assert.include(actual, role); }); - it('returns true for INPUT type is checkbox and has aria-pressed attribute', function() { + it('returns empty on type=checkbox and aria-pressed attr on input elm', function() { var node = document.createElement('input'); node.setAttribute('type', 'checkbox'); node.setAttribute('aria-pressed', ''); @@ -23,16 +23,16 @@ describe('aria.getElementUnallowedRoles', function() { assert.isEmpty(actual); }); - it('returns false for LI with role menubar', function() { + it('returns unallowed role=menubar when used on a li elm', function() { var node = document.createElement('li'); var role = 'menubar'; node.setAttribute('role', role); flatTreeSetup(node); var actual = getElementUnallowedRoles(node); - assert.isNotEmpty(actual); + assert.isNotEmpty(actual, role); }); - it('returns true for INPUT with type button and role menuitemcheckbox', function() { + it('returns empty on role=menuitemcheckbox with type=button on input elm', function() { var node = document.createElement('input'); var role = 'menuitemcheckbox'; node.setAttribute('role', role); @@ -42,7 +42,7 @@ describe('aria.getElementUnallowedRoles', function() { assert.isEmpty(actual); }); - it('returns false for SECTION with role option', function() { + it('returns unallowed role=option when used on section elm', function() { var node = document.createElement('section'); var role = 'option'; node.setAttribute('role', role); @@ -52,7 +52,7 @@ describe('aria.getElementUnallowedRoles', function() { assert.include(actual, role); }); - it('returns true for INPUT type radio with role menuitemradio', function() { + it('returns empty on role=menuitemradio and type=radio on input elm', function() { var node = document.createElement('input'); var role = 'menuitemradio'; node.setAttribute('role', role); @@ -62,25 +62,134 @@ describe('aria.getElementUnallowedRoles', function() { assert.isEmpty(actual); }); - it('returns false when role is implicit and allowImplicit is true (default)', function() { + it('returns unallowed role=textbox on a input elm and allowImplicit is true (default)', function() { var node = document.createElement('input'); var role = 'textbox'; node.setAttribute('role', role); flatTreeSetup(node); var actual = getElementUnallowedRoles(node, true); + assert.isEmpty(actual, role); + }); + + it('returns empty on role=button on div elm when role is not implicit and allowImplicit: false', function() { + var node = document.createElement('div'); + node.setAttribute('role', 'button'); + flatTreeSetup(node); + var actual = getElementUnallowedRoles(node, false); + assert.isEmpty(actual); + }); + + it('returns unallowed role=contentinfo on footer elm and allowImplicit: false', function() { + var node = document.createElement('footer'); + node.setAttribute('role', 'contentinfo'); + flatTreeSetup(node); + var actual = getElementUnallowedRoles(node, false); + assert.isNotEmpty(actual, 'contentinfo'); + }); + + it('returns unallowed role=banner on header elm and allowImplicit:false', function() { + var node = document.createElement('header'); + node.setAttribute('role', 'banner'); + flatTreeSetup(node); + var actual = getElementUnallowedRoles(node, false); + assert.isNotEmpty(actual, 'banner'); + }); + + it('returns empty on role=contentinfo on footer elm when allowImplicit:true', function() { + var node = document.createElement('footer'); + node.setAttribute('role', 'contentinfo'); + flatTreeSetup(node); + var actual = getElementUnallowedRoles(node); + assert.isEmpty(actual); + }); + + it('returns empty on role=banner on header elm when allowImplicit:true', function() { + var node = document.createElement('header'); + node.setAttribute('role', 'banner'); + flatTreeSetup(node); + var actual = getElementUnallowedRoles(node); + assert.isEmpty(actual); + }); + + it('returns empty role=doc-backlink on anchor elm and allowImplicit:false', function() { + var node = document.createElement('a'); + node.setAttribute('href', '#'); + node.setAttribute('role', 'doc-backlink'); + flatTreeSetup(node); + var actual = getElementUnallowedRoles(node, false); assert.isEmpty(actual); }); - it('returns false with implicit role of row for TR when allowImplicit is set to false via options', function() { + it('returns empty on role=doc-backlink on anchor elm when allowImplicit:true', function() { + var node = document.createElement('a'); + node.setAttribute('href', '#'); + node.setAttribute('role', 'doc-backlink'); + flatTreeSetup(node); + var actual = getElementUnallowedRoles(node); + assert.isEmpty(actual); + }); + + it('returns unallowed role=doc-backlink on anchor elm without href attr and allowImplicit:false', function() { + var node = document.createElement('a'); + node.setAttribute('role', 'doc-backlink'); + flatTreeSetup(node); + var actual = getElementUnallowedRoles(node, false); + assert.isNotEmpty(actual, 'doc-backlink'); + }); + + it('returns unallowed role=doc-backlink on anchor elm without href attr and allowImplicit:true', function() { + var node = document.createElement('a'); + node.setAttribute('role', 'doc-backlink'); + flatTreeSetup(node); + var actual = getElementUnallowedRoles(node); + assert.isNotEmpty(actual, 'doc-backlink'); + }); + + it('returns empty role=banner on header elm when using axe.configure and allowImplicit:false', function() { + axe.configure({ + standards: { + htmlElms: { + header: { + contentTypes: 'flow', + allowedRoles: ['banner'] + } + } + } + }); + var node = document.createElement('header'); + node.setAttribute('role', 'banner'); + flatTreeSetup(node); + var actual = getElementUnallowedRoles(node, false); + assert.isEmpty(actual); + }); + + it('returns empty role=contentinfo on footer elm when using axe.configure and allowImplicit:false', function() { + axe.configure({ + standards: { + htmlElms: { + footer: { + contentTypes: 'flow', + allowedRoles: ['contentinfo'] + } + } + } + }); + var node = document.createElement('footer'); + node.setAttribute('role', 'contentinfo'); + flatTreeSetup(node); + var actual = getElementUnallowedRoles(node, false); + assert.isEmpty(actual); + }); + + it('returns unallowed role=row when when used on TR element and allowImplicit:false', function() { var node = document.createElement('tr'); node.setAttribute('role', 'row'); flatTreeSetup(node); var actual = getElementUnallowedRoles(node, false); - assert.isNotEmpty(actual); - assert.include(actual, 'row'); + assert.isNotEmpty(actual, 'row'); }); - it('returns true for a SerialVirtualNode of INPUT with type checkbox and aria-pressed attribute', function() { + it('returns empty on type=checkbox and aria-pressed attr on SerialVirtualNode with a input elm', function() { var vNode = new axe.SerialVirtualNode({ nodeName: 'input', attributes: { @@ -92,7 +201,7 @@ describe('aria.getElementUnallowedRoles', function() { assert.isEmpty(actual); }); - it('returns false for a SerialVirtualNode of INPUT with role application', function() { + it('returns unallowed role=application for a SerialVirtualNode with a input elm', function() { var vNode = new axe.SerialVirtualNode({ nodeName: 'input', attributes: { diff --git a/test/commons/aria/is-aria-role-allowed-on-element.js b/test/commons/aria/is-aria-role-allowed-on-element.js index 6dbeb452f5..1f71bc0c43 100644 --- a/test/commons/aria/is-aria-role-allowed-on-element.js +++ b/test/commons/aria/is-aria-role-allowed-on-element.js @@ -215,12 +215,12 @@ describe('aria.isAriaRoleAllowedOnElement', function() { assert.isFalse(actual); }); - it('returns true if elements implicit role matches the role', function() { + it('returns false if elements implicit role matches the role', function() { var node = document.createElement('area'); node.setAttribute('href', '#yay'); node.setAttribute('role', 'link'); flatTreeSetup(node); var actual = axe.commons.aria.isAriaRoleAllowedOnElement(node, 'link'); - assert.isTrue(actual); + assert.isFalse(actual); }); }); diff --git a/test/core/base/audit.js b/test/core/base/audit.js index 734eec22f6..e20cafc933 100644 --- a/test/core/base/audit.js +++ b/test/core/base/audit.js @@ -1295,11 +1295,11 @@ describe('Audit', function() { describe('Audit#normalizeOptions', function() { var axeLog; - beforeEach(function () { + beforeEach(function() { axeLog = axe.log; }); - afterEach(function () { - axe.log = axeLog; + afterEach(function() { + axe.log = axeLog; }); it('returns the options object when it is valid', function() { @@ -1457,9 +1457,9 @@ describe('Audit', function() { }); }); - it('logs an issue when a tag is unknown', function () { + it('logs an issue when a tag is unknown', function() { var message = ''; - axe.log = function (m) { + axe.log = function(m) { message = m; }; a.normalizeOptions({ @@ -1471,9 +1471,9 @@ describe('Audit', function() { assert.include(message, 'Could not find tags'); }); - it('logs no issues for unknown WCAG level tags', function () { + it('logs no issues for unknown WCAG level tags', function() { var message = ''; - axe.log = function (m) { + axe.log = function(m) { message = m; }; a.normalizeOptions({ @@ -1485,9 +1485,9 @@ describe('Audit', function() { assert.isEmpty(message); }); - it('logs an issue when a tag is unknown, together with a wcag level tag', function () { + it('logs an issue when a tag is unknown, together with a wcag level tag', function() { var message = ''; - axe.log = function (m) { + axe.log = function(m) { message = m; }; a.normalizeOptions({ diff --git a/test/core/utils/get-frame-contexts.js b/test/core/utils/get-frame-contexts.js index 2c0367b0fa..4e58c37ee0 100644 --- a/test/core/utils/get-frame-contexts.js +++ b/test/core/utils/get-frame-contexts.js @@ -266,16 +266,16 @@ describe('utils.getFrameContexts', function() { }); }); - describe('options.iframes', function () { + describe('options.iframes', function() { it('returns a non-empty array with `iframes: true`', function() { fixture.innerHTML = ''; - var contexts = getFrameContexts({}, { iframes: true }) + var contexts = getFrameContexts({}, { iframes: true }); assert.lengthOf(contexts, 1); }); - it('returns an empty array with `iframes: false`', function () { + it('returns an empty array with `iframes: false`', function() { fixture.innerHTML = ''; - var contexts = getFrameContexts({}, { iframes: false }) + var contexts = getFrameContexts({}, { iframes: false }); assert.lengthOf(contexts, 0); }); }); diff --git a/test/core/utils/send-command-to-frame.js b/test/core/utils/send-command-to-frame.js index 6e76b4ed05..4b9eddd970 100644 --- a/test/core/utils/send-command-to-frame.js +++ b/test/core/utils/send-command-to-frame.js @@ -37,26 +37,28 @@ describe('axe.utils.sendCommandToFrame', function() { fixture.appendChild(frame); }); - it('adjusts skips ping with options.pingWaitTime=0', function (done) { + it('adjusts skips ping with options.pingWaitTime=0', function(done) { var frame = document.createElement('iframe'); var params = { - command: 'rules', + command: 'rules', options: { pingWaitTime: 0 } }; frame.addEventListener('load', function() { var topics = []; - frame.contentWindow.addEventListener('message', function (event) { + frame.contentWindow.addEventListener('message', function(event) { try { - topics.push(JSON.parse(event.data).topic) - } catch (_) { /* ignore */ } + topics.push(JSON.parse(event.data).topic); + } catch (_) { + /* ignore */ + } }); axe.utils.sendCommandToFrame( frame, params, captureError(function() { try { - assert.deepEqual(topics, ['axe.start']) + assert.deepEqual(topics, ['axe.start']); done(); } catch (e) { done(e); @@ -71,7 +73,7 @@ describe('axe.utils.sendCommandToFrame', function() { frame.id = 'level0'; frame.src = '../mock/frames/test.html'; fixture.appendChild(frame); - }) + }); it('should timeout if there is no response from frame', function(done) { var orig = window.setTimeout; diff --git a/test/integration/rules/aria-allowed-role/aria-allowed-role.html b/test/integration/rules/aria-allowed-role/aria-allowed-role.html index 077a00f711..2347efaa54 100644 --- a/test/integration/rules/aria-allowed-role/aria-allowed-role.html +++ b/test/integration/rules/aria-allowed-role/aria-allowed-role.html @@ -244,10 +244,20 @@

ok

ok
ok
- + +
+
ok
+ + - - - - + + + + diff --git a/test/integration/rules/aria-allowed-role/aria-allowed-role.json b/test/integration/rules/aria-allowed-role/aria-allowed-role.json index fa6f45dc90..2e87668337 100644 --- a/test/integration/rules/aria-allowed-role/aria-allowed-role.json +++ b/test/integration/rules/aria-allowed-role/aria-allowed-role.json @@ -73,6 +73,8 @@ ["#pass-graphics-document"], ["#pass-graphics-object"], ["#pass-graphics-symbol"], + ["#pass-header-banner"], + ["#pass-footer-contentinfo"], ["#pass-img-valid-role-aria-label"], ["#pass-img-valid-role-title"], ["#pass-img-valid-role-aria-labelledby"], diff --git a/test/integration/rules/aria-required-children/aria-required-children.html b/test/integration/rules/aria-required-children/aria-required-children.html index b411cd0d2c..1165908ff2 100644 --- a/test/integration/rules/aria-required-children/aria-required-children.html +++ b/test/integration/rules/aria-required-children/aria-required-children.html @@ -68,4 +68,4 @@
-
\ No newline at end of file +
From ef453771b252a04fb5854f7d4d5b10281889f395 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Wed, 24 Nov 2021 17:45:38 +0100 Subject: [PATCH 2/4] fix(scrollable-region-focusable): treat overflow:clip as hidden (#3304) --- lib/core/utils/get-scroll.js | 15 +++++++-------- test/core/utils/get-scroll.js | 12 ++++++++++++ .../scrollable-region-focusable.html | 6 ++++++ 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/lib/core/utils/get-scroll.js b/lib/core/utils/get-scroll.js index edbaf5310e..0771e71f3c 100644 --- a/lib/core/utils/get-scroll.js +++ b/lib/core/utils/get-scroll.js @@ -6,7 +6,7 @@ * @param {buffer} (Optional) allowed negligence in overflow * @returns {Object | undefined} */ -function getScroll(elm, buffer = 0) { + export default function getScroll(elm, buffer = 0) { const overflowX = elm.scrollWidth > elm.clientWidth + buffer; const overflowY = elm.scrollHeight > elm.clientHeight + buffer; @@ -19,12 +19,8 @@ function getScroll(elm, buffer = 0) { } const style = window.getComputedStyle(elm); - const overflowXStyle = style.getPropertyValue('overflow-x'); - const overflowYStyle = style.getPropertyValue('overflow-y'); - const scrollableX = - overflowXStyle !== 'visible' && overflowXStyle !== 'hidden'; - const scrollableY = - overflowYStyle !== 'visible' && overflowYStyle !== 'hidden'; + const scrollableX = isScrollable(style, 'overflow-x'); + const scrollableY = isScrollable(style, 'overflow-y'); /** * check direction of `overflow` and `scrollable` @@ -38,4 +34,7 @@ function getScroll(elm, buffer = 0) { } } -export default getScroll; +function isScrollable(style, prop) { + const overflowProp = style.getPropertyValue(prop); + return ['scroll', 'auto'].includes(overflowProp); +} diff --git a/test/core/utils/get-scroll.js b/test/core/utils/get-scroll.js index c3343ea8f8..33ceffeb13 100644 --- a/test/core/utils/get-scroll.js +++ b/test/core/utils/get-scroll.js @@ -45,6 +45,18 @@ describe('axe.utils.getScroll', function() { assert.isUndefined(actual); }); + it('returns undefined when element overflow is clip', function() { + var target = queryFixture( + '
' + + '
' + + '

Content

' + + '
' + + '
' + ); + var actual = axe.utils.getScroll(target.actualNode); + assert.isUndefined(actual); + }); + it('returns scroll offset when element overflow is auto', function() { var target = queryFixture( '
' + diff --git a/test/integration/rules/scrollable-region-focusable/scrollable-region-focusable.html b/test/integration/rules/scrollable-region-focusable/scrollable-region-focusable.html index 7ee3285851..c920d8e570 100644 --- a/test/integration/rules/scrollable-region-focusable/scrollable-region-focusable.html +++ b/test/integration/rules/scrollable-region-focusable/scrollable-region-focusable.html @@ -126,3 +126,9 @@
+ +
+
+

Content

+
+
\ No newline at end of file From f23d8c8e305d27c8323547731b335d2900e03239 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Wed, 24 Nov 2021 17:45:49 +0100 Subject: [PATCH 3/4] fix(core): Incomplete fallback was missing, and could cause infinite loop (#3302) --- build/configure.js | 12 ++--- .../helpers/incomplete-fallback-msg.js | 15 +++--- .../helpers/incomplete-fallback-msg.js | 47 +++++++++++++++++-- 3 files changed, 55 insertions(+), 19 deletions(-) diff --git a/build/configure.js b/build/configure.js index cec2b60744..1dfc2c7d6d 100644 --- a/build/configure.js +++ b/build/configure.js @@ -148,16 +148,10 @@ function buildRules(grunt, options, commons, callback) { } function getIncompleteMsg(summaries) { - var result = {}; - summaries.forEach(function(summary) { - if ( - summary.incompleteFallbackMessage && - doTRegex.test(summary.incompleteFallbackMessage) - ) { - result = doT.template(summary.incompleteFallbackMessage).toString(); - } + var summary = summaries.find(function(summary) { + return typeof summary.incompleteFallbackMessage === 'string'; }); - return result; + return summary ? summary.incompleteFallbackMessage : ''; } function replaceFunctions(string) { diff --git a/lib/core/reporters/helpers/incomplete-fallback-msg.js b/lib/core/reporters/helpers/incomplete-fallback-msg.js index 02678c7b6e..53d1d96553 100644 --- a/lib/core/reporters/helpers/incomplete-fallback-msg.js +++ b/lib/core/reporters/helpers/incomplete-fallback-msg.js @@ -3,10 +3,13 @@ * This mechanism allows the string to be localized. * @return {String} */ -function incompleteFallbackMessage() { - return typeof axe._audit.data.incompleteFallbackMessage === 'function' - ? axe._audit.data.incompleteFallbackMessage() - : axe._audit.data.incompleteFallbackMessage; + export default function incompleteFallbackMessage() { + let { incompleteFallbackMessage } = axe._audit.data; + if (typeof incompleteFallbackMessage === 'function') { + incompleteFallbackMessage = incompleteFallbackMessage(); + } + if (typeof incompleteFallbackMessage !== 'string') { + return ''; + } + return incompleteFallbackMessage; } - -export default incompleteFallbackMessage; diff --git a/test/core/reporters/helpers/incomplete-fallback-msg.js b/test/core/reporters/helpers/incomplete-fallback-msg.js index 5558b0ef84..4f0d9bbd44 100644 --- a/test/core/reporters/helpers/incomplete-fallback-msg.js +++ b/test/core/reporters/helpers/incomplete-fallback-msg.js @@ -1,6 +1,13 @@ describe('helpers.incompleteFallbackMessage', function() { 'use strict'; - beforeEach(function() { + + it('returns a non-empty string by default', function () { + var summary = helpers.incompleteFallbackMessage(); + assert.typeOf(summary, 'string'); + assert.notEqual(summary, ''); + }); + + it('should return a string', function() { axe._load({ messages: {}, rules: [], @@ -8,9 +15,6 @@ describe('helpers.incompleteFallbackMessage', function() { incompleteFallbackMessage: 'Dogs are the best' } }); - }); - - it('should return a string', function() { var summary = helpers.incompleteFallbackMessage(); assert.equal(summary, 'Dogs are the best'); }); @@ -29,4 +33,39 @@ describe('helpers.incompleteFallbackMessage', function() { var summary = helpers.incompleteFallbackMessage(); assert.equal(summary, 'Dogs are the best'); }); + + describe('when passed an invalid value', function () { + it('returns `` when set to an object', function () { + axe._load({ + messages: {}, + rules: [], + data: { + incompleteFallbackMessage: {} + } + }); + assert.equal(helpers.incompleteFallbackMessage(), ''); + }); + + it('returns `` when set to null', function () { + axe._load({ + messages: {}, + rules: [], + data: { + incompleteFallbackMessage: null + } + }); + assert.equal(helpers.incompleteFallbackMessage(), ''); + }); + + it('returns `` when set to undefined', function () { + axe._load({ + messages: {}, + rules: [], + data: { + incompleteFallbackMessage: undefined + } + }); + assert.equal(helpers.incompleteFallbackMessage(), ''); + }); + }); }); From 4bf7d35a1f283a181205bb31f8f4c64c450772ca Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Wed, 24 Nov 2021 17:45:59 +0100 Subject: [PATCH 4/4] fix(listitem): allow as child of menu (#3286) --- lib/checks/lists/listitem-evaluate.js | 16 ++-- test/checks/lists/listitem.js | 89 +++++++++++-------- test/integration/rules/listitem/listitem.html | 4 + test/integration/rules/listitem/listitem.json | 2 +- 4 files changed, 61 insertions(+), 50 deletions(-) diff --git a/lib/checks/lists/listitem-evaluate.js b/lib/checks/lists/listitem-evaluate.js index 0fa6382506..b09c5414bb 100644 --- a/lib/checks/lists/listitem-evaluate.js +++ b/lib/checks/lists/listitem-evaluate.js @@ -1,15 +1,14 @@ -import { getComposedParent } from '../../commons/dom'; -import { isValidRole } from '../../commons/aria'; +import { isValidRole, getExplicitRole } from '../../commons/aria'; -function listitemEvaluate(node) { - const parent = getComposedParent(node); +export default function listitemEvaluate(node, options, virtualNode) { + const { parent } = virtualNode; if (!parent) { // Can only happen with detached DOM nodes and roots: return undefined; } - const parentTagName = parent.nodeName.toUpperCase(); - const parentRole = (parent.getAttribute('role') || '').toLowerCase(); + const parentNodeName = parent.props.nodeName; + const parentRole = getExplicitRole(parent); if (['presentation', 'none', 'list'].includes(parentRole)) { return true; @@ -21,8 +20,5 @@ function listitemEvaluate(node) { }); return false; } - - return ['UL', 'OL'].includes(parentTagName); + return ['ul', 'ol', 'menu'].includes(parentNodeName); } - -export default listitemEvaluate; diff --git a/test/checks/lists/listitem.js b/test/checks/lists/listitem.js index 8d69f1fc2c..b9045e3937 100644 --- a/test/checks/lists/listitem.js +++ b/test/checks/lists/listitem.js @@ -1,74 +1,81 @@ describe('listitem', function() { 'use strict'; - var fixture = document.getElementById('fixture'); var shadowSupport = axe.testUtils.shadowSupport; var checkContext = axe.testUtils.MockCheckContext(); + var checkSetup = axe.testUtils.checkSetup; + var fixtureSetup = axe.testUtils.fixtureSetup; + var checkEvaluate = axe.testUtils.getCheckEvaluate('listitem'); afterEach(function() { - fixture.innerHTML = ''; checkContext.reset(); }); it('should pass if the listitem has a parent
    ', function() { - fixture.innerHTML = '
    1. My list item
    '; - var target = fixture.querySelector('#target'); - assert.isTrue(checks.listitem.evaluate.call(checkContext, target)); + var params = checkSetup('
    1. My list item
    '); + var result = checkEvaluate.apply(checkContext, params) + assert.isTrue(result); }); it('should pass if the listitem has a parent
      ', function() { - fixture.innerHTML = '
      • My list item
      '; - var target = fixture.querySelector('#target'); - assert.isTrue(checks.listitem.evaluate.call(checkContext, target)); + var params = checkSetup('
      • My list item
      '); + var result = checkEvaluate.apply(checkContext, params) + assert.isTrue(result); }); it('should pass if the listitem has a parent role=list', function() { - fixture.innerHTML = - '
    • My list item
      • '; - var target = fixture.querySelector('#target'); - assert.isTrue(checks.listitem.evaluate.call(checkContext, target)); + var params = checkSetup( + '
    • My list item
      • ' + ); + var result = checkEvaluate.apply(checkContext, params) + assert.isTrue(result); }); it('should pass if the listitem has a parent role=none', function() { - fixture.innerHTML = - '
      • My list item
      '; - var target = fixture.querySelector('#target'); - assert.isTrue(checks.listitem.evaluate.call(checkContext, target)); + var params = checkSetup( + '
      • My list item
      ' + ); + var result = checkEvaluate.apply(checkContext, params) + assert.isTrue(result); }); it('should pass if the listitem has a parent role=presentation', function() { - fixture.innerHTML = - '
      • My list item
      '; - var target = fixture.querySelector('#target'); - assert.isTrue(checks.listitem.evaluate.call(checkContext, target)); + var params = checkSetup( + '
      • My list item
      ' + ); + var result = checkEvaluate.apply(checkContext, params) + assert.isTrue(result); }); it('should fail if the listitem has an incorrect parent', function() { - fixture.innerHTML = '
    • My list item
      • '; - var target = fixture.querySelector('#target'); - assert.isFalse(checks.listitem.evaluate.call(checkContext, target)); + var params = checkSetup('
    • My list item
      • '); + var result = checkEvaluate.apply(checkContext, params) + assert.isFalse(result); }); it('should fail if the listitem has a parent
        with changed role', function() { - fixture.innerHTML = - '
        1. My list item
        '; - var target = fixture.querySelector('#target'); - assert.isFalse(checks.listitem.evaluate.call(checkContext, target)); + var params = checkSetup( + '
        1. My list item
        ' + ); + var result = checkEvaluate.apply(checkContext, params) + assert.isFalse(result); assert.equal(checkContext._data.messageKey, 'roleNotValid'); }); it('should pass if the listitem has a parent
          with an invalid role', function() { - fixture.innerHTML = - '
          1. My list item
          '; - var target = fixture.querySelector('#target'); - assert.isTrue(checks.listitem.evaluate.call(checkContext, target)); + var params = checkSetup( + '
          1. My list item
          ' + ); + var result = checkEvaluate.apply(checkContext, params) + assert.isTrue(result); }); it('should pass if the listitem has a parent
            with an abstract role', function() { - fixture.innerHTML = - '
            1. My list item
            '; - var target = fixture.querySelector('#target'); - assert.isTrue(checks.listitem.evaluate.call(checkContext, target)); + var params = checkSetup( + '
            1. My list item
            ' + ); + var result = checkEvaluate.apply(checkContext, params) + assert.isTrue(result); }); (shadowSupport.v1 ? it : xit)( @@ -78,9 +85,11 @@ describe('listitem', function() { node.innerHTML = '
          1. My list item
            2. '; var shadow = node.attachShadow({ mode: 'open' }); shadow.innerHTML = '
            '; - fixture.appendChild(node); + fixtureSetup(node); var target = node.querySelector('#target'); - assert.isTrue(checks.listitem.evaluate.call(checkContext, target)); + var virtualTarget = axe.utils.getNodeFromTree(target); + var result = checkEvaluate.apply(checkContext, [target, {}, virtualTarget]) + assert.isTrue(result); } ); @@ -91,9 +100,11 @@ describe('listitem', function() { node.innerHTML = '
          2. My list item
            3. '; var shadow = node.attachShadow({ mode: 'open' }); shadow.innerHTML = '
            '; - fixture.appendChild(node); + fixtureSetup(node); var target = node.querySelector('#target'); - assert.isFalse(checks.listitem.evaluate.call(checkContext, target)); + var virtualTarget = axe.utils.getNodeFromTree(target); + var result = checkEvaluate.apply(checkContext, [target, {}, virtualTarget]) + assert.isFalse(result); } ); }); diff --git a/test/integration/rules/listitem/listitem.html b/test/integration/rules/listitem/listitem.html index 71fbfb6d55..77fea1a212 100644 --- a/test/integration/rules/listitem/listitem.html +++ b/test/integration/rules/listitem/listitem.html @@ -19,3 +19,7 @@
            1. I too do not belong to a list.
            + + + + diff --git a/test/integration/rules/listitem/listitem.json b/test/integration/rules/listitem/listitem.json index 462226fdd1..3d9bd49139 100644 --- a/test/integration/rules/listitem/listitem.json +++ b/test/integration/rules/listitem/listitem.json @@ -2,5 +2,5 @@ "description": "listitem test", "rule": "listitem", "violations": [["#uncontained"], ["#ulrolechanged"], ["#olrolechanged"]], - "passes": [["#contained"], ["#alsocontained"], ["#presentation"], ["#none"]] + "passes": [["#contained"], ["#alsocontained"], ["#presentation"], ["#none"], ["#menuitem"]] }