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 42 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/backwards-compatibility-doc.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ In a minor release, we may add support for new technologies in the Public Utils

If the HTML page is unchanged, calls to the analysis function(s) when compared across minor or patch releases will return the same exact selector for the nodes in any of the result arrays. If the HTML page has changed, it is possible for the selector to be different but it is not guaranteed that the selector will be different.

APIs may be deprecated in a major or minor release. APIs that have been deprecated for 6 months or more will be removed in the next major release.
APIs may be deprecated in a major or minor release. APIs that have been deprecated for 6 months or more will be removed in the next major release.

A major or a minor release may introduce new Public Utils.

Expand Down
1 change: 1 addition & 0 deletions doc/rule-descriptions.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
| html-has-lang | Ensures every HTML document has a lang attribute | Serious | cat.language, wcag2a, wcag311 | true |
| html-lang-valid | Ensures the lang attribute of the <html> element has a valid value | Serious | cat.language, wcag2a, wcag311 | true |
| 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 |
| identical-links-same-purpose | Ensure that links with the same accessible name serve a similar purpose | Minor | wcag2aaa, wcag249, best-practice, experimental | 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 |
| image-redundant-alt | Ensure image alternative is not repeated as text | Minor | cat.text-alternatives, best-practice | true |
| input-button-name | Ensures input buttons have discernible text | Critical | cat.name-role-value, wcag2a, wcag412, section508, section508.22.a | true |
Expand Down
99 changes: 99 additions & 0 deletions lib/checks/navigation/identical-links-same-purpose-after.js
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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."
}
}
}
48 changes: 46 additions & 2 deletions lib/commons/dom/is-visible.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,39 @@ 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
* - using `.closest` fails in `phantomjs`, hence using `dom.findUp`
*/
const mapEl = dom.findUp(el, 'map');
if (!mapEl) {
return false;
}

const mapElName = mapEl.getAttribute('name');
if (!mapElName) {
return false;
}
const refs = document.querySelectorAll(
jeeyyy marked this conversation as resolved.
Show resolved Hide resolved
`img[usemap="#${axe.utils.escapeSelector(mapElName)}"]`
);
if (!refs || !refs.length) {
return false;
}

return Array.from(refs).some(ref => dom.isVisible(ref, screenReader, recursed))
}

/**
* Determine whether an element is visible
* @method isVisible
Expand Down Expand Up @@ -74,9 +107,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 +126,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
154 changes: 154 additions & 0 deletions lib/commons/dom/url-props-from-attribute.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
/* 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: pathname.replace(/\/$/, ''), // remove trialing slashes
jeeyyy marked this conversation as resolved.
Show resolved Hide resolved
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) {
/**
* Note: IE11 resolves port to `443` for any URL with `https:`
jeeyyy marked this conversation as resolved.
Show resolved Hide resolved
*/
const excludePorts = [`443`, `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<String>}
jeeyyy marked this conversation as resolved.
Show resolved Hide resolved
*/
function getPathnameOrFilename(pathname) {
if (!pathname) {
return;
jeeyyy marked this conversation as resolved.
Show resolved Hide resolved
}

const filename = pathname.split('/').pop();
if (!filename || filename.indexOf('.') === -1) {
return {
pathname
jeeyyy marked this conversation as resolved.
Show resolved Hide resolved
};
}

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

// ignore filename when index.*
filename: /index./.test(filename)
? undefined
: 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
}

const curatedSearchStr = searchStr[0] === `?` ? searchStr.substring(1) : searchStr
jeeyyy marked this conversation as resolved.
Show resolved Hide resolved
const pairs = curatedSearchStr.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);
}

/**
* Sort query `keys` alphabetically
*/
return Object.keys(query)
jeeyyy marked this conversation as resolved.
Show resolved Hide resolved
.sort()
.reduce((out, key) => {
out[key] = query[key]
return out
}, {});
}

/**
* 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|undefined}
jeeyyy marked this conversation as resolved.
Show resolved Hide resolved
*/
function getHashRoute(hash) {
if (!hash) {
return undefined;
}

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

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

return hash;
}
Loading