Skip to content

Commit

Permalink
Merge branch 'develop' into aria-prohibited-fix
Browse files Browse the repository at this point in the history
  • Loading branch information
WilcoFiers committed Jan 13, 2022
2 parents 49e32b5 + fb5d990 commit c5fb527
Show file tree
Hide file tree
Showing 17 changed files with 175 additions and 53 deletions.
4 changes: 2 additions & 2 deletions axe.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ declare namespace axe {
type CrossFrameSelector = CrossTreeSelector[];

type ContextObject = {
include?: BaseSelector | Array<BaseSelector | BaseSelector[]>;
exclude?: BaseSelector | Array<BaseSelector | BaseSelector[]>;
include?: Node | BaseSelector | Array<Node | BaseSelector | BaseSelector[]>;
exclude?: Node | BaseSelector | Array<Node | BaseSelector | BaseSelector[]>;
};

type RunCallback = (error: Error, results: AxeResults) => void;
Expand Down
2 changes: 1 addition & 1 deletion doc/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ By default, `axe.run` will test the entire document. The context object is an op
The include exclude object is a JSON object with two attributes: include and exclude. Either include or exclude is required. If only `exclude` is specified; include will default to the entire `document`.

- A node, or
- An array of arrays of [CSS selectors](./developer-guide.md#supported-css-selectors)
- An array of Nodes or an array of arrays of [CSS selectors](./developer-guide.md#supported-css-selectors)
- If the nested array contains a single string, that string is the CSS selector
- If the nested array contains multiple strings
- The last string is the final CSS selector
Expand Down
10 changes: 8 additions & 2 deletions lib/checks/aria/aria-allowed-attr-evaluate.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { uniqueArray, closest } from '../../core/utils';
import { uniqueArray, closest, isHtmlElement } from '../../core/utils';
import { getRole, allowedAttr, validateAttr } from '../../commons/aria';
import { isFocusable } from '../../commons/dom';
import cache from '../../core/base/cache';

/**
Expand Down Expand Up @@ -69,7 +70,7 @@ function ariaAllowedAttrEvaluate(node, options, virtualNode) {
ariaAttr.forEach(attr => {
preChecks[attr] = validateRowAttrs;
});
if (role && allowed) {
if (allowed) {
for (let i = 0; i < attrs.length; i++) {
const attrName = attrs[i];
if (validateAttr(attrName) && preChecks[attrName]?.()) {
Expand All @@ -82,6 +83,11 @@ function ariaAllowedAttrEvaluate(node, options, virtualNode) {

if (invalid.length) {
this.data(invalid);

if (!isHtmlElement(virtualNode) && !role && !isFocusable(virtualNode)) {
return undefined;
}

return false;
}

Expand Down
3 changes: 2 additions & 1 deletion lib/checks/aria/aria-allowed-attr.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
"fail": {
"singular": "ARIA attribute is not allowed: ${data.values}",
"plural": "ARIA attributes are not allowed: ${data.values}"
}
},
"incomplete": "Check that there is no problem if the ARIA attribute is ignored on this element: ${data.values}"
}
}
}
20 changes: 9 additions & 11 deletions lib/checks/label/label-content-name-mismatch-evaluate.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import {
accessibleText,
isHumanInterpretable,
visibleTextNodes,
isIconLigature,
subtreeText,
sanitize,
removeUnicode
} from '../../commons/text';
Expand Down Expand Up @@ -46,15 +45,14 @@ function labelContentNameMismatchEvaluate(node, options, virtualNode) {
return undefined;
}

const textVNodes = visibleTextNodes(virtualNode);
const nonLigatureText = textVNodes
.filter(
textVNode =>
!isIconLigature(textVNode, pixelThreshold, occuranceThreshold)
)
.map(textVNode => textVNode.actualNode.nodeValue)
.join('');
const visibleText = sanitize(nonLigatureText).toLowerCase();
const visibleText = sanitize(
subtreeText(virtualNode, {
subtreeDescendant: true,
ignoreIconLigature: true,
pixelThreshold,
occuranceThreshold
})
).toLowerCase();
if (!visibleText) {
return true;
}
Expand Down
21 changes: 21 additions & 0 deletions lib/commons/text/accessible-text-virtual.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import subtreeText from './subtree-text';
import titleText from './title-text';
import sanitize from './sanitize';
import isVisible from '../dom/is-visible';
import isIconLigature from '../text/is-icon-ligature';

/**
* Finds virtual node and calls accessibleTextVirtual()
Expand All @@ -26,6 +27,11 @@ function accessibleTextVirtual(virtualNode, context = {}) {
return '';
}

// Ignore ligature icons
if (shouldIgnoreIconLigature(virtualNode, context)) {
return '';
}

const computationSteps = [
arialabelledbyText, // Step 2B.1
arialabelText, // Step 2C
Expand Down Expand Up @@ -91,6 +97,21 @@ function shouldIgnoreHidden({ actualNode }, context) {
return !isVisible(actualNode, true);
}

/**
* Check if a ligature icon should be ignored
* @param {VirtualNode} element
* @param {VirtualNode} element
* @param {Object} context
* @return {Boolean}
*/
function shouldIgnoreIconLigature(virtualNode, context) {
const { ignoreIconLigature, pixelThreshold, occuranceThreshold } = context;
if (virtualNode.props.nodeType !== 3 || !ignoreIconLigature) {
return false;
}
return isIconLigature(virtualNode, pixelThreshold, occuranceThreshold);
}

/**
* Apply defaults to the context
* @param {VirtualNode} element
Expand Down
1 change: 1 addition & 0 deletions lib/commons/text/visible-text-nodes.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import isVisible from '../dom/is-visible';
* @instance
* @param {VirtualNode} vNode
* @return {VitrualNode[]}
* @deprecated
*/
function visibleTextNodes(vNode) {
const parentVisible = isVisible(vNode.actualNode);
Expand Down
51 changes: 30 additions & 21 deletions test/aria-practices/apg.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ const { getWebdriver, connectToChromeDriver } = require('./run-server');
const { assert } = require('chai');
const globby = require('globby');

describe('aria-practices', function () {
describe('aria-practices', function() {
// Use path.resolve rather than require.resolve because APG has no package.json
const apgPath = path.resolve(__dirname, '../../node_modules/aria-practices/');
const filePaths = globby.sync(`${apgPath}/examples/**/*.html`)
const testFiles = filePaths.map(fileName => fileName.split('/aria-practices/examples/')[1])
const filePaths = globby.sync(`${apgPath}/examples/**/*.html`);
const testFiles = filePaths.map(
fileName => fileName.split('/aria-practices/examples/')[1]
);
const port = 9515;
const addr = `http://localhost:9876/node_modules/aria-practices/`;
let driver, axeSource;
Expand All @@ -36,22 +38,24 @@ describe('aria-practices', function () {
'color-contrast',
'heading-order', // w3c/aria-practices#2119
'list', // w3c/aria-practices#2118
'scrollable-region-focusable', // w3c/aria-practices#2114
'scrollable-region-focusable' // w3c/aria-practices#2114
],
'feed/feedDisplay.html': ['page-has-heading-one'], // w3c/aria-practices#2120
// "page within a page" type thing going on
'menubar/menubar-navigation.html': [
'aria-allowed-role',
'landmark-banner-is-top-level',
'landmark-contentinfo-is-top-level',
'landmark-contentinfo-is-top-level'
],
// "page within a page" type thing going on
'treeview/treeview-navigation.html': [
'aria-allowed-role',
'landmark-banner-is-top-level',
'landmark-contentinfo-is-top-level'
]
}
],
//https://github.com/w3c/aria-practices/issues/2199
'button/button_idl.html': ['aria-allowed-attr']
};

// Not an actual content file
const skippedPages = [
Expand All @@ -60,19 +64,24 @@ describe('aria-practices', function () {
'toolbar/help.html' // Embedded into another page
];

testFiles.filter(filePath => !skippedPages.includes(filePath)).forEach(filePath => {
it(`finds no issue in "${filePath}"`, async () => {
await driver.get(`${addr}/examples/${filePath}`);

const builder = new AxeBuilder(driver, axeSource);
builder.disableRules([
...disabledRules['*'],
...(disabledRules[filePath] || []),
]);

const { violations } = await builder.analyze();
const issues = violations.map(({ id, nodes }) => ({ id, issues: nodes.length }))
assert.lengthOf(issues, 0);
testFiles
.filter(filePath => !skippedPages.includes(filePath))
.forEach(filePath => {
it(`finds no issue in "${filePath}"`, async () => {
await driver.get(`${addr}/examples/${filePath}`);

const builder = new AxeBuilder(driver, axeSource);
builder.disableRules([
...disabledRules['*'],
...(disabledRules[filePath] || [])
]);

const { violations } = await builder.analyze();
const issues = violations.map(({ id, nodes }) => ({
id,
issues: nodes.length
}));
assert.lengthOf(issues, 0);
});
});
});
});
46 changes: 37 additions & 9 deletions test/checks/aria/allowed-attr.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('aria-allowed-attr', function() {
assert.isNull(checkContext._data);
});

it.skip('should return false for non-global attributes if there is no role', function() {
it('should return false for non-global attributes if there is no role', function() {
var vNode = queryFixture(
'<div id="target" tabindex="1" aria-selected="true" aria-owns="foo"></div>'
);
Expand All @@ -72,21 +72,22 @@ describe('aria-allowed-attr', function() {
assert.deepEqual(checkContext._data, ['aria-selected="true"']);
});

it('should return true for non-global attributes if there is no role', function() {
it('should not report on invalid attributes', function() {
var vNode = queryFixture(
'<div id="target" tabindex="1" aria-selected="true" aria-owns="foo"></div>'
'<div role="dialog" id="target" tabindex="1" aria-cats="true"></div>'
);

assert.isTrue(
axe.testUtils
.getCheckEvaluate('aria-allowed-attr')
.call(checkContext, null, null, vNode)
);
assert.isNull(checkContext._data);
});

it('should not report on invalid attributes', function() {
it('should not report on allowed attributes', function() {
var vNode = queryFixture(
'<div role="dialog" id="target" tabindex="1" aria-cats="true"></div>'
'<div role="radio" id="target" tabindex="1" aria-required="true" aria-checked="true"></div>'
);

assert.isTrue(
Expand All @@ -97,18 +98,45 @@ describe('aria-allowed-attr', function() {
assert.isNull(checkContext._data);
});

it('should not report on allowed attributes', function() {
it('should return undefined for custom element that has no role and is not focusable', function() {
var vNode = queryFixture(
'<div role="radio" id="target" tabindex="1" aria-required="true" aria-checked="true"></div>'
'<my-custom-element id="target" aria-expanded="true"></my-custom-element>'
);

assert.isTrue(
assert.isUndefined(
axe.testUtils
.getCheckEvaluate('aria-allowed-attr')
.call(checkContext, null, null, vNode)
);
assert.isNull(checkContext._data);
assert.isNotNull(checkContext._data);
});

it("should return false for custom element that has a role which doesn't allow the attribute", function() {
var vNode = queryFixture(
'<my-custom-element role="insertion" id="target" aria-expanded="true"></my-custom-element>'
);

assert.isFalse(
axe.testUtils
.getCheckEvaluate('aria-allowed-attr')
.call(checkContext, null, null, vNode)
);
assert.isNotNull(checkContext._data);
});

it('should return false for custom element that is focusable', function() {
var vNode = queryFixture(
'<my-custom-element tabindex="1" id="target" aria-expanded="true"></my-custom-element>'
);

assert.isFalse(
axe.testUtils
.getCheckEvaluate('aria-allowed-attr')
.call(checkContext, null, null, vNode)
);
assert.isNotNull(checkContext._data);
});

describe('invalid aria-attributes when used on role=row as a descendant of a table or a grid', function() {
[
'aria-posinset="1"',
Expand Down
8 changes: 8 additions & 0 deletions test/checks/label/label-content-name-mismatch.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,4 +178,12 @@ describe('label-content-name-mismatch tests', function() {
var actual = check.evaluate(vNode.actualNode, options, vNode);
assert.isFalse(actual);
});

it('returns true when text contains <br/>', function() {
var vNode = queryFixture(
'<button id="target" aria-label="button label">button<br>label</button>'
);
var actual = check.evaluate(vNode.actualNode, options, vNode);
assert.isTrue(actual);
});
});
30 changes: 30 additions & 0 deletions test/commons/text/accessible-text.js
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,36 @@ describe('text.accessibleTextVirtual', function() {
assert.equal(axe.commons.text.accessibleTextVirtual(target), 'Hello World');
});

(!!document.fonts ? it : xit)(
'should allow ignoring icon ligatures',
function(done) {
var materialFont = new FontFace(
'Material Icons',
'url(https://fonts.gstatic.com/s/materialicons/v48/flUhRq6tzZclQEJ-Vdg-IuiaDsNcIhQ8tQ.woff2)'
);
materialFont.load().then(function() {
document.fonts.add(materialFont);

fixture.innerHTML =
'<button id="target">Hello World<span style="font-family: \'Material Icons\'">delete</span></button>';
axe.testUtils.flatTreeSetup(fixture);

var target = axe.utils.querySelectorAll(axe._tree, 'button')[0];
try {
assert.equal(
axe.commons.text.accessibleTextVirtual(target, {
ignoreIconLigature: true
}),
'Hello World'
);
done();
} catch (err) {
done(err);
}
});
}
);

(shadowSupport.v1 ? it : xit)(
'should only find aria-labelledby element in the same context ',
function() {
Expand Down
4 changes: 2 additions & 2 deletions test/integration/rules/aria-allowed-attr/failures.html
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@
<kbd aria-label="value" id="fail27"></kbd>
<abbr aria-label="value" id="fail28"></abbr>
<custom-elm aria-label="value" id="fail29"></custom-elm>
<!-- <audio
<audio
src="/test/assets/moon-speech.mp3"
controls
aria-orientation="horizontal"
id="fail30"
></audio> -->
></audio>
<div role="mark" aria-label="value" id="fail31"></div>
<div role="mark" aria-labelledby="value" id="fail32"></div>
<div role="suggestion" aria-label="value" id="fail33"></div>
Expand Down
1 change: 1 addition & 0 deletions test/integration/rules/aria-allowed-attr/failures.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
["#fail27"],
["#fail28"],
["#fail29"],
["#fail30"],
["#fail31"],
["#fail32"],
["#fail33"],
Expand Down
3 changes: 2 additions & 1 deletion test/integration/rules/aria-allowed-attr/incomplete.html
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
<div id="incomplete0" aria-label="foo">Foo</div>
<div id="incomplete1" aria-labelledby="missing">Foo</div>
<div id="incomplete2" aria-label="foo" role="code">Foo</div>
<my-custom-elm id="incomplete2" aria-expanded="true">Foo</my-custom-elm>
<div id="incomplete3" aria-label="foo" role="code">Foo</div>
Loading

0 comments on commit c5fb527

Please sign in to comment.