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

feat(rule): identical-links-same-purpose #1649

Merged
merged 60 commits into from Jan 15, 2020
Merged
Show file tree
Hide file tree
Changes from 52 commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
0e74359
initial commit, files generated
jeeyyy Jun 5, 2019
56a79a6
add tests for rule matches
jeeyyy Jun 18, 2019
7a9f5e5
reconcile results in after fn
jeeyyy Jun 20, 2019
b4d85a8
add integration tests
jeeyyy Jun 21, 2019
41e0cbf
update tags and messages
jeeyyy Jun 21, 2019
54a58e1
update rule desc
jeeyyy Jun 21, 2019
b6313b9
some updates based on review
jeeyyy Jun 24, 2019
2ecfbb2
update
jeeyyy Jul 2, 2019
f365c13
update test
jeeyyy Jul 2, 2019
ddece99
chore: merge from develop
jeeyyy Aug 6, 2019
a580de9
fix: update rule implementation
jeeyyy Aug 13, 2019
7f45dcd
Merge branch 'develop' into rule-identical-links-same-purpose
jeeyyy Aug 13, 2019
67e1dfa
docs: update rule descriptions
jeeyyy Aug 13, 2019
d1ec366
fix: edge case when identical name do not have resource
jeeyyy Aug 15, 2019
2efa44e
test: update integration tests and add inapplicable tests
jeeyyy Aug 23, 2019
13222ed
fix: update matches, check and the tests
jeeyyy Aug 23, 2019
05b8d3a
update impl
jeeyyy Aug 26, 2019
6b69bcd
test: shadowDOM tests
jeeyyy Aug 26, 2019
4311d78
test: add after tests
jeeyyy Aug 26, 2019
6771316
refactor: helper methods that can be extracted to commons namespace l…
jeeyyy Aug 26, 2019
004026f
docs: update function comments
jeeyyy Aug 26, 2019
bff93fc
update
jeeyyy Aug 27, 2019
5953137
update
jeeyyy Aug 27, 2019
33a321d
update
jeeyyy Aug 27, 2019
48072b3
fix: update isVisible to handle AREA element
jeeyyy Sep 13, 2019
ac17884
fix: move getParsedResource to commons.dom namespace
jeeyyy Sep 13, 2019
425392a
fix: update rule and tests
jeeyyy Sep 13, 2019
1253bec
test: skip in phantomJs
jeeyyy Sep 13, 2019
0fcda3c
test: add nested iframe tests
jeeyyy Sep 13, 2019
9a562dc
test: update isvisible test
jeeyyy Sep 13, 2019
ebcfeaf
test: update rule matches tests
jeeyyy Sep 13, 2019
14dade1
chore: merge from develop
jeeyyy Sep 13, 2019
0498d1d
test: skip area tests in phantomjs
jeeyyy Sep 13, 2019
be24a4e
fix: tests
jeeyyy Sep 16, 2019
b3bab39
test: remove isPhantomJs skip for tests
jeeyyy Sep 16, 2019
9459f58
update fn urlPropsFromAttribute
jeeyyy Oct 28, 2019
5f2084a
update fn isAreaVisible
jeeyyy Oct 28, 2019
c1a6bb7
update integration tests
jeeyyy Oct 28, 2019
c11d793
update fn urlPropsFromAttribute
jeeyyy Oct 28, 2019
c959e5e
update after fn and fix tests
jeeyyy Oct 28, 2019
efeddf3
remove shadowDOM test in check
jeeyyy Oct 28, 2019
535f849
update port resolution from url to exclude defaults
jeeyyy Oct 28, 2019
882b3d5
chore: merge from develop
jeeyyy Jan 8, 2020
42dc874
update isVisible computation
jeeyyy Jan 9, 2020
f1276c3
add tests for isVisible
jeeyyy Jan 9, 2020
f0e1207
update urlPropsFromAttribute
jeeyyy Jan 9, 2020
8eb6a65
update tests
jeeyyy Jan 9, 2020
f59736a
update test
jeeyyy Jan 9, 2020
07d2168
chore: merge from develop
jeeyyy Jan 9, 2020
471cbc7
update assertion
jeeyyy Jan 9, 2020
40b9671
chore: merge from develop
jeeyyy Jan 10, 2020
4da8e0b
use findUp instead of closest
jeeyyy Jan 10, 2020
e3c33a2
updates based on review
jeeyyy Jan 14, 2020
c8a0667
remove experimental tag
jeeyyy Jan 14, 2020
e7a3e61
chore: merge from develop
jeeyyy Jan 14, 2020
dece1ef
ignore FTP url in IE11
jeeyyy Jan 14, 2020
4d65d51
update after fn to handle link results with no data
jeeyyy Jan 14, 2020
ef9337c
skip FTP testt on IE11
jeeyyy Jan 14, 2020
0f01e9d
empty commit to trigger build
jeeyyy Jan 14, 2020
34a4a84
changes based on review
jeeyyy Jan 15, 2020
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
1 change: 1 addition & 0 deletions doc/rule-descriptions.md
Expand Up @@ -42,6 +42,7 @@
| html-has-lang | Ensures every HTML document has a lang attribute | Serious | cat.language, wcag2a, wcag311 | true | true | false |
| html-lang-valid | Ensures the lang attribute of the <html> element has a valid value | Serious | cat.language, wcag2a, wcag311 | true | true | false |
| html-xml-lang-mismatch | Ensure that HTML elements with both valid lang and xml:lang attributes agree on the base language of the page | Moderate | cat.language, wcag2a, wcag311 | true | true | false |
| identical-links-same-purpose | Ensure that links with the same accessible name serve a similar purpose | Minor | wcag2aaa, wcag249, best-practice, experimental | true | false | true |
| image-alt | Ensures <img> elements have alternate text or a role of none or presentation | Critical | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a | true | true | false |
| image-redundant-alt | Ensure image alternative is not repeated as text | Minor | cat.text-alternatives, best-practice | true | true | false |
| input-button-name | Ensures input buttons have discernible text | Critical | cat.name-role-value, wcag2a, wcag412, section508, section508.22.a | true | true | false |
Expand Down
99 changes: 99 additions & 0 deletions lib/checks/navigation/identical-links-same-purpose-after.js
@@ -0,0 +1,99 @@
/**
* Skip, as no results to curate
*/
if (results.length < 2) {
return results;
}

