refactor: make element spec processing more cosistent (#4093)

* DqElement.setSerializer initial impl * implemented and passing integration tests * Separated nodeSerializer * Cleanup & tests * prevent html: "Undefined" when noHtml is set * Update test/core/public/run-partial.js * Tweaks * Change to nodeSerializer.update * WIP: Delay DqElement serialization * Delay node serialization * Tweak * Simplify a bit further * Cleanup * Improve performance * Update raw reporter * DqElement in checkHelper * Update lib/core/utils/check-helper.js Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com> * Update lib/core/utils/dq-element.js * Update lib/core/utils/node-serializer.js Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com> --------- Co-authored-by: Wilco Fiers <wilco.fiers@deque.com> Co-authored-by: Wilco Fiers <WilcoFiers@users.noreply.github.com> Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>
dequelabs · Aug 21, 2023 · 381b2c3 · 381b2c3
1 parent 1494b4c
commit 381b2c3
Show file tree

Hide file tree

Showing 32 changed files with 1,008 additions and 262 deletions.
diff --git a/doc/run-partial.md b/doc/run-partial.md
@@ -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.

diff --git a/lib/core/base/audit.js b/lib/core/base/audit.js
@@ -4,6 +4,7 @@ import standards from '../../standards';
 import RuleResult from './rule-result';
 import {
   clone,
+  DqElement,
   queue,
   preload,
   findBy,
@@ -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 = [];

diff --git a/lib/core/base/check.js b/lib/core/base/check.js
@@ -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 */
@@ -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;
@@ -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;
   }

diff --git a/lib/core/base/rule.js b/lib/core/base/rule.js
@@ -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
@@ -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

diff --git a/lib/core/public/finish-run.js b/lib/core/public/finish-run.js
@@ -3,7 +3,7 @@ import {
   mergeResults,
   publishMetaData,
   finalizeRuleResult,
-  DqElement,
+  nodeSerializer,
   clone
 } from '../utils';
 
@@ -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);
   });
 }
 

diff --git a/lib/core/public/load.js b/lib/core/public/load.js
@@ -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
@@ -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();

diff --git a/lib/core/public/run-partial.js b/lib/core/public/run-partial.js
@@ -1,7 +1,7 @@
 import Context from '../base/context';
 import teardown from './teardown';
 import {
-  DqElement,
+  nodeSerializer,
   getSelectorData,
   assert,
   getEnvironmentData
@@ -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;
+
   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) {
@@ -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;
-}
diff --git a/lib/core/reporters/helpers/process-aggregate.js b/lib/core/reporters/helpers/process-aggregate.js
@@ -1,4 +1,5 @@
 import constants from '../../constants';
+import { nodeSerializer } from '../../utils';
 
 const resultKeys = constants.resultGroups;
 
@@ -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;
@@ -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) {
+  // 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;
+}
diff --git a/lib/core/reporters/raw.js b/lib/core/reporters/raw.js
@@ -1,3 +1,5 @@
+import { nodeSerializer } from '../utils';
+
 const rawReporter = (results, options, callback) => {
   if (typeof options === 'function') {
     callback = options;
@@ -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;

diff --git a/lib/core/utils/check-helper.js b/lib/core/utils/check-helper.js
@@ -43,7 +43,7 @@ function checkHelper(checkResult, options, resolve, reject) {
           node = node.actualNode;
         }
         if (node instanceof window.Node) {
-          const dqElm = new DqElement(node, options);
+          const dqElm = new DqElement(node);
           checkResult.relatedNodes.push(dqElm);
         }
       });

diff --git a/lib/core/utils/collect-results-from-frames.js b/lib/core/utils/collect-results-from-frames.js
@@ -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;
 

diff --git a/lib/core/utils/dq-element.js b/lib/core/utils/dq-element.js
@@ -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;
@@ -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;
@@ -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 };
   }
@@ -106,14 +116,24 @@ 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;
   }
 };
 
@@ -122,14 +142,29 @@ DqElement.fromFrame = function fromFrame(node, options, 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;
diff --git a/lib/core/utils/index.js b/lib/core/utils/index.js
@@ -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';