Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Solve several accessible-name issues #1163

Merged
merged 29 commits into from
Feb 18, 2019
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
8c80f14
chore(WIP): rewrite accessibleText
WilcoFiers Sep 20, 2018
82e5c74
Merge branch 'develop' into a11y-name
WilcoFiers Sep 26, 2018
9d2451e
chore: More refactoring for accname
WilcoFiers Sep 26, 2018
305c864
chore(WIP): More improvements to accessibleName
WilcoFiers Oct 10, 2018
8501f34
feat: Reimplement accessible name computation
WilcoFiers Oct 10, 2018
dec7220
chore: All accessible name tests passing
WilcoFiers Oct 13, 2018
abb0673
chore(accName): All tests passing
WilcoFiers Oct 13, 2018
4de5489
chore: Add tests
WilcoFiers Nov 9, 2018
a98448b
chore: Test form-control-value
WilcoFiers Nov 23, 2018
e264958
chore: Merge develop
WilcoFiers Nov 23, 2018
1aded4a
chore: Refactor and add docs to accessible-text
WilcoFiers Nov 24, 2018
6e67b52
chore: Add tests for namedFromContents
WilcoFiers Nov 25, 2018
2a5020e
chore: Refactor subtreeText method
WilcoFiers Nov 25, 2018
6ff002b
chore: Refactor native accessible text methods
WilcoFiers Nov 25, 2018
4c6c351
chore: Coverage for text.labelText
WilcoFiers Dec 3, 2018
6917ec9
Merge branch 'develop' into a11y-name
WilcoFiers Jan 7, 2019
73ded40
Merge branch 'develop' into a11y-name
jeeyyy Jan 23, 2019
2244ed2
fix: update to axe.commons.matches usage
jeeyyy Jan 23, 2019
6c35b51
Merge branch 'develop' into a11y-name
jeeyyy Jan 23, 2019
9bddd36
test: fix nativeTextboxValue tests
jeeyyy Jan 23, 2019
3f9a969
test: fix failing tests
jeeyyy Jan 25, 2019
1227102
chore: merge from develop
jeeyyy Jan 25, 2019
dfa1bd7
fix: compute includeHidden as a part of accessibleName fn
jeeyyy Jan 25, 2019
d0f45e7
fix: do not mutate context in accessibleText
jeeyyy Jan 25, 2019
c3a1bdc
Merge branch 'develop' into a11y-name
jeeyyy Jan 25, 2019
c4f7b2b
Merge branch 'develop' into a11y-name
WilcoFiers Feb 5, 2019
516952c
chore: Refactor a11yText method for readability
WilcoFiers Feb 5, 2019
8fc69a3
chore: Update a11yText test results
WilcoFiers Feb 5, 2019
994155b
Merge branch 'develop' into a11y-name
WilcoFiers Feb 18, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 7 additions & 2 deletions lib/checks/forms/group-labelledby.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,17 @@ matchingNodes.forEach(groupItem => {
);
});

const accessibleTextOptions = {
// Prevent following further aria-labelledby refs:
inLabelledByContext: true
};

// filter items with no accessible name, do this last for performance reasons
uniqueLabels = uniqueLabels.filter(labelNode =>
text.accessibleText(labelNode, true)
text.accessibleText(labelNode, accessibleTextOptions)
);
sharedLabels = sharedLabels.filter(labelNode =>
text.accessibleText(labelNode, true)
text.accessibleText(labelNode, accessibleTextOptions)
);