/**
* for each result
* - get other results with matching accessible name
* - check if same purpose is served
* - if not change `result` to `undefined`
* - construct a list of unique results with relatedNodes to return
*/
const uniqueResults = [];
const nameMap = {};

for (let index = 0; index < results.length; index++) {
const currentResult = results[index];
const { name, urlProps } = currentResult.data;

/**
* This is to avoid duplications in the `nodeMap`
*/
if (nameMap[name]) {
jeeyyy marked this conversation as resolved.
Show resolved Hide resolved
continue;
}

const sameNameResults = results
.filter(({ data }, resultNum) =>
data.name === name && resultNum !== index
)
const isSameUrl = sameNameResults
.every(({ data }) => isIdenticalObject(data.urlProps, urlProps))

/**
* when identical nodes exists but do not resolve to same url, flag result as `incomplete`
*/
if (sameNameResults.length && !isSameUrl) {
currentResult.result = undefined;
}

/**
* -> deduplicate results (for both `pass` or `incomplete`) and add `relatedNodes` if any
*/
currentResult.relatedNodes = [];
currentResult.relatedNodes.push(
...sameNameResults.map(node => node.relatedNodes[0])
);

/**
* Update `nodeMap` with `sameNameResults`
*/
nameMap[name] = sameNameResults;

uniqueResults.push(currentResult);
}

return uniqueResults;


/**
* Check if two given objects are the same (Note: this fn is not extensive in terms of depth equality)
* @param {Object} a object a, to compare
* @param {*} b object b, to compare
* @returns {Boolean}
*/
function isIdenticalObject(a, b) {
if (!a || !b) {
return false
}

const aProps = Object.getOwnPropertyNames(a);
const bProps = Object.getOwnPropertyNames(b);

if (aProps.length !== bProps.length) {
return false;
}

const result = aProps.every(propName => {
const aValue = a[propName]
const bValue = b[propName]

if (typeof aValue !== typeof bValue) {
return false
}

if (
typeof aValue === `object` ||
typeof bValue === `object`
) {
return isIdenticalObject(aValue, bValue)
}

return aValue === bValue
})

return result;
}
31 changes: 31 additions & 0 deletions lib/checks/navigation/identical-links-same-purpose.js
@@ -0,0 +1,31 @@
/**
* Note: `identical-links-same-purpose-after` fn, helps reconcile the results
*/
const { dom, text } = axe.commons;

