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

refactor: make element spec processing more cosistent #4093

Merged
merged 23 commits into from
Aug 21, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
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: 2 additions & 0 deletions lib/core/base/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import standards from '../../standards';
import RuleResult from './rule-result';
import {
clone,
DqElement,
queue,
preload,
findBy,
Expand Down Expand Up @@ -270,6 +271,7 @@ export default class Audit {
*/
run(context, options, resolve, reject) {
this.normalizeOptions(options);
DqElement.setRunOptions(options);

// TODO: es-modules_selectCache
axe._selectCache = [];
Expand Down
6 changes: 3 additions & 3 deletions lib/core/base/check.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import metadataFunctionMap from './metadata-function-map';
import CheckResult from './check-result';
import { DqElement, checkHelper, deepMerge } from '../utils';
import { nodeSerializer, checkHelper, deepMerge } from '../utils';

export function createExecutionContext(spec) {
/*eslint no-eval:0 */
Expand Down Expand Up @@ -108,7 +108,7 @@ Check.prototype.run = function run(node, options, context, resolve, reject) {
// possible reference error.
if (node && node.actualNode) {
// Save a reference to the node we errored on for futher debugging.
e.errorNode = new DqElement(node).toJSON();
e.errorNode = nodeSerializer.toSpec(node);
}
reject(e);
return;
Expand Down Expand Up @@ -162,7 +162,7 @@ Check.prototype.runSync = function runSync(node, options, context) {
// possible reference error.
if (node && node.actualNode) {
// Save a reference to the node we errored on for futher debugging.
e.errorNode = new DqElement(node).toJSON();
e.errorNode = nodeSerializer.toSpec(node);
}
throw e;
}
Expand Down
6 changes: 3 additions & 3 deletions lib/core/base/rule.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
getAllChecks,
getCheckOption,
queue,
DqElement,
nodeSerializer,
select,
assert
} from '../utils';
Expand Down Expand Up @@ -258,7 +258,7 @@ Rule.prototype.run = function run(context, options = {}, resolve, reject) {
.then(results => {
const result = getResult(results);
if (result) {
result.node = new DqElement(node, options);
result.node = nodeSerializer.toSpec(node);
ruleResult.nodes.push(result);

// mark rule as incomplete rather than failure for rules with reviewOnFail
Expand Down Expand Up @@ -327,7 +327,7 @@ Rule.prototype.runSync = function runSync(context, options = {}) {

const result = getResult(results);
if (result) {
result.node = node.actualNode ? new DqElement(node, options) : null;
result.node = node.actualNode ? nodeSerializer.toSpec(node) : null;
ruleResult.nodes.push(result);

// mark rule as incomplete rather than failure for rules with reviewOnFail
Expand Down
4 changes: 2 additions & 2 deletions lib/core/public/finish-run.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
mergeResults,
publishMetaData,
finalizeRuleResult,
DqElement,
nodeSerializer,
clone
} from '../utils';

Expand Down Expand Up @@ -47,7 +47,7 @@ function getMergedFrameSpecs({
}
// Include the selector/ancestry/... from the parent frames
return childFrameSpecs.map(childFrameSpec => {
return DqElement.mergeSpecs(childFrameSpec, parentFrameSpec);
return nodeSerializer.mergeSpecs(childFrameSpec, parentFrameSpec);
});
}

Expand Down
24 changes: 5 additions & 19 deletions lib/core/public/run-partial.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Context from '../base/context';
import teardown from './teardown';
import {
DqElement,
nodeSerializer,
getSelectorData,
assert,
getEnvironmentData
Expand All @@ -22,17 +22,16 @@ export default function runPartial(...args) {
axe._selectorData = getSelectorData(contextObj.flatTree);
axe._running = true;

// Even in the top frame, we don't support this with runPartial
options.elementRef = false;
straker marked this conversation as resolved.
Show resolved Hide resolved

return (
new Promise((res, rej) => {
axe._audit.run(contextObj, options, res, rej);
})
.then(results => {
results = results.map(({ nodes, ...result }) => ({
nodes: nodes.map(serializeNode),
...result
}));
const frames = contextObj.frames.map(({ node }) => {
return new DqElement(node, options).toJSON();
return nodeSerializer.toSpec(node);
});
let environmentData;
if (contextObj.initiator) {
Expand All @@ -50,16 +49,3 @@ export default function runPartial(...args) {
})
);
}

function serializeNode({ node, ...nodeResult }) {
nodeResult.node = node.toJSON();
for (const type of ['any', 'all', 'none']) {
nodeResult[type] = nodeResult[type].map(
({ relatedNodes, ...checkResult }) => ({
...checkResult,
relatedNodes: relatedNodes.map(relatedNode => relatedNode.toJSON())
})
);
}
return nodeResult;
}
54 changes: 25 additions & 29 deletions lib/core/reporters/helpers/process-aggregate.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,8 @@ export default function processAggregate(results, options) {
if (Array.isArray(ruleResult.nodes) && ruleResult.nodes.length > 0) {
ruleResult.nodes = ruleResult.nodes.map(subResult => {
if (typeof subResult.node === 'object') {
subResult.html = subResult.node.source;
if (options.elementRef && !subResult.node.fromFrame) {
subResult.element = subResult.node.element;
}
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;
}
const serialElm = trimElementSpec(subResult.node, options);
Object.assign(subResult, serialElm);
}
delete subResult.result;
delete subResult.node;
Expand Down Expand Up @@ -86,23 +75,30 @@ function normalizeRelatedNodes(node, options) {
.filter(checkRes => Array.isArray(checkRes.relatedNodes))
.forEach(checkRes => {
checkRes.relatedNodes = checkRes.relatedNodes.map(relatedNode => {
const res = {
html: relatedNode?.source ?? 'Undefined'
};
if (options.elementRef && !relatedNode?.fromFrame) {
res.element = relatedNode?.element ?? null;
}
if (options.selectors !== false || relatedNode?.fromFrame) {
res.target = relatedNode?.selector ?? [':root'];
}
if (options.ancestry) {
res.ancestry = relatedNode?.ancestry ?? [':root'];
}
if (options.xpath) {
res.xpath = relatedNode?.xpath ?? ['/'];
}
return res;
return trimElementSpec(relatedNode, options);
});
});
});
}

function trimElementSpec(elmSpec = {}, runOptions) {
straker marked this conversation as resolved.
Show resolved Hide resolved
const serialElm = {};
if (axe._audit.noHtml) {
serialElm.html = null;
} else {
serialElm.html = elmSpec.source ?? 'Undefined';
}
if (runOptions.elementRef && !elmSpec.fromFrame) {
serialElm.element = elmSpec.element ?? null;
}
if (runOptions.selectors !== false || elmSpec.fromFrame) {
serialElm.target = elmSpec.selector ?? [':root'];
}
if (runOptions.ancestry) {
serialElm.ancestry = elmSpec.ancestry ?? [':root'];
}
if (runOptions.xpath) {
serialElm.xpath = elmSpec.xpath ?? ['/'];
}
return serialElm;
}
6 changes: 3 additions & 3 deletions lib/core/utils/check-helper.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import toArray from './to-array';
import DqElement from './dq-element';
import nodeSerializer from './node-serializer';
import AbstractVirtualNode from '../base/virtual-node/abstract-virtual-node';

/**
Expand Down Expand Up @@ -43,8 +43,8 @@ function checkHelper(checkResult, options, resolve, reject) {
node = node.actualNode;
}
if (node instanceof window.Node) {
const dqElm = new DqElement(node, options);
checkResult.relatedNodes.push(dqElm);
const nodeSpec = nodeSerializer.toSpec(node);
dbjorge marked this conversation as resolved.
Show resolved Hide resolved
checkResult.relatedNodes.push(nodeSpec);
}
});
}
Expand Down
3 changes: 3 additions & 0 deletions lib/core/utils/collect-results-from-frames.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ export default function collectResultsFromFrames(
resolve,
reject
) {
// elementRefs can't be passed across frame boundaries
options = { ...options, elementRef: false };

var q = queue();
var frames = parentContent.frames;

Expand Down
56 changes: 47 additions & 9 deletions lib/core/utils/dq-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ import getAncestry from './get-ancestry';
import getXpath from './get-xpath';
import getNodeFromTree from './get-node-from-tree';
import AbstractVirtualNode from '../base/virtual-node/abstract-virtual-node';
import cache from '../base/cache';

const CACHE_KEY = 'DqElm.RunOptions';

function truncate(str, maxLength) {
maxLength = maxLength || 300;
Expand Down Expand Up @@ -30,9 +33,14 @@ function getSource(element) {
* "Serialized" `HTMLElement`. It will calculate the CSS selector,
* grab the source (outerHTML) and offer an array for storing frame paths
* @param {HTMLElement} element The element to serialize
* @param {Object} options Propagated from axe.run/etc
* @param {Object} spec Properties to use in place of the element when instantiated on Elements from other frames
*/
function DqElement(elm, options = {}, spec = {}) {
function DqElement(elm, options = null, spec = {}) {
if (!options) {
options = cache.get(CACHE_KEY) ?? {};
}

this.spec = spec;
if (elm instanceof AbstractVirtualNode) {
this._virtualNode = elm;
Expand All @@ -48,6 +56,8 @@ function DqElement(elm, options = {}, spec = {}) {
*/
this.fromFrame = this.spec.selector?.length > 1;

this._includeElementInJson = options.elementRef;

if (options.absolutePaths) {
this._options = { toRoot: true };
}
Expand Down Expand Up @@ -106,30 +116,58 @@ DqElement.prototype = {
return this._element;
},

/**
* Converts to a "spec", a form suitable for use with JSON.stringify
* (*not* to pre-stringified JSON)
* @returns {Object}
*/
toJSON() {
return {
const spec = {
selector: this.selector,
source: this.source,
xpath: this.xpath,
ancestry: this.ancestry,
nodeIndexes: this.nodeIndexes
nodeIndexes: this.nodeIndexes,
fromFrame: this.fromFrame
};
if (this._includeElementInJson) {
spec.element = this._element;
}
return spec;
}
};

/**
* @deprecated
*/
WilcoFiers marked this conversation as resolved.
Show resolved Hide resolved
DqElement.fromFrame = function fromFrame(node, options, frame) {
const spec = DqElement.mergeSpecs(node, frame);
return new DqElement(frame.element, options, spec);
};

DqElement.mergeSpecs = function mergeSpec(node, frame) {
DqElement.mergeSpecs = function mergeSpecs(child, parentFrame) {
// Parameter order reversed for backcompat
return {
...node,
selector: [...frame.selector, ...node.selector],
ancestry: [...frame.ancestry, ...node.ancestry],
xpath: [...frame.xpath, ...node.xpath],
nodeIndexes: [...frame.nodeIndexes, ...node.nodeIndexes]
...child,
selector: [...parentFrame.selector, ...child.selector],
ancestry: [...parentFrame.ancestry, ...child.ancestry],
xpath: [...parentFrame.xpath, ...child.xpath],
nodeIndexes: [...parentFrame.nodeIndexes, ...child.nodeIndexes],
fromFrame: true
};
};

/**
* Set the default options to be used
* @param {Object} RunOptions Options passed to axe.run()
* @property {boolean} elementRef Add element when toJSON is called
* @property {boolean} absolutePaths Use absolute path fro selectors
*/
DqElement.setRunOptions = function setRunOptions({
elementRef,
absolutePaths
}) {
cache.set(CACHE_KEY, { elementRef, absolutePaths });
};

export default DqElement;
1 change: 1 addition & 0 deletions lib/core/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export {
export { default as matchAncestry } from './match-ancestry';
export { default as memoize } from './memoize';
export { default as mergeResults } from './merge-results';
export { default as nodeSerializer } from './node-serializer';
export { default as nodeSorter } from './node-sorter';
export { default as nodeLookup } from './node-lookup';
export { default as parseCrossOriginStylesheet } from './parse-crossorigin-stylesheet';
Expand Down
Loading