if (uniqueLabels.length > 0 && sharedLabels.length > 0) {
Expand Down
6 changes: 4 additions & 2 deletions lib/checks/label/implicit.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
var label = axe.commons.dom.findUpVirtual(virtualNode, 'label');
const { dom, text } = axe.commons;

var label = dom.findUpVirtual(virtualNode, 'label');
if (label) {
return !!axe.commons.text.accessibleTextVirtual(label);
return !!text.accessibleText(label, { inControlContext: true });
}
return false;
4 changes: 2 additions & 2 deletions lib/checks/shared/aria-label.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
var label = node.getAttribute('aria-label');
return !!(label ? axe.commons.text.sanitize(label).trim() : '');
const { text, aria } = axe.commons;
return !!text.sanitize(aria.arialabelText(node));
7 changes: 2 additions & 5 deletions lib/checks/shared/aria-labelledby.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,2 @@
var getIdRefs = axe.commons.dom.idrefs;

return getIdRefs(node, 'aria-labelledby').some(function(elm) {
return elm && axe.commons.text.accessibleText(elm, true);
});
const { text, aria } = axe.commons;
return !!text.sanitize(aria.arialabelledbyText(node));
4 changes: 2 additions & 2 deletions lib/checks/shared/non-empty-title.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
var title = node.getAttribute('title');
return !!(title ? axe.commons.text.sanitize(title).trim() : '');
const { text } = axe.commons;
return !!text.sanitize(text.titleText(node));
15 changes: 15 additions & 0 deletions lib/commons/aria/arialabel-text.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/* global aria */

/**
* Get the text value of aria-label, if any
*
* @param {VirtualNode|Element} element
* @return {string} ARIA label
*/
aria.arialabelText = function arialabelText(node) {
node = node.actualNode || node;
if (node.nodeType !== 1) {
return '';
}
return node.getAttribute('aria-label') || '';
};
52 changes: 52 additions & 0 deletions lib/commons/aria/arialabelledby-text.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/* global aria, dom, text */

/**
* Get the accessible name based on aria-labelledby
*
* @param {VirtualNode} element
* @param {Object} context
* @property {Bool} inLabelledByContext Whether or not the lookup is part of aria-labelledby reference
* @property {Bool} inControlContext Whether or not the lookup is part of a native label reference
* @property {Element} startNode First node in accessible name computation
* @property {Bool} debug Enable logging for formControlValue
* @return {string} Cancatinated text value for referenced elements
*/
aria.arialabelledbyText = function arialabelledbyText(node, context = {}) {
node = node.actualNode || node;
/**
* Note: The there are significant difference in how many "leads" browsers follow.
* - Firefox stops after the first IDREF, so it
* doesn't follow aria-labelledby after a for:>ID ref.
* - Chrome seems to just keep iterating no matter how many levels deep.
* - AccName-AAM 1.1 suggests going one level deep, but to treat
* each ref type separately.
*
* Axe-core's implementation behaves most closely like Firefox as it seems
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: describe the way this works as "implementing the common denominator"

* to be the most common deniminator. Main difference is that Firefox
* includes the value of form controls in addition to aria-label(s),
* something no other browser seems to do. Axe doesn't do that.
*/
if (
node.nodeType !== 1 ||
context.inLabelledByContext ||
context.inControlContext
) {
return '';
}

const refs = dom.idrefs(node, 'aria-labelledby').filter(elm => elm);
return refs.reduce((accessibleName, elm) => {
const accessibleNameAdd = text.accessibleText(elm, {
// Prevent the infinite reference loop:
inLabelledByContext: true,
startNode: context.startNode || node,
...context
});

if (!accessibleName) {
return accessibleNameAdd;
} else {
return `${accessibleName} ${accessibleNameAdd}`;
}
}, '');
};
24 changes: 24 additions & 0 deletions lib/commons/aria/get-owned-virtual.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/* global aria, dom */

/**
* Get an element's owned elements
*
* @param {VirtualNode} element
* @return {VirtualNode[]} Owned elements
*/
aria.getOwnedVirtual = function getOwned({ actualNode, children }) {
jeeyyy marked this conversation as resolved.
Show resolved Hide resolved
if (!actualNode || !children) {
throw new Error('getOwnedVirtual requires a virtual node');
}
// TODO: Check that the element has a role
// TODO: Descend into children with role=presentation|none
// TODO: Exclude descendents owned by other elements

return dom.idrefs(actualNode, 'aria-owns').reduce((ownedElms, element) => {
if (element) {
const virtualNode = axe.utils.getNodeFromTree(axe._tree[0], element);
ownedElms.push(virtualNode);
}
return ownedElms;
}, children);
};
39 changes: 39 additions & 0 deletions lib/commons/aria/named-from-contents.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/* global aria */

/**
* Check if an element is named from contents
*
* @param {Node|VirtualNode} element
* @param {Object} options
* @property {Bool} strict Whether or not to follow the spects strictly
* @return {Bool}
*/
aria.namedFromContents = function namedFromContents(node, { strict } = {}) {
node = node.actualNode || node;
if (node.nodeType !== 1) {
return false;
}

const role = aria.getRole(node);
const roleDef = aria.lookupTable.role[role];

if (
(roleDef && roleDef.nameFrom.includes('contents')) ||
// TODO: This is a workaround for axe-core's over-assertive implicitRole computation
// once we fix that, this extra noImplicit check can be removed.
node.nodeName.toUpperCase() === 'TABLE'
) {
return true;
}

/**
* Note: Strictly speaking if the role is null, presentation, or none, the element
* isn't named from contents. Axe-core often needs to know if an element
* has content anyway, so we're allowing it here.
* Use { strict: true } to disable this behavior.
*/
if (strict) {
return false;
}
return !roleDef || ['presentation', 'none'].includes(role);
};