const accText = text.accessibleTextVirtual(virtualNode);
const name = text
.sanitize(
text.removeUnicode(accText, {
emoji: true,
nonBmp: true,
punctuations: true
})
)
.toLowerCase();

if (!name) {
return undefined;
}

/**
* Set `data` and `relatedNodes` for use in `after` fn
*/
const afterData = {
name,
urlProps: dom.urlPropsFromAttribute(node, 'href')
};
this.data(afterData);
this.relatedNodes([node]);
jeeyyy marked this conversation as resolved.
Show resolved Hide resolved

return true;
12 changes: 12 additions & 0 deletions lib/checks/navigation/identical-links-same-purpose.json
@@ -0,0 +1,12 @@
{
"id": "identical-links-same-purpose",
"evaluate": "identical-links-same-purpose.js",
"after": "identical-links-same-purpose-after.js",
"metadata": {
"impact": "minor",
"messages": {
"pass": "There are no other links with the same name, that go to a different URL",
"incomplete": "Check that links have the same purpose, or are intentionally ambiguous."
}
}
}
59 changes: 57 additions & 2 deletions lib/commons/dom/is-visible.js
Expand Up @@ -39,6 +39,50 @@ function isClipped(style) {
return false;
}

/**
* Check `AREA` element is visible
* - validate if it is a child of `map`
* - ensure `map` is referred by `img` using the `usemap` attribute
* @param {Element} areaEl `AREA` element
* @retruns {Boolean}
*/
function isAreaVisible(el, screenReader, recursed) {
/**
* Note:
* - Verified that `map` element cannot refer to `area` elements across different document trees
* - Verified that `map` element does not get affected by altering `display` property
*/
const mapEl = dom.findUp(el, 'map');
if (!mapEl) {
return false;
}

const mapElName = mapEl.getAttribute('name');
if (!mapElName) {
return false;
}

/**
* `map` element has to be in light DOM
*/
const mapElRootNode = dom.getRootNode(el);
if (!mapElRootNode || mapElRootNode.nodeType !== 9) {
return false;
}

const refs = axe.utils.querySelectorAll(
axe._tree,
`img[usemap="#${axe.utils.escapeSelector(mapElName)}"]`
);
if (!refs || !refs.length) {
return false;
}

return refs.some(({ actualNode }) =>
jeeyyy marked this conversation as resolved.
Show resolved Hide resolved
dom.isVisible(actualNode, screenReader, recursed)
);
}

