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 19 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 doc/run-partial.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ const axeResults = await axe.finishRun(partialResults, options);

**note**: The code in this page uses native DOM methods. This will only work on frames with the same origin. Scripts do not have access to `contentWindow` of cross-origin frames. Use of `runPartial` and `finishRun` in browser drivers like Selenium and Puppeteer works in the same way.

**note**: Because `axe.runPartial()` is designed to be serialized, it will not return element references even if the `elementRef` option is set.

## axe.runPartial(context, options): Promise<PartialResult>

When using `axe.runPartial()` it is important to keep in mind that the `context` may be different for different frames. For example, `context` can be done in such a way that in frame A, `main` is excluded, and in frame B `footer` is. The `axe.utils.getFrameContexts` method will provide a list of frames that must be tested, and what context to test it with.
Expand Down
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
4 changes: 2 additions & 2 deletions lib/core/base/rule.js
Original file line number Diff line number Diff line change
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 = new DqElement(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 ? new DqElement(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
3 changes: 3 additions & 0 deletions lib/core/public/load.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import Audit from '../base/audit';
import cleanup from './cleanup';
import runRules from './run-rules';
import respondable from '../utils/respondable';
import nodeSerializer from '../utils/node-serializer';

/**
* Sets up Rules, Messages and default options for Checks, must be invoked before attempting analysis
Expand Down Expand Up @@ -33,6 +34,8 @@ function runCommand(data, keepalive, callback) {
context,
options,
(results, cleanupFn) => {
// Serialize all DqElements
results = nodeSerializer.mapRawResults(results);
resolve(results);
// Cleanup AFTER resolve so that selectors can be generated
cleanupFn();
Expand Down
25 changes: 6 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,17 @@ 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
}));
results = nodeSerializer.mapRawResults(results);
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 +50,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;
}
57 changes: 28 additions & 29 deletions lib/core/reporters/helpers/process-aggregate.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import constants from '../../constants';
import { nodeSerializer } from '../../utils';

const resultKeys = constants.resultGroups;

Expand Down Expand Up @@ -43,19 +44,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 +76,32 @@ 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
// Pass options to limit which properties are calculated
elmSpec = nodeSerializer.dqElmToSpec(elmSpec, runOptions);
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;
}
15 changes: 5 additions & 10 deletions lib/core/reporters/raw.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { nodeSerializer } from '../utils';

const rawReporter = (results, options, callback) => {
if (typeof options === 'function') {
callback = options;
Expand All @@ -13,16 +15,9 @@ const rawReporter = (results, options, callback) => {
const transformedResult = { ...result };
const types = ['passes', 'violations', 'incomplete', 'inapplicable'];
for (const type of types) {
// Some tests don't include all of the types, so we have to guard against that here.
// TODO: ensure tests always use "proper" results to avoid having these hacks in production code paths.
if (transformedResult[type] && Array.isArray(transformedResult[type])) {
transformedResult[type] = transformedResult[type].map(
({ node, ...typeResult }) => {
node = typeof node?.toJSON === 'function' ? node.toJSON() : node;
return { node, ...typeResult };
}
);
}
transformedResult[type] = nodeSerializer.mapRawNodeResults(
transformedResult[type]
);
}

return transformedResult;
Expand Down
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