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 <[email protected]>

* Update lib/core/utils/dq-element.js

* Update lib/core/utils/node-serializer.js

Co-authored-by: Steven Lambert <[email protected]>

---------

Co-authored-by: Wilco Fiers <[email protected]>
Co-authored-by: Wilco Fiers <[email protected]>
Co-authored-by: Steven Lambert <[email protected]>

Author: 3 people

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.

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 = [];

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;
   }

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

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);
   });
 }
 

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();

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;
-}

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;
+}

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;

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);
         }
       });