/**
* Determine whether an element is visible
* @method isVisible
Expand Down Expand Up @@ -74,9 +118,13 @@ dom.isVisible = function(el, screenReader, recursed) {
}

const nodeName = el.nodeName.toUpperCase();

if (
style.getPropertyValue('display') === 'none' ||
/**
* Note:
* Firefox's user-agent always sets `AREA` element to `display:none`
* hence excluding the edge case, for visibility computation
*/
(nodeName !== 'AREA' && style.getPropertyValue('display') === 'none') ||
['STYLE', 'SCRIPT', 'NOSCRIPT', 'TEMPLATE'].includes(nodeName) ||
(!screenReader && isClipped(style)) ||
(!recursed &&
Expand All @@ -89,6 +137,13 @@ dom.isVisible = function(el, screenReader, recursed) {
return false;
}

/**
* check visibility of `AREA`
*/
if (nodeName === 'AREA') {
return isAreaVisible(el, screenReader, recursed);
}

const parent = el.assignedSlot ? el.assignedSlot : el.parentNode;
let isVisible = false;
if (parent) {
Expand Down
136 changes: 136 additions & 0 deletions lib/commons/dom/url-props-from-attribute.js
@@ -0,0 +1,136 @@
/* global dom */

/**
* Parse resource object for a given node from a specified attribute
* @method urlPropsFromAttribute
* @param {HTMLElement} node given node
* @param {String} attribute attribute of the node from which resource should be parsed
* @returns {Object}
*/
dom.urlPropsFromAttribute = function urlPropsFromAttribute(node, attribute) {
const value = node[attribute];
if (!value) {
return undefined;
}

const nodeName = node.nodeName.toUpperCase();
let parser = node;

/**
* Note:
* The need to create a parser, is to keep this function generic, to be able to parse resource from element like `iframe` with `src` attribute
*/
if (!['A', 'AREA'].includes(nodeName)) {
parser = document.createElement('a');
parser.href = value;
}

const { pathname, filename } = getPathnameOrFilename(parser.pathname);

return {
protocol: parser.protocol,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not so sure we should include this. http://deque.com is the same URL as https://deque.com I'd say.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Upon further reading, the protocol section of the URI is usually called a scheme & I found this long list of schemes - https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml that can form the part of the URI.

At this juncture I am not sure if keeping a finite list of protocols and tweaking it is a good idea (as in convert https to http). Please provide your thoughts.

Copy link
Contributor

Choose a reason for hiding this comment

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

Even so, "http" and "https" should be treated as the same, same for ftp and ftps. Please add that.

hostname: parser.hostname,
port: getPort(parser.port),
pathname: /\/$/.test(pathname) ? pathname : `${pathname}/`,
search: getSearchPairs(parser.search),
hash: getHashRoute(parser.hash),
filename
};
};

/**
* Resolve given port excluding default port(s)
* @param {String} port port
* @returns {String}
*/
function getPort(port) {
const excludePorts = [
`443`, // default `https` port
`80`
];
return !excludePorts.includes(port) ? port : ``;
}

/**
* Resolve if a given pathname has filename & resolve the same as parts
* @method getPathnameOrFilename
* @param {String} pathname pathname part of a given uri
* @returns {Array<Object>}
*/
function getPathnameOrFilename(pathname) {
const filename = pathname.split('/').pop();
if (!filename || filename.indexOf('.') === -1) {
return {
pathname,
filename: ``
};
}

return {
// remove `filename` from `pathname`
pathname: pathname.replace(filename, ''),

// ignore filename when index.*
filename: /index./.test(filename) ? `` : filename
};
}

/**
* Parse a given query string to key/value pairs sorted alphabetically
* @param {String} searchStr search string
* @returns {Object}
*/
function getSearchPairs(searchStr) {
const query = {};

if (!searchStr || !searchStr.length) {
return query;
}

// `substring` to remove `?` at the beginning of search string
const pairs = searchStr.substring(1).split(`&`);
if (!pairs || !pairs.length) {
return query;
}

for (let index = 0; index < pairs.length; index++) {
const pair = pairs[index];
const [key, value = ''] = pair.split(`=`);
query[decodeURIComponent(key)] = decodeURIComponent(value);
}

return query;
}

/**
* Interpret a given hash
* if `hash`
* -> is `hashbang` -or- `hash` is followed by `slash`
* -> it resolves to a different resource
* @method getHashRoute
* @param {String} hash hash component of a parsed uri
* @returns {String}
*/
function getHashRoute(hash) {
if (!hash) {
return ``;
}

/**
* Check for any conventionally-formatted hashbang that may be present
* eg: `#, #/, #!, #!/`
*/
const hashRegex = /#!?\/?/g;
const hasMatch = hash.match(hashRegex);
if (!hasMatch) {
return ``;
}

// do not resolve inline link as hash
const [matchedStr] = hasMatch;
if (matchedStr === '#') {
return ``;
}

return hash;
}
13 changes: 13 additions & 0 deletions lib/rules/identical-links-same-purpose-matches.js
@@ -0,0 +1,13 @@
const { aria, text } = axe.commons;

const hasAccName = !!text.accessibleTextVirtual(virtualNode);
if (!hasAccName) {
return false;
}

const role = aria.getRole(node);
if (role && role !== 'link') {
return false;
}

return true;
14 changes: 14 additions & 0 deletions lib/rules/identical-links-same-purpose.json
@@ -0,0 +1,14 @@
{
"id": "identical-links-same-purpose",
"selector": "a[href], area[href], [role=\"link\"]",
"excludeHidden": false,
"matches": "identical-links-same-purpose-matches.js",
jeeyyy marked this conversation as resolved.
Show resolved Hide resolved
"tags": ["wcag2aaa", "wcag249", "best-practice", "experimental"],
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if this needs to be experimental. Let me get back to that once this is closer to an approval.

"metadata": {
"description": "Ensure that links with the same accessible name serve a similar purpose",
"help": "Links with the same name have a similar purpose"
},
"all": ["identical-links-same-purpose"],
"any": [],
"none": []
}