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(options): add ancestry CSS selector to nodes #2389

Merged
merged 8 commits into from
Jul 20, 2020
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
1 change: 1 addition & 0 deletions doc/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ Additionally, there are a number or properties that allow configuration of diffe
| `rules` | n/a | Enable or disable rules using the `enabled` property |
| `reporter` | `v1` | Which reporter to use (see [Configuration](#api-name-axeconfigure)) |
| `resultTypes` | n/a | Limit which result types are processed and aggregated |
| `ancestry` | `false` | Return CSS selector for elements, with all the element's ancestors |
| `xpath` | `false` | Return xpath selectors for elements |
| `absolutePaths` | `false` | Use absolute paths when creating element selectors |
| `iframes` | `true` | Tell axe to run inside iframes |
Expand Down
6 changes: 6 additions & 0 deletions lib/core/reporters/helpers/process-aggregate.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ function normalizeRelatedNodes(node, options) {
if (options.selectors !== false || relatedNode.fromFrame) {
res.target = relatedNode.selector;
}
if (options.ancestry) {
res.ancestry = relatedNode.ancestry;
}
if (options.xpath) {
res.xpath = relatedNode.xpath;
}
Expand Down Expand Up @@ -77,6 +80,9 @@ function processAggregate(results, options) {
if (options.selectors !== false || subResult.node.fromFrame) {
subResult.target = subResult.node.selector;
}
if (options.ancestry) {
subResult.ancestry = subResult.node.ancestry;
}
if (options.xpath) {
subResult.xpath = subResult.node.xpath;
}
Expand Down
24 changes: 19 additions & 5 deletions lib/core/utils/dq-element.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import getSelector from './get-selector';
import getAncestry from './get-ancestry';
import getXpath from './get-xpath';

function truncate(str, maxLength) {
Expand Down Expand Up @@ -51,13 +52,21 @@ function DqElement(element, options, spec) {

DqElement.prototype = {
/**
* A unique CSS selector for the element
* A unique CSS selector for the element, designed for readability
* @return {String}
*/
get selector() {
return this.spec.selector || [getSelector(this.element, this._options)];
},

/**
* A unique CSS selector for the element, including its ancestors down to the root node
* @return {String}
*/
get ancestry() {
return this.spec.ancestry || [getAncestry(this.element)];
},

/**
* Xpath to the element
* @return {String}
Expand All @@ -82,15 +91,20 @@ DqElement.prototype = {
return {
selector: this.selector,
source: this.source,
xpath: this.xpath
xpath: this.xpath,
ancestry: this.ancestry
};
}
};

DqElement.fromFrame = function(node, options, frame) {
node.selector.unshift(frame.selector);
node.xpath.unshift(frame.xpath);
return new DqElement(frame.element, options, node);
const spec = {
...node,
selector: [...frame.selector, ...node.selector],
ancestry: [...frame.ancestry, ...node.ancestry],
xpath: [...frame.xpath, ...node.xpath]
};
return new DqElement(frame.element, options, spec);
};

export default DqElement;
31 changes: 31 additions & 0 deletions lib/core/utils/get-ancestry.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import getShadowSelector from './get-shadow-selector';

function generateAncestry(node) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should cache these results on the virtual node as we process them so we aren't recalculating the entire ancestry tree of every descendant of a node.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see why that would make much of a difference. The way this is written is very fast. I doubt doing a lookup in a large set is going to be noticeably faster than walking a an additional two or three ancestors.

const nodeName = node.nodeName.toLowerCase();
const parent = node.parentElement;
if (!parent) {
return nodeName;
}

let nthChild = '';
if (
nodeName !== 'head' &&
nodeName !== 'body' &&
parent.children.length > 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be better if this was only checking if the parent had multiple of the nodeName? I know getSelector only adds nth-child stuff if it needs to.

Right now if the parent has two children but they are both different nodeNames, you still get the nth-child selector which doesn't do much but add noise:

<html>
  <body>
    <div>
      <a href="/foo">Foo</a>
      <span>Hello</span>
    </div> 
  </body>
</html>

Results in html > body > div:nth-child(1) > a:nth-child(1)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What would the benefit of that be? This seems faster.

) {
const index = Array.prototype.indexOf.call(parent.children, node) + 1;
nthChild = `:nth-child(${index})`;
}

return generateAncestry(parent) + ' > ' + nodeName + nthChild;
}

/**
* Gets a unique CSS selector
* @param {HTMLElement} node The element to get the selector for
* @param {Object} optional options
* @returns {String|Array<String>} Unique CSS selector for the node
*/
export default function getAncestry(elm, options) {
return getShadowSelector(generateAncestry, elm, options);
}
28 changes: 3 additions & 25 deletions lib/core/utils/get-selector.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import getFriendlyUriEnd from './get-friendly-uri-end';
import getNodeAttributes from './get-node-attributes';
import matchesSelector from './element-matches';
import isXHTML from './is-xhtml';
import getShadowSelector from './get-shadow-selector';

let xhtml;
const ignoredAttributes = [
Expand Down Expand Up @@ -389,29 +390,6 @@ function generateSelector(elm, options, doc) {
* @param {Object} optional options
* @returns {String|Array<String>} Unique CSS selector for the node
*/
function getSelector(elm, options = {}) {
if (!elm) {
return '';
}
let doc = (elm.getRootNode && elm.getRootNode()) || document;
if (doc.nodeType === 11) {
// DOCUMENT_FRAGMENT
let stack = [];
while (doc.nodeType === 11) {
if (!doc.host) {
return '';
}
stack.push({ elm: elm, doc: doc });
elm = doc.host;
doc = elm.getRootNode();
}
stack.push({ elm: elm, doc: doc });
return stack.reverse().map(comp => {
return generateSelector(comp.elm, options, comp.doc);
});
} else {
return generateSelector(elm, options, doc);
}
export default function getSelector(elm, options) {
return getShadowSelector(generateSelector, elm, options);
}

export default getSelector;
31 changes: 31 additions & 0 deletions lib/core/utils/get-shadow-selector.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/**
* Gets a unique CSS selector
* @param {HTMLElement} node The element to get the selector for
* @param {Object} optional options
* @returns {String|Array<String>} Unique CSS selector for the node
*/
function getShadowSelector(generateSelector, elm, options = {}) {
if (!elm) {
return '';
}
let doc = (elm.getRootNode && elm.getRootNode()) || document;
// Not a DOCUMENT_FRAGMENT - shadow DOM
if (doc.nodeType !== 11) {
return generateSelector(elm, options, doc);
}

let stack = [];
while (doc.nodeType === 11) {
if (!doc.host) {
return '';
}
stack.unshift({ elm, doc });
elm = doc.host;
doc = elm.getRootNode();
}

stack.unshift({ elm, doc });
return stack.map(({ elm, doc }) => generateSelector(elm, options, doc));
}

export default getShadowSelector;
2 changes: 2 additions & 0 deletions lib/core/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,11 @@ export { default as getNodeFromTree } from './get-node-from-tree';
export { default as getRootNode } from './get-root-node';
export { default as getScrollState } from './get-scroll-state';
export { default as getScroll } from './get-scroll';
export { default as getShadowSelector } from './get-shadow-selector';
export { default as getSelector, getSelectorData } from './get-selector';
export { default as getStyleSheetFactory } from './get-stylesheet-factory';
export { default as getXpath } from './get-xpath';
export { default as getAncestry } from './get-ancestry';
export { default as injectStyle } from './inject-style';
export { default as isHidden } from './is-hidden';
export { default as isHtmlElement } from './is-html-element';
Expand Down
76 changes: 32 additions & 44 deletions lib/core/utils/merge-results.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import getXpath from './get-xpath';
import DqElement from './dq-element';
import getAllChecks from './get-all-checks';
import nodeSorter from './node-sorter';
Expand All @@ -11,25 +10,16 @@ import findBy from './find-by';
* @param {HTMLElement} frameElement The frame element
* @param {String} frameSelector Unique CSS selector for the frame
*/
function pushFrame(resultSet, options, frameElement, frameSelector) {
var frameXpath = getXpath(frameElement);
var frameSpec = {
element: frameElement,
selector: frameSelector,
xpath: frameXpath
};
function pushFrame(resultSet, dqFrame, options) {
resultSet.forEach(res => {
res.node = DqElement.fromFrame(res.node, options, dqFrame);
const checks = getAllChecks(res);

resultSet.forEach(function(res) {
res.node = DqElement.fromFrame(res.node, options, frameSpec);

var checks = getAllChecks(res);
if (checks.length) {
checks.forEach(function(check) {
check.relatedNodes = check.relatedNodes.map(node =>
DqElement.fromFrame(node, options, frameSpec)
);
});
}
checks.forEach(check => {
check.relatedNodes = check.relatedNodes.map(node =>
DqElement.fromFrame(node, options, dqFrame)
);
});
});
}

Expand All @@ -41,20 +31,19 @@ function pushFrame(resultSet, options, frameElement, frameSelector) {
* @return {Array} The merged and sorted result
*/
function spliceNodes(target, to) {
'use strict';
const firstFromFrame = to[0].node;

var firstFromFrame = to[0].node,
sorterResult,
t;
for (var i = 0, l = target.length; i < l; i++) {
t = target[i].node;
sorterResult = nodeSorter(
{ actualNode: t.element },
for (let i = 0; i < target.length; i++) {
const node = target[i].node;
const sorterResult = nodeSorter(
{ actualNode: node.element },
{ actualNode: firstFromFrame.element }
);

if (
sorterResult > 0 ||
(sorterResult === 0 && firstFromFrame.selector.length < t.selector.length)
(sorterResult === 0 &&
firstFromFrame.selector.length < node.selector.length)
) {
target.splice.apply(target, [i, 0].concat(to));
return;
Expand All @@ -65,8 +54,6 @@ function spliceNodes(target, to) {
}

function normalizeResult(result) {
'use strict';

if (!result || !result.results) {
return null;
}
Expand All @@ -89,34 +76,35 @@ function normalizeResult(result) {
* @return {Array} The merged RuleResults; should only have one result per rule
*/
function mergeResults(frameResults, options) {
var result = [];
frameResults.forEach(function(frameResult) {
var results = normalizeResult(frameResult);
const mergedResult = [];
frameResults.forEach(frameResult => {
const results = normalizeResult(frameResult);
if (!results || !results.length) {
return;
}

results.forEach(function(ruleResult) {
if (ruleResult.nodes && frameResult.frame) {
pushFrame(
ruleResult.nodes,
options,
frameResult.frameElement,
frameResult.frame
);
let dqFrame;
if (frameResult.frameElement) {
const spec = { selector: [frameResult.frame] };
dqFrame = new DqElement(frameResult.frameElement, options, spec);
}

results.forEach(ruleResult => {
if (ruleResult.nodes && dqFrame) {
pushFrame(ruleResult.nodes, dqFrame, options);
}

var res = findBy(result, 'id', ruleResult.id);
var res = findBy(mergedResult, 'id', ruleResult.id);
if (!res) {
result.push(ruleResult);
mergedResult.push(ruleResult);
} else {
if (ruleResult.nodes.length) {
spliceNodes(res.nodes, ruleResult.nodes);
}
}
});
});
return result;
return mergedResult;
}

export default mergeResults;
Loading