From 0e167158ee48466d502d43677bdcc580959aebf9 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Tue, 22 Mar 2022 08:19:45 -0600 Subject: [PATCH 01/12] fix(utils): greatly improve the speed of our querySelectorAll code --- lib/core/base/virtual-node/virtual-node.js | 3 + lib/core/utils/matches.js | 4 + lib/core/utils/query-selector-all-filter.js | 12 +++ lib/core/utils/selector-cache.js | 103 ++++++++++++++++++++ 4 files changed, 122 insertions(+) create mode 100644 lib/core/utils/selector-cache.js diff --git a/lib/core/base/virtual-node/virtual-node.js b/lib/core/base/virtual-node/virtual-node.js index 4235b23c20..d546b84dc9 100644 --- a/lib/core/base/virtual-node/virtual-node.js +++ b/lib/core/base/virtual-node/virtual-node.js @@ -2,6 +2,7 @@ import AbstractVirtualNode from './abstract-virtual-node'; import { isXHTML, validInputTypes } from '../../utils'; import { isFocusable, getTabbableElements } from '../../../commons/dom'; import cache from '../cache'; +import { cacheNodeSelectors } from '../../utils/selector-cache'; let isXHTMLGlobal; let nodeIndex = 0; @@ -50,6 +51,8 @@ class VirtualNode extends AbstractVirtualNode { if (cache.get('nodeMap')) { cache.get('nodeMap').set(node, this); } + + cacheNodeSelectors(this); } // abstract Node properties so we can run axe in DOM-less environments. diff --git a/lib/core/utils/matches.js b/lib/core/utils/matches.js index ee1461498d..54884b9d86 100644 --- a/lib/core/utils/matches.js +++ b/lib/core/utils/matches.js @@ -224,6 +224,10 @@ export function convertSelector(selector) { * @returns {Boolean} */ function optimizedMatchesExpression(vNode, expressions, index, matchAnyParent) { + if (!vNode) { + return false; + } + const isArray = Array.isArray(expressions); const expression = isArray ? expressions[index] : expressions; let matches = matchExpression(vNode, expression); diff --git a/lib/core/utils/query-selector-all-filter.js b/lib/core/utils/query-selector-all-filter.js index b4b59d71ff..84fcf02313 100644 --- a/lib/core/utils/query-selector-all-filter.js +++ b/lib/core/utils/query-selector-all-filter.js @@ -1,4 +1,5 @@ import { matchesExpression, convertSelector } from './matches'; +import { getNodesMatchingSelector } from './selector-cache'; function createLocalVariables( vNodes, @@ -123,6 +124,17 @@ function matchExpressions(domTree, expressions, filter) { */ function querySelectorAllFilter(domTree, selector, filter) { domTree = Array.isArray(domTree) ? domTree : [domTree]; + + // see if the passed in node is the root node of the tree and can + // find nodes using the cache rather than looping through the + // the entire tree + const nodes = getNodesMatchingSelector(domTree, selector, filter); + if (nodes) { + return nodes; + } + + // if the selector cache is not set up or if not passed the + // top level node we default back to parsing the whole tree const expressions = convertSelector(selector); return matchExpressions(domTree, expressions, filter); } diff --git a/lib/core/utils/selector-cache.js b/lib/core/utils/selector-cache.js new file mode 100644 index 0000000000..d1e3b36a94 --- /dev/null +++ b/lib/core/utils/selector-cache.js @@ -0,0 +1,103 @@ +import { convertSelector, matchesExpression } from './matches'; + +let selectorMap = {}; + +function cacheSelector(key, vNode) { + selectorMap[key] = selectorMap[key] || []; + selectorMap[key].push(vNode); +} + +/** + * Cache selector information about a VirtalNode + * @param {VirtualNode} vNode + */ +export function cacheNodeSelectors(vNode) { + if (vNode.props.nodeType !== 1) { + return; + } + + // cache the selector map to the root node of the tree + if (vNode.nodeIndex === 0) { + selectorMap = {}; + vNode._selectorMap = selectorMap; + } + + cacheSelector(vNode.props.nodeName, vNode); + cacheSelector('*', vNode); + + vNode.attrNames.forEach(attrName => { + cacheSelector(`[${attrName}]`, vNode); + }); +} + +/** + * Get nodes from the selector cache that match the selector + * @param {VirtualTree[]} domTree flattened tree collection to search + * @param {String} selector + * @param {Function} filter function (optional) + * @return {Mixed} Array of nodes that match the selector or undefined if the selector map is not setup + */ +export function getNodesMatchingSelector(domTree, selector, filter) { + // check to see if the domTree is the root and has the selector + // map. if not we just return and let our QSA code do the finding + const selectorMap = domTree[0]._selectorMap; + if (!selectorMap) { + return; + } + + const shadowId = domTree[0].shadowId; + const expressions = convertSelector(selector); + let matchedNodes = []; + + // find nodes that match just a part of the selector in order + // to speed up traversing the entire tree + expressions.forEach(expression => { + // use the last part of the expression to find nodes as it's more + // specific. e.g. for `body h1` use `h1` and not `body` + const exp = expression[expression.length - 1]; + + // the expression `[id]` will use `*` as the tag name + const isGlobalSelector = + exp.tag === '*' && !exp.attributes && !exp.id && !exp.classes; + let nodes = []; + + if (isGlobalSelector && selectorMap['*']) { + nodes = selectorMap['*']; + } + // for `h1[role]` we want to use the tag name as it is more + // specific than using all nodes with the role attribute + else if (exp.tag && exp.tag !== '*' && selectorMap[exp.tag]) { + nodes = selectorMap[exp.tag]; + } else if (exp.id && selectorMap['[id]']) { + // when using id selector (#one) we should only select nodes + // that match the shadowId of the root + nodes = selectorMap['[id]'].filter(node => node.shadowId === shadowId); + } else if (exp.classes && selectorMap['[class]']) { + nodes = selectorMap['[class]']; + } else if (exp.attributes) { + // break once we find nodes that match any of the attributes + for (let i = 0; i < exp.attributes.length; i++) { + const attrName = exp.attributes[i].key; + if (selectorMap['['.concat(attrName, ']')]) { + nodes = selectorMap['['.concat(attrName, ']')]; + break; + } + } + } + + // now that we have a list of all nodes that match a part of + // the expression we need to check if the node actually matches + // the entire expression + nodes.forEach(node => { + if (matchesExpression(node, expression) && !matchedNodes.includes(node)) { + matchedNodes.push(node); + } + }); + }); + + if (filter) { + matchedNodes = matchedNodes.filter(filter); + } + + return matchedNodes.sort((a, b) => a.nodeIndex - b.nodeIndex); +} From f6b9bc01171406f960d9a6796dc98437fd57bb4e Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Wed, 23 Mar 2022 17:13:57 -0600 Subject: [PATCH 02/12] fixes --- lib/core/utils/selector-cache.js | 179 +++++++++++++++++++++++++------ 1 file changed, 149 insertions(+), 30 deletions(-) diff --git a/lib/core/utils/selector-cache.js b/lib/core/utils/selector-cache.js index d1e3b36a94..8de84e424c 100644 --- a/lib/core/utils/selector-cache.js +++ b/lib/core/utils/selector-cache.js @@ -1,10 +1,29 @@ import { convertSelector, matchesExpression } from './matches'; +import tokenList from './token-list'; +// since attribute names can't contain whitespace, this will be +// a reserved list for ids +const idsKey = ' [idsMap]'; let selectorMap = {}; -function cacheSelector(key, vNode) { - selectorMap[key] = selectorMap[key] || []; - selectorMap[key].push(vNode); +function isGlobalSelector(exp) { + return exp.tag === '*' && !exp.attributes && !exp.id && !exp.classes; +} + +function cacheSelector(key, vNode, map = selectorMap) { + map[key] = map[key] || []; + map[key].push(vNode); +} + +/** + * Inner join two arrays. + */ +function innerJoin(a, b) { + if (!b.length) { + return a; + } + + return a.filter(node => b.includes(node)); } /** @@ -26,6 +45,14 @@ export function cacheNodeSelectors(vNode) { cacheSelector('*', vNode); vNode.attrNames.forEach(attrName => { + // element ids are the only values we'll match + if (attrName === 'id') { + selectorMap[idsKey] = selectorMap[idsKey] || {}; + tokenList(vNode.attr(attrName)).forEach(value => { + cacheSelector(value, vNode, selectorMap[idsKey]); + }); + } + cacheSelector(`[${attrName}]`, vNode); }); } @@ -47,6 +74,19 @@ export function getNodesMatchingSelector(domTree, selector, filter) { const shadowId = domTree[0].shadowId; const expressions = convertSelector(selector); + + // if the selector uses a global selector with a combinator + // (A *, A > *) it's actually faster to use our QSA code than + // getting all nodes and using matchesExpression + for (let i = 0; i < expressions.length; i++) { + if ( + expressions[i].length > 1 && + expressions[i].some(expression => isGlobalSelector(expression)) + ) { + return; + } + } + let matchedNodes = []; // find nodes that match just a part of the selector in order @@ -55,44 +95,123 @@ export function getNodesMatchingSelector(domTree, selector, filter) { // use the last part of the expression to find nodes as it's more // specific. e.g. for `body h1` use `h1` and not `body` const exp = expression[expression.length - 1]; - - // the expression `[id]` will use `*` as the tag name - const isGlobalSelector = - exp.tag === '*' && !exp.attributes && !exp.id && !exp.classes; let nodes = []; - if (isGlobalSelector && selectorMap['*']) { + // a complex selector is one that will require using + // matchesExpression to determine if it matches. these include + // pseudo selectors (:not), combinators (A > B), and any + // attribute value ([class=.foo]). + let isComplexSelector = expression.length > 1 || exp.pseudos || exp.classes; + + if (isGlobalSelector(exp) && selectorMap['*']) { nodes = selectorMap['*']; - } - // for `h1[role]` we want to use the tag name as it is more - // specific than using all nodes with the role attribute - else if (exp.tag && exp.tag !== '*' && selectorMap[exp.tag]) { - nodes = selectorMap[exp.tag]; - } else if (exp.id && selectorMap['[id]']) { - // when using id selector (#one) we should only select nodes - // that match the shadowId of the root - nodes = selectorMap['[id]'].filter(node => node.shadowId === shadowId); - } else if (exp.classes && selectorMap['[class]']) { - nodes = selectorMap['[class]']; - } else if (exp.attributes) { - // break once we find nodes that match any of the attributes - for (let i = 0; i < exp.attributes.length; i++) { - const attrName = exp.attributes[i].key; - if (selectorMap['['.concat(attrName, ']')]) { - nodes = selectorMap['['.concat(attrName, ']')]; - break; + } else { + // a selector must match all parts, otherwise we can just exit + // early + if (exp.id) { + if (!selectorMap[idsKey] || !selectorMap[idsKey][exp.id]?.length) { + return; + } + + // when using id selector (#one) we should only select nodes + // that match the shadowId of the root + nodes = selectorMap[idsKey][exp.id].filter( + node => node.shadowId === shadowId + ); + } + + if (exp.tag && exp.tag !== '*') { + if (!selectorMap[exp.tag]?.length) { + return; + } + + nodes = innerJoin(selectorMap[exp.tag], nodes); + } + + if (exp.classes) { + if (!selectorMap['[class]']?.length) { + return; + } + + nodes = innerJoin(selectorMap[exp.classes], nodes); + } + + if (exp.attributes) { + for (let i = 0; i < exp.attributes.length; i++) { + const attr = exp.attributes[i]; + + // to test if the attribute is looking for existence of + // the attribute or a specific value, we need to use + // the "test" function (since the expression doesn't + // differentiate between [href] and [href=""]). + // since specific values use string matching, passing + // the boolean true will only return true for existence + // tests. + // a specific value check is a complex selector + if (!attr.test(true)) { + isComplexSelector = true; + } + + if (!selectorMap[`[${attr.key}]`]?.length) { + return; + } + + nodes = innerJoin(selectorMap[`[${attr.key}]`], nodes); } } } - // now that we have a list of all nodes that match a part of - // the expression we need to check if the node actually matches - // the entire expression nodes.forEach(node => { - if (matchesExpression(node, expression) && !matchedNodes.includes(node)) { + if (isComplexSelector && !matchesExpression(node, expression)) { + return; + } + + if (!matchedNodes.includes(node)) { matchedNodes.push(node); } }); + + // // use the last part of the expression to find nodes as it's more + // // specific. e.g. for `body h1` use `h1` and not `body` + // const exp = expression[expression.length - 1]; + + // // the expression `[id]` will use `*` as the tag name + // const isGlobalSelector = + // exp.tag === '*' && !exp.attributes && !exp.id && !exp.classes; + // let nodes = []; + + // if (isGlobalSelector && selectorMap['*']) { + // nodes = selectorMap['*']; + // } + // // for `h1[role]` we want to use the tag name as it is more + // // specific than using all nodes with the role attribute + // else if (exp.tag && exp.tag !== '*' && selectorMap[exp.tag]) { + // nodes = selectorMap[exp.tag]; + // } else if (exp.id && selectorMap['[id]']) { + // // when using id selector (#one) we should only select nodes + // // that match the shadowId of the root + // nodes = selectorMap['[id]'].filter(node => node.shadowId === shadowId); + // } else if (exp.classes && selectorMap['[class]']) { + // nodes = selectorMap['[class]']; + // } else if (exp.attributes) { + // // break once we find nodes that match any of the attributes + // for (let i = 0; i < exp.attributes.length; i++) { + // const attrName = exp.attributes[i].key; + // if (selectorMap['['.concat(attrName, ']')]) { + // nodes = selectorMap['['.concat(attrName, ']')]; + // break; + // } + // } + // } + + // // now that we have a list of all nodes that match a part of + // // the expression we need to check if the node actually matches + // // the entire expression + // nodes.forEach(node => { + // if (matchesExpression(node, expression) && !matchedNodes.includes(node)) { + // matchedNodes.push(node); + // } + // }); }); if (filter) { From 785ce336aca61258d3bef073dcdf31fbaeb7bf77 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Wed, 23 Mar 2022 17:54:01 -0600 Subject: [PATCH 03/12] finalize --- lib/core/utils/selector-cache.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/core/utils/selector-cache.js b/lib/core/utils/selector-cache.js index 8de84e424c..4435b2553c 100644 --- a/lib/core/utils/selector-cache.js +++ b/lib/core/utils/selector-cache.js @@ -87,7 +87,7 @@ export function getNodesMatchingSelector(domTree, selector, filter) { } } - let matchedNodes = []; + let matchedNodes = new Set(); // find nodes that match just a part of the selector in order // to speed up traversing the entire tree @@ -166,9 +166,10 @@ export function getNodesMatchingSelector(domTree, selector, filter) { return; } - if (!matchedNodes.includes(node)) { - matchedNodes.push(node); - } + matchedNodes.add(node); + // if (!matchedNodes.includes(node)) { + // matchedNodes.push(node); + // } }); // // use the last part of the expression to find nodes as it's more @@ -214,6 +215,8 @@ export function getNodesMatchingSelector(domTree, selector, filter) { // }); }); + matchedNodes = Array.from(matchedNodes); + if (filter) { matchedNodes = matchedNodes.filter(filter); } From 75e29da34aba70770e98a13d492a4b018fcb0b74 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Thu, 24 Mar 2022 16:22:38 -0600 Subject: [PATCH 04/12] tests --- lib/core/core.js | 8 + lib/core/utils/matches.js | 1 + lib/core/utils/query-selector-all-filter.js | 6 +- lib/core/utils/selector-cache.js | 108 ++--- test/core/base/virtual-node/virtual-node.js | 14 + test/core/utils/qsa.js | 414 +++++++++++--------- test/core/utils/selector-cache.js | 242 ++++++++++++ 7 files changed, 537 insertions(+), 256 deletions(-) create mode 100644 test/core/utils/selector-cache.js diff --git a/lib/core/core.js b/lib/core/core.js index 7664e8b676..c3e5717fb9 100644 --- a/lib/core/core.js +++ b/lib/core/core.js @@ -46,6 +46,10 @@ import v2Reporter from './reporters/v2'; import * as commons from '../commons'; import * as utils from './utils'; +import { + cacheNodeSelectors, + getNodesMatchingExpression +} from './utils/selector-cache'; axe.constants = constants; axe.log = log; @@ -70,6 +74,10 @@ axe._thisWillBeDeletedDoNotUse.base = { axe._thisWillBeDeletedDoNotUse.public = { reporters }; +axe._thisWillBeDeletedDoNotUse.utils = + axe._thisWillBeDeletedDoNotUse.utils || {}; +axe._thisWillBeDeletedDoNotUse.utils.cacheNodeSelectors = cacheNodeSelectors; +axe._thisWillBeDeletedDoNotUse.utils.getNodesMatchingExpression = getNodesMatchingExpression; axe.imports = imports; diff --git a/lib/core/utils/matches.js b/lib/core/utils/matches.js index 54884b9d86..412dfcf837 100644 --- a/lib/core/utils/matches.js +++ b/lib/core/utils/matches.js @@ -128,6 +128,7 @@ function convertAttributes(atts) { return { key: attributeKey, value: attributeValue, + type: typeof att.value === 'undefined' ? 'attrExist' : 'attrValue', test: test }; }); diff --git a/lib/core/utils/query-selector-all-filter.js b/lib/core/utils/query-selector-all-filter.js index 84fcf02313..770e3651c1 100644 --- a/lib/core/utils/query-selector-all-filter.js +++ b/lib/core/utils/query-selector-all-filter.js @@ -1,5 +1,5 @@ import { matchesExpression, convertSelector } from './matches'; -import { getNodesMatchingSelector } from './selector-cache'; +import { getNodesMatchingExpression } from './selector-cache'; function createLocalVariables( vNodes, @@ -124,18 +124,18 @@ function matchExpressions(domTree, expressions, filter) { */ function querySelectorAllFilter(domTree, selector, filter) { domTree = Array.isArray(domTree) ? domTree : [domTree]; + const expressions = convertSelector(selector); // see if the passed in node is the root node of the tree and can // find nodes using the cache rather than looping through the // the entire tree - const nodes = getNodesMatchingSelector(domTree, selector, filter); + const nodes = getNodesMatchingExpression(domTree, expressions, filter); if (nodes) { return nodes; } // if the selector cache is not set up or if not passed the // top level node we default back to parsing the whole tree - const expressions = convertSelector(selector); return matchExpressions(domTree, expressions, filter); } diff --git a/lib/core/utils/selector-cache.js b/lib/core/utils/selector-cache.js index 4435b2553c..d5ad44518c 100644 --- a/lib/core/utils/selector-cache.js +++ b/lib/core/utils/selector-cache.js @@ -1,22 +1,36 @@ -import { convertSelector, matchesExpression } from './matches'; +import { matchesExpression } from './matches'; import tokenList from './token-list'; // since attribute names can't contain whitespace, this will be -// a reserved list for ids +// a reserved list for ids so we can perform virtual id lookups const idsKey = ' [idsMap]'; let selectorMap = {}; +/** + * Non-tag selectors use `*` for the tag name so a global selector won't have any other properties of the expression. + * @param {Object} exp Selector Expression + * @returns {Boolean} + */ function isGlobalSelector(exp) { return exp.tag === '*' && !exp.attributes && !exp.id && !exp.classes; } +/** + * Save a selector and vNode to the selectorMap + * @param {String} key + * @param {VirtualNode} vNode + * @param {Object} map + */ function cacheSelector(key, vNode, map = selectorMap) { map[key] = map[key] || []; map[key].push(vNode); } /** - * Inner join two arrays. + * Inner join two arrays (find all nodes in a that are also in b). + * @param {*[]} a + * @param {*[]} b + * @returns {*[]} */ function innerJoin(a, b) { if (!b.length) { @@ -27,7 +41,7 @@ function innerJoin(a, b) { } /** - * Cache selector information about a VirtalNode + * Cache selector information about a VirtalNode. * @param {VirtualNode} vNode */ export function cacheNodeSelectors(vNode) { @@ -58,13 +72,13 @@ export function cacheNodeSelectors(vNode) { } /** - * Get nodes from the selector cache that match the selector + * Get nodes from the selector cache that match the selector. * @param {VirtualTree[]} domTree flattened tree collection to search - * @param {String} selector + * @param {Object} expressions * @param {Function} filter function (optional) * @return {Mixed} Array of nodes that match the selector or undefined if the selector map is not setup */ -export function getNodesMatchingSelector(domTree, selector, filter) { +export function getNodesMatchingExpression(domTree, expressions, filter) { // check to see if the domTree is the root and has the selector // map. if not we just return and let our QSA code do the finding const selectorMap = domTree[0]._selectorMap; @@ -73,10 +87,9 @@ export function getNodesMatchingSelector(domTree, selector, filter) { } const shadowId = domTree[0].shadowId; - const expressions = convertSelector(selector); // if the selector uses a global selector with a combinator - // (A *, A > *) it's actually faster to use our QSA code than + // (e.g. A *, A > *) it's actually faster to use our QSA code than // getting all nodes and using matchesExpression for (let i = 0; i < expressions.length; i++) { if ( @@ -100,21 +113,22 @@ export function getNodesMatchingSelector(domTree, selector, filter) { // a complex selector is one that will require using // matchesExpression to determine if it matches. these include // pseudo selectors (:not), combinators (A > B), and any - // attribute value ([class=.foo]). - let isComplexSelector = expression.length > 1 || exp.pseudos || exp.classes; + // attribute value ([class=foo]). + let isComplexSelector = + expression.length > 1 || !!exp.pseudos || !!exp.classes; - if (isGlobalSelector(exp) && selectorMap['*']) { + if (isGlobalSelector(exp)) { nodes = selectorMap['*']; } else { - // a selector must match all parts, otherwise we can just exit - // early if (exp.id) { + // a selector must match all parts, otherwise we can just exit + // early if (!selectorMap[idsKey] || !selectorMap[idsKey][exp.id]?.length) { return; } - // when using id selector (#one) we should only select nodes - // that match the shadowId of the root + // when using id selector (#one) we only find nodes that + // match the shadowId of the root nodes = selectorMap[idsKey][exp.id].filter( node => node.shadowId === shadowId ); @@ -133,22 +147,16 @@ export function getNodesMatchingSelector(domTree, selector, filter) { return; } - nodes = innerJoin(selectorMap[exp.classes], nodes); + nodes = innerJoin(selectorMap['[class]'], nodes); } if (exp.attributes) { for (let i = 0; i < exp.attributes.length; i++) { const attr = exp.attributes[i]; - // to test if the attribute is looking for existence of - // the attribute or a specific value, we need to use - // the "test" function (since the expression doesn't - // differentiate between [href] and [href=""]). - // since specific values use string matching, passing - // the boolean true will only return true for existence - // tests. - // a specific value check is a complex selector - if (!attr.test(true)) { + // an attribute selector that looks for a specific value is + // a complex selector + if (attr.type === 'attrValue') { isComplexSelector = true; } @@ -162,57 +170,15 @@ export function getNodesMatchingSelector(domTree, selector, filter) { } nodes.forEach(node => { + // for complex selectors we need to verify that the node + // actually matches the entire selector since we only have + // nodes that partially match the last part of the selector if (isComplexSelector && !matchesExpression(node, expression)) { return; } matchedNodes.add(node); - // if (!matchedNodes.includes(node)) { - // matchedNodes.push(node); - // } }); - - // // use the last part of the expression to find nodes as it's more - // // specific. e.g. for `body h1` use `h1` and not `body` - // const exp = expression[expression.length - 1]; - - // // the expression `[id]` will use `*` as the tag name - // const isGlobalSelector = - // exp.tag === '*' && !exp.attributes && !exp.id && !exp.classes; - // let nodes = []; - - // if (isGlobalSelector && selectorMap['*']) { - // nodes = selectorMap['*']; - // } - // // for `h1[role]` we want to use the tag name as it is more - // // specific than using all nodes with the role attribute - // else if (exp.tag && exp.tag !== '*' && selectorMap[exp.tag]) { - // nodes = selectorMap[exp.tag]; - // } else if (exp.id && selectorMap['[id]']) { - // // when using id selector (#one) we should only select nodes - // // that match the shadowId of the root - // nodes = selectorMap['[id]'].filter(node => node.shadowId === shadowId); - // } else if (exp.classes && selectorMap['[class]']) { - // nodes = selectorMap['[class]']; - // } else if (exp.attributes) { - // // break once we find nodes that match any of the attributes - // for (let i = 0; i < exp.attributes.length; i++) { - // const attrName = exp.attributes[i].key; - // if (selectorMap['['.concat(attrName, ']')]) { - // nodes = selectorMap['['.concat(attrName, ']')]; - // break; - // } - // } - // } - - // // now that we have a list of all nodes that match a part of - // // the expression we need to check if the node actually matches - // // the entire expression - // nodes.forEach(node => { - // if (matchesExpression(node, expression) && !matchedNodes.includes(node)) { - // matchedNodes.push(node); - // } - // }); }); matchedNodes = Array.from(matchedNodes); diff --git a/test/core/base/virtual-node/virtual-node.js b/test/core/base/virtual-node/virtual-node.js index 976035e942..564e046eba 100644 --- a/test/core/base/virtual-node/virtual-node.js +++ b/test/core/base/virtual-node/virtual-node.js @@ -81,6 +81,20 @@ describe('VirtualNode', function() { assert.equal(vNode.props.nodeName, 'foobar'); }); + it('should add selectorMap to root element', function() { + node.setAttribute('data-foo', 'bar'); + var vNode = new VirtualNode(node); + + assert.exists(vNode._selectorMap); + }); + + it('should not add selectorMap to non-root element', function() { + node.setAttribute('data-foo', 'bar'); + var vNode = new VirtualNode(node, new VirtualNode(node.cloneNode())); + + assert.notExists(vNode._selectorMap); + }); + describe('attr', function() { it('should return the value of the given attribute', function() { node.setAttribute('data-foo', 'bar'); diff --git a/test/core/utils/qsa.js b/test/core/utils/qsa.js index db5dd63f34..4ff2cb0d1e 100644 --- a/test/core/utils/qsa.js +++ b/test/core/utils/qsa.js @@ -57,190 +57,240 @@ describe('axe.utils.querySelectorAllFilter', function() { 'use strict'; var dom; afterEach(function() {}); - beforeEach(function() { - dom = getTestDom(); - }); - it('should find nodes using just the tag', function() { - var result = axe.utils.querySelectorAllFilter(dom, 'li'); - assert.equal(result.length, 4); - }); - it('should find nodes using parent selector', function() { - var result = axe.utils.querySelectorAllFilter(dom, 'ul > li'); - assert.equal(result.length, 4); - }); - it('should NOT find nodes using parent selector', function() { - var result = axe.utils.querySelectorAllFilter(dom, 'div > li'); - assert.equal(result.length, 0); - }); - it('should find nodes using nested parent selectors', function() { - var result = axe.utils.querySelectorAllFilter( - dom, - 'span > span > span > span' - ); - assert.equal(result.length, 2); - }); - it('should find nodes using hierarchical selector', function() { - var result = axe.utils.querySelectorAllFilter(dom, 'div li'); - assert.equal(result.length, 4); - }); - it('should find nodes using class selector', function() { - var result = axe.utils.querySelectorAllFilter(dom, '.breaking'); - assert.equal(result.length, 2); - }); - it('should find nodes using hierarchical class selector', function() { - var result = axe.utils.querySelectorAllFilter(dom, '.first .breaking'); - assert.equal(result.length, 2); - }); - it('should NOT find nodes using hierarchical class selector', function() { - var result = axe.utils.querySelectorAllFilter(dom, '.second .breaking'); - assert.equal(result.length, 0); - }); - it('should find nodes using multiple class selector', function() { - var result = axe.utils.querySelectorAllFilter(dom, '.second.third'); - assert.equal(result.length, 1); - }); - it('should find nodes using id', function() { - var result = axe.utils.querySelectorAllFilter(dom, '#one'); - assert.equal(result.length, 1); - }); - it('should find nodes using id, but not in shadow DOM', function() { - var result = axe.utils.querySelectorAllFilter(dom[0].children[0], '#one'); - assert.equal(result.length, 1); - }); - it('should find nodes using id, within a shadow DOM', function() { - var result = axe.utils.querySelectorAllFilter( - dom[0].children[0].children[2], - '#one' - ); - assert.equal(result.length, 1); - }); - it('should find nodes using attribute', function() { - var result = axe.utils.querySelectorAllFilter(dom, '[role]'); - assert.equal(result.length, 2); - }); - it('should find nodes using attribute with value', function() { - var result = axe.utils.querySelectorAllFilter(dom, '[role=tab]'); - assert.equal(result.length, 1); - }); - it('should find nodes using attribute with value', function() { - var result = axe.utils.querySelectorAllFilter(dom, '[role="button"]'); - assert.equal(result.length, 1); - }); - it('should find nodes using parent attribute with value', function() { - var result = axe.utils.querySelectorAllFilter( - dom, - '[data-a11yhero="faulkner"] > ul' - ); - assert.equal(result.length, 1); - }); - it('should find nodes using hierarchical attribute with value', function() { - var result = axe.utils.querySelectorAllFilter( - dom, - '[data-a11yhero="faulkner"] li' - ); - assert.equal(result.length, 2); - }); - it('should find nodes using :not selector with class', function() { - var result = axe.utils.querySelectorAllFilter(dom, 'div:not(.first)'); - assert.equal(result.length, 2); - }); - it('should find nodes using :not selector with matching id', function() { - var result = axe.utils.querySelectorAllFilter(dom, 'div:not(#one)'); - assert.equal(result.length, 2); - }); - it('should find nodes using :not selector with matching attribute selector', function() { - var result = axe.utils.querySelectorAllFilter( - dom, - 'div:not([data-a11yhero])' - ); - assert.equal(result.length, 2); - }); - it('should find nodes using :not selector with matching attribute selector with value', function() { - var result = axe.utils.querySelectorAllFilter( - dom, - 'div:not([data-a11yhero=faulkner])' - ); - assert.equal(result.length, 2); - }); - it('should find nodes using :not selector with bogus attribute selector with value', function() { - var result = axe.utils.querySelectorAllFilter( - dom, - 'div:not([data-a11yhero=wilco])' - ); - assert.equal(result.length, 3); - }); - it('should find nodes using :not selector with bogus id', function() { - var result = axe.utils.querySelectorAllFilter(dom, 'div:not(#thangy)'); - assert.equal(result.length, 3); - }); - it('should find nodes using :not selector with attribute', function() { - var result = axe.utils.querySelectorAllFilter(dom, 'div:not([id])'); - assert.equal(result.length, 2); - }); - it('should find nodes hierarchically using :not selector', function() { - var result = axe.utils.querySelectorAllFilter(dom, 'div:not(.first) li'); - assert.equal(result.length, 2); - }); - it('should find same nodes hierarchically using more :not selector', function() { - var result = axe.utils.querySelectorAllFilter( - dom, - 'div:not(.first) li:not(.breaking)' - ); - assert.equal(result.length, 2); - }); - it('should NOT find nodes hierarchically using :not selector', function() { - var result = axe.utils.querySelectorAllFilter( - dom, - 'div:not(.second) li:not(.breaking)' - ); - assert.equal(result.length, 0); - }); - it('should find nodes using ^= attribute selector', function() { - var result = axe.utils.querySelectorAllFilter(dom, '[class^="sec"]'); - assert.equal(result.length, 1); - }); - it('should find nodes using $= attribute selector', function() { - var result = axe.utils.querySelectorAllFilter(dom, '[id$="ne"]'); - assert.equal(result.length, 3); - }); - it('should find nodes using *= attribute selector', function() { - var result = axe.utils.querySelectorAllFilter(dom, '[role*="t"]'); - assert.equal(result.length, 2); - }); - it('should put it all together', function() { - var result = axe.utils.querySelectorAllFilter( - dom, - '.first[data-a11yhero="faulkner"] > ul li.breaking' - ); - assert.equal(result.length, 2); - }); - it('should find an element only once', function() { - var divs = axe.utils.querySelectorAllFilter(dom, 'div'); - var ones = axe.utils.querySelectorAllFilter(dom, '#one'); - var divOnes = axe.utils.querySelectorAllFilter(dom, 'div, #one'); - - assert.isBelow( - divOnes.length, - divs.length + ones.length, - 'Elements matching both parts of a selector should not be included twice' - ); - }); - it('should return nodes sorted by document position', function() { - var result = axe.utils.querySelectorAllFilter(dom, 'ul, #one'); - assert.equal(result[0].actualNode.nodeName, 'UL'); - assert.equal(result[1].actualNode.nodeName, 'DIV'); - assert.equal(result[2].actualNode.nodeName, 'UL'); - }); - it('should filter the returned nodes when passed a filter function', function() { - var result = axe.utils.querySelectorAllFilter(dom, 'ul, #one', function( - node - ) { - return node.actualNode.nodeName !== 'UL'; + + var tests = ['without cache', 'with cache']; + for (var i = 0; i < tests.length; i++) { + var describeName = tests[i]; + describe(describeName, function() { + afterEach(function() {}); + + if (describeName === 'without cache') { + beforeEach(function() { + dom = getTestDom(); + + // prove we're using the DOM by deleting the cache + delete dom[0]._selectorCache; + }); + + it('should not have a primed cache', function() { + assert.isUndefined(dom[0]._selectorCache); + }); + } else { + beforeEach(function() { + dom = getTestDom(); + + // prove we're using the cache by deleting all the children + dom[0].children = []; + }); + + it('should not use the cache if not using the top-level node', function() { + var nodes = axe.utils.querySelectorAllFilter(dom, 'ul'); + + // this would return 4 nodes if we were still using the + // top-level cache + var result = axe.utils.querySelectorAllFilter(nodes[0], 'li'); + assert.equal(result.length, 2); + }); + } + + it('should find nodes using just the tag', function() { + var result = axe.utils.querySelectorAllFilter(dom, 'li'); + assert.equal(result.length, 4); + }); + it('should find nodes using parent selector', function() { + var result = axe.utils.querySelectorAllFilter(dom, 'ul > li'); + assert.equal(result.length, 4); + }); + it('should NOT find nodes using parent selector', function() { + var result = axe.utils.querySelectorAllFilter(dom, 'div > li'); + assert.equal(result.length, 0); + }); + it('should find nodes using nested parent selectors', function() { + var result = axe.utils.querySelectorAllFilter( + dom, + 'span > span > span > span' + ); + assert.equal(result.length, 2); + }); + it('should find nodes using hierarchical selector', function() { + var result = axe.utils.querySelectorAllFilter(dom, 'div li'); + assert.equal(result.length, 4); + }); + it('should find nodes using class selector', function() { + var result = axe.utils.querySelectorAllFilter(dom, '.breaking'); + assert.equal(result.length, 2); + }); + it('should find nodes using hierarchical class selector', function() { + var result = axe.utils.querySelectorAllFilter(dom, '.first .breaking'); + assert.equal(result.length, 2); + }); + it('should NOT find nodes using hierarchical class selector', function() { + var result = axe.utils.querySelectorAllFilter(dom, '.second .breaking'); + assert.equal(result.length, 0); + }); + it('should find nodes using multiple class selector', function() { + var result = axe.utils.querySelectorAllFilter(dom, '.second.third'); + assert.equal(result.length, 1); + }); + it('should find nodes using id', function() { + var result = axe.utils.querySelectorAllFilter(dom, '#one'); + assert.equal(result.length, 1); + }); + + // can only select shadow dom nodes when we're not using the + // top-level node. but since the top-level node is the one + // with the cache, this only works when we are testing the full + // tree (i.e. without cache) + if (describeName === 'without cache') { + it('should find nodes using id, but not in shadow DOM', function() { + var result = axe.utils.querySelectorAllFilter( + dom[0].children[0], + '#one' + ); + assert.equal(result.length, 1); + }); + it('should find nodes using id, within a shadow DOM', function() { + var result = axe.utils.querySelectorAllFilter( + dom[0].children[0].children[2], + '#one' + ); + assert.equal(result.length, 1); + }); + } + + it('should find nodes using attribute', function() { + var result = axe.utils.querySelectorAllFilter(dom, '[role]'); + assert.equal(result.length, 2); + }); + it('should find nodes using attribute with value', function() { + var result = axe.utils.querySelectorAllFilter(dom, '[role=tab]'); + assert.equal(result.length, 1); + }); + it('should find nodes using attribute with value', function() { + var result = axe.utils.querySelectorAllFilter(dom, '[role="button"]'); + assert.equal(result.length, 1); + }); + it('should find nodes using parent attribute with value', function() { + var result = axe.utils.querySelectorAllFilter( + dom, + '[data-a11yhero="faulkner"] > ul' + ); + assert.equal(result.length, 1); + }); + it('should find nodes using hierarchical attribute with value', function() { + var result = axe.utils.querySelectorAllFilter( + dom, + '[data-a11yhero="faulkner"] li' + ); + assert.equal(result.length, 2); + }); + it('should find nodes using :not selector with class', function() { + var result = axe.utils.querySelectorAllFilter(dom, 'div:not(.first)'); + assert.equal(result.length, 2); + }); + it('should find nodes using :not selector with matching id', function() { + var result = axe.utils.querySelectorAllFilter(dom, 'div:not(#one)'); + assert.equal(result.length, 2); + }); + it('should find nodes using :not selector with matching attribute selector', function() { + var result = axe.utils.querySelectorAllFilter( + dom, + 'div:not([data-a11yhero])' + ); + assert.equal(result.length, 2); + }); + it('should find nodes using :not selector with matching attribute selector with value', function() { + var result = axe.utils.querySelectorAllFilter( + dom, + 'div:not([data-a11yhero=faulkner])' + ); + assert.equal(result.length, 2); + }); + it('should find nodes using :not selector with bogus attribute selector with value', function() { + var result = axe.utils.querySelectorAllFilter( + dom, + 'div:not([data-a11yhero=wilco])' + ); + assert.equal(result.length, 3); + }); + it('should find nodes using :not selector with bogus id', function() { + var result = axe.utils.querySelectorAllFilter(dom, 'div:not(#thangy)'); + assert.equal(result.length, 3); + }); + it('should find nodes using :not selector with attribute', function() { + var result = axe.utils.querySelectorAllFilter(dom, 'div:not([id])'); + assert.equal(result.length, 2); + }); + it('should find nodes hierarchically using :not selector', function() { + var result = axe.utils.querySelectorAllFilter( + dom, + 'div:not(.first) li' + ); + assert.equal(result.length, 2); + }); + it('should find same nodes hierarchically using more :not selector', function() { + var result = axe.utils.querySelectorAllFilter( + dom, + 'div:not(.first) li:not(.breaking)' + ); + assert.equal(result.length, 2); + }); + it('should NOT find nodes hierarchically using :not selector', function() { + var result = axe.utils.querySelectorAllFilter( + dom, + 'div:not(.second) li:not(.breaking)' + ); + assert.equal(result.length, 0); + }); + it('should find nodes using ^= attribute selector', function() { + var result = axe.utils.querySelectorAllFilter(dom, '[class^="sec"]'); + assert.equal(result.length, 1); + }); + it('should find nodes using $= attribute selector', function() { + var result = axe.utils.querySelectorAllFilter(dom, '[id$="ne"]'); + assert.equal(result.length, 3); + }); + it('should find nodes using *= attribute selector', function() { + var result = axe.utils.querySelectorAllFilter(dom, '[role*="t"]'); + assert.equal(result.length, 2); + }); + it('should put it all together', function() { + var result = axe.utils.querySelectorAllFilter( + dom, + '.first[data-a11yhero="faulkner"] > ul li.breaking' + ); + assert.equal(result.length, 2); + }); + it('should find an element only once', function() { + var divs = axe.utils.querySelectorAllFilter(dom, 'div'); + var ones = axe.utils.querySelectorAllFilter(dom, '#one'); + var divOnes = axe.utils.querySelectorAllFilter(dom, 'div, #one'); + + assert.isBelow( + divOnes.length, + divs.length + ones.length, + 'Elements matching both parts of a selector should not be included twice' + ); + }); + it('should return nodes sorted by document position', function() { + var result = axe.utils.querySelectorAllFilter(dom, 'ul, #one'); + assert.equal(result[0].actualNode.nodeName, 'UL'); + assert.equal(result[1].actualNode.nodeName, 'DIV'); + assert.equal(result[2].actualNode.nodeName, 'UL'); + }); + it('should filter the returned nodes when passed a filter function', function() { + var result = axe.utils.querySelectorAllFilter(dom, 'ul, #one', function( + node + ) { + return node.actualNode.nodeName !== 'UL'; + }); + assert.equal(result[0].actualNode.nodeName, 'DIV'); + assert.equal(result.length, 1); + }); }); - assert.equal(result[0].actualNode.nodeName, 'DIV'); - assert.equal(result.length, 1); - }); + } }); + describe('axe.utils.querySelectorAll', function() { 'use strict'; var dom; diff --git a/test/core/utils/selector-cache.js b/test/core/utils/selector-cache.js new file mode 100644 index 0000000000..56e5de7246 --- /dev/null +++ b/test/core/utils/selector-cache.js @@ -0,0 +1,242 @@ +describe('utils.selector-cache', function() { + var fixture = document.querySelector('#fixture'); + var cacheNodeSelectors = + axe._thisWillBeDeletedDoNotUse.utils.cacheNodeSelectors; + var getNodesMatchingExpression = + axe._thisWillBeDeletedDoNotUse.utils.getNodesMatchingExpression; + var convertSelector = axe.utils.convertSelector; + + var vNode; + beforeEach(function() { + fixture.innerHTML = '
'; + vNode = new axe.VirtualNode(fixture.firstChild); + }); + + afterEach(function() { + fixture.innerHTML = ''; + }); + + describe('cacheNodeSelectors', function() { + it('should add the node to the global selector', function() { + cacheNodeSelectors(vNode); + assert.deepEqual(vNode._selectorMap['*'], [vNode]); + }); + + it('should add the node to the nodeName', function() { + cacheNodeSelectors(vNode); + assert.deepEqual(vNode._selectorMap.div, [vNode]); + }); + + it('should add the node to all attribute selectors', function() { + cacheNodeSelectors(vNode); + assert.deepEqual(vNode._selectorMap['[id]'], [vNode]); + assert.deepEqual(vNode._selectorMap['[class]'], [vNode]); + assert.deepEqual(vNode._selectorMap['[aria-label]'], [vNode]); + }); + + it('should add the node to the id map', function() { + cacheNodeSelectors(vNode); + assert.deepEqual(vNode._selectorMap[' [idsMap]'].target, [vNode]); + }); + + it('should not add the node to selectors it does not match', function() { + cacheNodeSelectors(vNode); + assert.isUndefined(vNode._selectorMap['[for]']); + assert.isUndefined(vNode._selectorMap.h1); + }); + + it('should ignore non-element nodes', function() { + fixture.innerHTML = 'Hello'; + vNode = new axe.VirtualNode(fixture.firstChild); + cacheNodeSelectors(vNode); + + assert.isUndefined(vNode._selectorMap); + }); + }); + + describe('getNodesMatchingExpression', function() { + var tree; + var spanVNode; + + function createTree() { + var parent = document.createElement('section'); + var parentVNode = new axe.VirtualNode(parent); + tree = [parentVNode]; + var nodes = []; + + for (var i = 0; i < fixture.children.length; i++) { + var child = fixture.children[i]; + var isShadow = child.hasAttribute('data-shadow'); + var childVNode = new axe.VirtualNode( + child, + parentVNode, + isShadow ? i : undefined + ); + parentVNode.children.push(child); + nodes.push(childVNode); + } + + return nodes; + } + + beforeEach(function() { + var span = document.createElement('span'); + span.setAttribute('class', 'bar'); + span.setAttribute('id', 'notTarget'); + span.setAttribute('aria-labelledby', 'target'); + spanVNode = new axe.VirtualNode(span, vNode); + vNode.children.push(spanVNode); + tree = [vNode]; + }); + + it('should return undefined if the cache is not primed', function() { + tree[0]._selectorMap = null; + var expression = convertSelector('div'); + assert.isUndefined(getNodesMatchingExpression(tree, expression)); + }); + + it('should return a list of matching nodes by global selector', function() { + var expression = convertSelector('*'); + assert.deepEqual(getNodesMatchingExpression(tree, expression), [ + vNode, + spanVNode + ]); + }); + + it('should return a list of matching nodes by nodeName', function() { + var expression = convertSelector('div'); + assert.deepEqual(getNodesMatchingExpression(tree, expression), [vNode]); + }); + + it('should return a list of matching nodes by id', function() { + var expression = convertSelector('#target'); + assert.deepEqual(getNodesMatchingExpression(tree, expression), [vNode]); + }); + + it('should only return nodes matching shadowId when matching by id', function() { + fixture.innerHTML = + '
'; + var nodes = createTree(); + var expression = convertSelector('#target'); + assert.deepEqual(getNodesMatchingExpression(tree, expression), [ + nodes[0] + ]); + }); + + it('should return a list of matching nodes by class', function() { + var expression = convertSelector('.foo'); + assert.deepEqual(getNodesMatchingExpression(tree, expression), [vNode]); + }); + + it('should return a list of matching nodes by attribute', function() { + var expression = convertSelector('[aria-label]'); + assert.deepEqual(getNodesMatchingExpression(tree, expression), [vNode]); + }); + + it('should return an empty array if selector does not match', function() { + var expression = convertSelector('h1'); + assert.lengthOf(getNodesMatchingExpression(tree, expression), 0); + }); + + it('should return nodes for each expression', function() { + fixture.innerHTML = + '
'; + var nodes = createTree(); + var expression = convertSelector('[role], [id]'); + assert.deepEqual(getNodesMatchingExpression(tree, expression), nodes); + }); + + it('should return nodes for child combinator selector', function() { + var expression = convertSelector('div span'); + assert.deepEqual(getNodesMatchingExpression(tree, expression), [ + spanVNode + ]); + }); + + it('should return nodes for direct child combinator selector', function() { + var expression = convertSelector('div > span'); + assert.deepEqual(getNodesMatchingExpression(tree, expression), [ + spanVNode + ]); + }); + + it('should return nodes for attribute value selector', function() { + var expression = convertSelector('[id="target"]'); + assert.deepEqual(getNodesMatchingExpression(tree, expression), [vNode]); + }); + + it('should return undefined for combinator selector with global selector', function() { + var expression = convertSelector('body *'); + assert.isUndefined(getNodesMatchingExpression(tree, expression)); + }); + + it('should return nodes for multipart selectors', function() { + var expression = convertSelector('div.foo[id]'); + assert.deepEqual(getNodesMatchingExpression(tree, expression), [vNode]); + }); + + it('should remove duplicates', function() { + fixture.innerHTML = '
'; + var nodes = createTree(); + var expression = convertSelector('div, [aria-label]'); + assert.deepEqual(getNodesMatchingExpression(tree, expression), nodes); + }); + + it('should sort nodes by added order', function() { + fixture.innerHTML = + '
' + + '' + + '
' + + '' + + '
' + + '' + + '
' + + '' + + '
' + + ''; + createTree(); + + var expression = convertSelector('div, span'); + var nodes = getNodesMatchingExpression(tree, expression); + var ids = []; + for (var i = 0; i < nodes.length; i++) { + ids.push(nodes[i].attr('id')); + } + + assert.deepEqual(ids, [ + 'id0', + 'id1', + 'id2', + 'id3', + 'id4', + 'id5', + 'id6', + 'id7', + 'id8', + 'id9' + ]); + }); + + it('should filter nodes', function() { + fixture.innerHTML = + '
'; + var nodes = createTree(); + + function filter(node) { + return node.hasAttr('role'); + } + + var nonFilteredNodes = getNodesMatchingExpression( + tree, + convertSelector('div, [aria-label]') + ); + var filteredNodes = getNodesMatchingExpression( + tree, + convertSelector('div, [aria-label]'), + filter + ); + assert.deepEqual(nonFilteredNodes, nodes); + assert.deepEqual(filteredNodes, [nodes[0]]); + }); + }); +}); From 6ddff55856c42c3273f2e286e6281970c5b91771 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Thu, 24 Mar 2022 17:26:33 -0600 Subject: [PATCH 05/12] fix ie11 --- lib/core/utils/selector-cache.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/core/utils/selector-cache.js b/lib/core/utils/selector-cache.js index d5ad44518c..474effe85f 100644 --- a/lib/core/utils/selector-cache.js +++ b/lib/core/utils/selector-cache.js @@ -28,9 +28,9 @@ function cacheSelector(key, vNode, map = selectorMap) { /** * Inner join two arrays (find all nodes in a that are also in b). - * @param {*[]} a - * @param {*[]} b - * @returns {*[]} + * @param {Mixed[]} a + * @param {Mixed[]} b + * @returns {Mixed[]} */ function innerJoin(a, b) { if (!b.length) { @@ -100,7 +100,7 @@ export function getNodesMatchingExpression(domTree, expressions, filter) { } } - let matchedNodes = new Set(); + const nodeSet = new Set(); // find nodes that match just a part of the selector in order // to speed up traversing the entire tree @@ -177,11 +177,16 @@ export function getNodesMatchingExpression(domTree, expressions, filter) { return; } - matchedNodes.add(node); + nodeSet.add(node); }); }); - matchedNodes = Array.from(matchedNodes); + // ie11 does not work with Array.from without a polyfill (missing + // `.entries`) but does have forEach. using a set saves time when + // wanting to make sure the same node isn't added twice (~3 seconds + // total over using array.includes) + const matchedNodes = []; + nodeSet.forEach(node => matchedNodes.push(node)); if (filter) { matchedNodes = matchedNodes.filter(filter); From 86fe4fcb5b5feb83075cfb0bec8680eb16af58d9 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Thu, 24 Mar 2022 17:34:37 -0600 Subject: [PATCH 06/12] const... --- lib/core/utils/selector-cache.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/core/utils/selector-cache.js b/lib/core/utils/selector-cache.js index 474effe85f..b156e5c34e 100644 --- a/lib/core/utils/selector-cache.js +++ b/lib/core/utils/selector-cache.js @@ -185,7 +185,7 @@ export function getNodesMatchingExpression(domTree, expressions, filter) { // `.entries`) but does have forEach. using a set saves time when // wanting to make sure the same node isn't added twice (~3 seconds // total over using array.includes) - const matchedNodes = []; + let matchedNodes = []; nodeSet.forEach(node => matchedNodes.push(node)); if (filter) { From bac9e39316904e7070782d2bbb8320161ec33311 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Thu, 14 Apr 2022 10:38:28 -0600 Subject: [PATCH 07/12] changes --- lib/core/utils/selector-cache.js | 286 ++++++++++++++++-------------- test/core/utils/selector-cache.js | 22 ++- 2 files changed, 168 insertions(+), 140 deletions(-) diff --git a/lib/core/utils/selector-cache.js b/lib/core/utils/selector-cache.js index b156e5c34e..a82b5175e3 100644 --- a/lib/core/utils/selector-cache.js +++ b/lib/core/utils/selector-cache.js @@ -4,72 +4,7 @@ import tokenList from './token-list'; // since attribute names can't contain whitespace, this will be // a reserved list for ids so we can perform virtual id lookups const idsKey = ' [idsMap]'; -let selectorMap = {}; - -/** - * Non-tag selectors use `*` for the tag name so a global selector won't have any other properties of the expression. - * @param {Object} exp Selector Expression - * @returns {Boolean} - */ -function isGlobalSelector(exp) { - return exp.tag === '*' && !exp.attributes && !exp.id && !exp.classes; -} - -/** - * Save a selector and vNode to the selectorMap - * @param {String} key - * @param {VirtualNode} vNode - * @param {Object} map - */ -function cacheSelector(key, vNode, map = selectorMap) { - map[key] = map[key] || []; - map[key].push(vNode); -} - -/** - * Inner join two arrays (find all nodes in a that are also in b). - * @param {Mixed[]} a - * @param {Mixed[]} b - * @returns {Mixed[]} - */ -function innerJoin(a, b) { - if (!b.length) { - return a; - } - - return a.filter(node => b.includes(node)); -} - -/** - * Cache selector information about a VirtalNode. - * @param {VirtualNode} vNode - */ -export function cacheNodeSelectors(vNode) { - if (vNode.props.nodeType !== 1) { - return; - } - - // cache the selector map to the root node of the tree - if (vNode.nodeIndex === 0) { - selectorMap = {}; - vNode._selectorMap = selectorMap; - } - - cacheSelector(vNode.props.nodeName, vNode); - cacheSelector('*', vNode); - - vNode.attrNames.forEach(attrName => { - // element ids are the only values we'll match - if (attrName === 'id') { - selectorMap[idsKey] = selectorMap[idsKey] || {}; - tokenList(vNode.attr(attrName)).forEach(value => { - cacheSelector(value, vNode, selectorMap[idsKey]); - }); - } - - cacheSelector(`[${attrName}]`, vNode); - }); -} +let selectors = {}; /** * Get nodes from the selector cache that match the selector. @@ -100,97 +35,178 @@ export function getNodesMatchingExpression(domTree, expressions, filter) { } } + // it turned out to be more performant to use a Set to generate a + // unique list of nodes rather than an array and array.includes + // (~3 seconds total on a benchmark site) const nodeSet = new Set(); - // find nodes that match just a part of the selector in order - // to speed up traversing the entire tree - expressions.forEach(expression => { - // use the last part of the expression to find nodes as it's more - // specific. e.g. for `body h1` use `h1` and not `body` - const exp = expression[expression.length - 1]; - let nodes = []; - - // a complex selector is one that will require using - // matchesExpression to determine if it matches. these include - // pseudo selectors (:not), combinators (A > B), and any - // attribute value ([class=foo]). - let isComplexSelector = - expression.length > 1 || !!exp.pseudos || !!exp.classes; - - if (isGlobalSelector(exp)) { - nodes = selectorMap['*']; - } else { - if (exp.id) { - // a selector must match all parts, otherwise we can just exit - // early - if (!selectorMap[idsKey] || !selectorMap[idsKey][exp.id]?.length) { - return; - } + expressions.forEach(expression => + findMatchingNodes(expression, selectorMap, shadowId, nodeSet) + ); + + // Sets in ie11 do not work with Array.from without a polyfill + //(missing `.entries`), but do have forEach + let matchedNodes = []; + nodeSet.forEach(node => matchedNodes.push(node)); + + if (filter) { + matchedNodes = matchedNodes.filter(filter); + } + + return matchedNodes.sort((a, b) => a.nodeIndex - b.nodeIndex); +} - // when using id selector (#one) we only find nodes that - // match the shadowId of the root - nodes = selectorMap[idsKey][exp.id].filter( - node => node.shadowId === shadowId - ); +/** + * Add nodes to the passed in Set that match just a part of the selector in order to speed up traversing the entire tree. + * @param {Object} expression Selector Expression + * @param {Object} selectorMap Selector map cache + * @param {String} shadowId ShadowID of the root node + * @param {Set} nodeSet + */ +function findMatchingNodes(expression, selectorMap, shadowId, nodeSet) { + // use the last part of the expression to find nodes as it's more + // specific. e.g. for `body h1` use `h1` and not `body` + const exp = expression[expression.length - 1]; + let nodes = []; + + // a complex selector is one that will require using + // matchesExpression to determine if it matches. these include + // pseudo selectors (:not), combinators (A > B), and any + // attribute value ([class=foo]). + let isComplexSelector = + expression.length > 1 || !!exp.pseudos || !!exp.classes; + + if (isGlobalSelector(exp)) { + nodes = selectorMap['*']; + } else { + if (exp.id) { + // a selector must match all parts, otherwise we can just exit + // early + if (!selectorMap[idsKey] || !selectorMap[idsKey][exp.id]?.length) { + return; } - if (exp.tag && exp.tag !== '*') { - if (!selectorMap[exp.tag]?.length) { - return; - } + // when using id selector (#one) we only find nodes that + // match the shadowId of the root + nodes = selectorMap[idsKey][exp.id].filter( + node => node.shadowId === shadowId + ); + } - nodes = innerJoin(selectorMap[exp.tag], nodes); + if (exp.tag && exp.tag !== '*') { + if (!selectorMap[exp.tag]?.length) { + return; } - if (exp.classes) { - if (!selectorMap['[class]']?.length) { - return; - } + const cachedNodes = selectorMap[exp.tag]; + nodes = nodes.length ? getSharedValues(cachedNodes, nodes) : cachedNodes; + } - nodes = innerJoin(selectorMap['[class]'], nodes); + if (exp.classes) { + if (!selectorMap['[class]']?.length) { + return; } - if (exp.attributes) { - for (let i = 0; i < exp.attributes.length; i++) { - const attr = exp.attributes[i]; + const cachedNodes = selectorMap['[class]']; + nodes = nodes.length ? getSharedValues(cachedNodes, nodes) : cachedNodes; + } - // an attribute selector that looks for a specific value is - // a complex selector - if (attr.type === 'attrValue') { - isComplexSelector = true; - } + if (exp.attributes) { + for (let i = 0; i < exp.attributes.length; i++) { + const attr = exp.attributes[i]; - if (!selectorMap[`[${attr.key}]`]?.length) { - return; - } + // an attribute selector that looks for a specific value is + // a complex selector + if (attr.type === 'attrValue') { + isComplexSelector = true; + } - nodes = innerJoin(selectorMap[`[${attr.key}]`], nodes); + if (!selectorMap[`[${attr.key}]`]?.length) { + return; } + + const cachedNodes = selectorMap[`[${attr.key}]`]; + nodes = nodes.length + ? getSharedValues(cachedNodes, nodes) + : cachedNodes; } } + } - nodes.forEach(node => { - // for complex selectors we need to verify that the node - // actually matches the entire selector since we only have - // nodes that partially match the last part of the selector - if (isComplexSelector && !matchesExpression(node, expression)) { - return; - } + nodes.forEach(node => { + // for complex selectors we need to verify that the node + // actually matches the entire selector since we only have + // nodes that partially match the last part of the selector + if (isComplexSelector && !matchesExpression(node, expression)) { + return; + } - nodeSet.add(node); - }); + nodeSet.add(node); }); +} - // ie11 does not work with Array.from without a polyfill (missing - // `.entries`) but does have forEach. using a set saves time when - // wanting to make sure the same node isn't added twice (~3 seconds - // total over using array.includes) - let matchedNodes = []; - nodeSet.forEach(node => matchedNodes.push(node)); +/** + * Non-tag selectors use `*` for the tag name so a global selector won't have any other properties of the expression. Pseudo selectors that use `*` (e.g. `*:not([class])`) will still be considered a global selector since we don't cache anything for pseudo selectors and will rely on filtering with matchesExpression. + * @param {Object} expression Selector Expression + * @returns {Boolean} + */ +function isGlobalSelector(expression) { + return ( + expression.tag === '*' && + !expression.attributes && + !expression.id && + !expression.classes + ); +} - if (filter) { - matchedNodes = matchedNodes.filter(filter); +/** + * Find all nodes in A that are also in B. + * @param {Mixed[]} a + * @param {Mixed[]} b + * @returns {Mixed[]} + */ +function getSharedValues(a, b) { + return a.filter(node => b.includes(node)); +} + +/** + * Save a selector and vNode to the selectorMap. + * @param {String} key + * @param {VirtualNode} vNode + * @param {Object} map + */ +function cacheSelector(key, vNode, map = selectors) { + map[key] = map[key] || []; + map[key].push(vNode); +} + +/** + * Cache selector information about a VirtalNode. + * @param {VirtualNode} vNode + */ +export function cacheNodeSelectors(vNode) { + if (vNode.props.nodeType !== 1) { + return; } - return matchedNodes.sort((a, b) => a.nodeIndex - b.nodeIndex); + // cache the selector map to the root node of the tree + if (vNode.nodeIndex === 0) { + selectors = {}; + vNode._selectorMap = selectors; + } + + cacheSelector(vNode.props.nodeName, vNode); + cacheSelector('*', vNode); + + vNode.attrNames.forEach(attrName => { + // element ids are the only values we'll match + if (attrName === 'id') { + selectors[idsKey] = selectors[idsKey] || {}; + tokenList(vNode.attr(attrName)).forEach(value => { + cacheSelector(value, vNode, selectors[idsKey]); + }); + } + + cacheSelector(`[${attrName}]`, vNode); + }); } diff --git a/test/core/utils/selector-cache.js b/test/core/utils/selector-cache.js index 56e5de7246..aabbc37fa9 100644 --- a/test/core/utils/selector-cache.js +++ b/test/core/utils/selector-cache.js @@ -57,6 +57,7 @@ describe('utils.selector-cache', function() { describe('getNodesMatchingExpression', function() { var tree; var spanVNode; + var headingVNode; function createTree() { var parent = document.createElement('section'); @@ -80,12 +81,17 @@ describe('utils.selector-cache', function() { } beforeEach(function() { + var heading = document.createElement('h1'); + headingVNode = new axe.VirtualNode(heading, vNode); + var span = document.createElement('span'); span.setAttribute('class', 'bar'); span.setAttribute('id', 'notTarget'); span.setAttribute('aria-labelledby', 'target'); - spanVNode = new axe.VirtualNode(span, vNode); - vNode.children.push(spanVNode); + spanVNode = new axe.VirtualNode(span, headingVNode); + + vNode.children.push(headingVNode); + headingVNode.children.push(spanVNode); tree = [vNode]; }); @@ -99,6 +105,7 @@ describe('utils.selector-cache', function() { var expression = convertSelector('*'); assert.deepEqual(getNodesMatchingExpression(tree, expression), [ vNode, + headingVNode, spanVNode ]); }); @@ -134,7 +141,7 @@ describe('utils.selector-cache', function() { }); it('should return an empty array if selector does not match', function() { - var expression = convertSelector('h1'); + var expression = convertSelector('main'); assert.lengthOf(getNodesMatchingExpression(tree, expression), 0); }); @@ -154,12 +161,17 @@ describe('utils.selector-cache', function() { }); it('should return nodes for direct child combinator selector', function() { - var expression = convertSelector('div > span'); + var expression = convertSelector('div > h1'); assert.deepEqual(getNodesMatchingExpression(tree, expression), [ - spanVNode + headingVNode ]); }); + it('should not return nodes for direct child combinator selector that does not match', function() { + var expression = convertSelector('div > span'); + assert.lengthOf(getNodesMatchingExpression(tree, expression), 0); + }); + it('should return nodes for attribute value selector', function() { var expression = convertSelector('[id="target"]'); assert.deepEqual(getNodesMatchingExpression(tree, expression), [vNode]); From ff984da26ef5b6aae2eb70b1bf5c281ad0615366 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Tue, 19 Apr 2022 11:26:22 -0600 Subject: [PATCH 08/12] tie map to cache --- lib/core/base/virtual-node/virtual-node.js | 3 - lib/core/utils/get-flattened-tree.js | 24 ++++- lib/core/utils/selector-cache.js | 18 ++-- test/core/base/virtual-node/virtual-node.js | 7 -- test/core/utils/flattened-tree.js | 5 + test/core/utils/selector-cache.js | 100 ++++++++++---------- 6 files changed, 83 insertions(+), 74 deletions(-) diff --git a/lib/core/base/virtual-node/virtual-node.js b/lib/core/base/virtual-node/virtual-node.js index d546b84dc9..4235b23c20 100644 --- a/lib/core/base/virtual-node/virtual-node.js +++ b/lib/core/base/virtual-node/virtual-node.js @@ -2,7 +2,6 @@ import AbstractVirtualNode from './abstract-virtual-node'; import { isXHTML, validInputTypes } from '../../utils'; import { isFocusable, getTabbableElements } from '../../../commons/dom'; import cache from '../cache'; -import { cacheNodeSelectors } from '../../utils/selector-cache'; let isXHTMLGlobal; let nodeIndex = 0; @@ -51,8 +50,6 @@ class VirtualNode extends AbstractVirtualNode { if (cache.get('nodeMap')) { cache.get('nodeMap').set(node, this); } - - cacheNodeSelectors(this); } // abstract Node properties so we can run axe in DOM-less environments. diff --git a/lib/core/utils/get-flattened-tree.js b/lib/core/utils/get-flattened-tree.js index af50400157..387182fa47 100644 --- a/lib/core/utils/get-flattened-tree.js +++ b/lib/core/utils/get-flattened-tree.js @@ -1,6 +1,7 @@ import isShadowRoot from './is-shadow-root'; import VirtualNode from '../base/virtual-node/virtual-node'; import cache from '../base/cache'; +import { cacheNodeSelectors } from './selector-cache'; /** * This implemnts the flatten-tree algorithm specified: @@ -38,6 +39,20 @@ function getSlotChildren(node) { return retVal; } +/** + * Create a virtual node + * @param {Node} node the current node + * @param {VirtualNode} parent the parent VirtualNode + * @param {String} shadowId, optional ID of the shadow DOM that is the closest shadow ancestor of the node + * @return {VirtualNode} + */ +function createNode(node, parent, shadowId) { + const vNode = new VirtualNode(node, parent, shadowId); + cacheNodeSelectors(vNode, cache.get('selectorMap')); + + return vNode; +} + /** * Recursvely returns an array of the virtual DOM nodes at this level * excluding comment nodes and the shadow DOM nodes and @@ -67,7 +82,7 @@ function flattenTree(node, shadowId, parent) { if (isShadowRoot(node)) { // generate an ID for this shadow root and overwrite the current // closure shadowId with this value so that it cascades down the tree - retVal = new VirtualNode(node, parent, shadowId); + retVal = createNode(node, parent, shadowId); shadowId = 'a' + Math.random() @@ -102,7 +117,7 @@ function flattenTree(node, shadowId, parent) { if (false && styl.display !== 'contents') { // intentionally commented out // has a box - retVal = new VirtualNode(node, parent, shadowId); + retVal = createNode(node, parent, shadowId); retVal.children = realArray.reduce((res, child) => { return reduceShadowDOM(res, child, retVal); }, []); @@ -115,7 +130,7 @@ function flattenTree(node, shadowId, parent) { } } else { if (node.nodeType === 1) { - retVal = new VirtualNode(node, parent, shadowId); + retVal = createNode(node, parent, shadowId); realArray = Array.from(node.childNodes); retVal.children = realArray.reduce((res, child) => { return reduceShadowDOM(res, child, retVal); @@ -124,7 +139,7 @@ function flattenTree(node, shadowId, parent) { return [retVal]; } else if (node.nodeType === 3) { // text - return [new VirtualNode(node, parent)]; + return [createNode(node, parent)]; } return undefined; } @@ -141,6 +156,7 @@ function flattenTree(node, shadowId, parent) { */ function getFlattenedTree(node = document.documentElement, shadowId) { cache.set('nodeMap', new WeakMap()); + cache.set('selectorMap', {}); // specifically pass `null` to the parent to designate the top // node of the tree. if parent === undefined then we know diff --git a/lib/core/utils/selector-cache.js b/lib/core/utils/selector-cache.js index a82b5175e3..593cdccf5d 100644 --- a/lib/core/utils/selector-cache.js +++ b/lib/core/utils/selector-cache.js @@ -4,7 +4,6 @@ import tokenList from './token-list'; // since attribute names can't contain whitespace, this will be // a reserved list for ids so we can perform virtual id lookups const idsKey = ' [idsMap]'; -let selectors = {}; /** * Get nodes from the selector cache that match the selector. @@ -175,7 +174,7 @@ function getSharedValues(a, b) { * @param {VirtualNode} vNode * @param {Object} map */ -function cacheSelector(key, vNode, map = selectors) { +function cacheSelector(key, vNode, map) { map[key] = map[key] || []; map[key].push(vNode); } @@ -184,29 +183,28 @@ function cacheSelector(key, vNode, map = selectors) { * Cache selector information about a VirtalNode. * @param {VirtualNode} vNode */ -export function cacheNodeSelectors(vNode) { +export function cacheNodeSelectors(vNode, selectorMap) { if (vNode.props.nodeType !== 1) { return; } // cache the selector map to the root node of the tree if (vNode.nodeIndex === 0) { - selectors = {}; - vNode._selectorMap = selectors; + vNode._selectorMap = selectorMap; } - cacheSelector(vNode.props.nodeName, vNode); - cacheSelector('*', vNode); + cacheSelector(vNode.props.nodeName, vNode, selectorMap); + cacheSelector('*', vNode, selectorMap); vNode.attrNames.forEach(attrName => { // element ids are the only values we'll match if (attrName === 'id') { - selectors[idsKey] = selectors[idsKey] || {}; + selectorMap[idsKey] = selectorMap[idsKey] || {}; tokenList(vNode.attr(attrName)).forEach(value => { - cacheSelector(value, vNode, selectors[idsKey]); + cacheSelector(value, vNode, selectorMap[idsKey]); }); } - cacheSelector(`[${attrName}]`, vNode); + cacheSelector(`[${attrName}]`, vNode, selectorMap); }); } diff --git a/test/core/base/virtual-node/virtual-node.js b/test/core/base/virtual-node/virtual-node.js index 564e046eba..f8bf611e72 100644 --- a/test/core/base/virtual-node/virtual-node.js +++ b/test/core/base/virtual-node/virtual-node.js @@ -81,13 +81,6 @@ describe('VirtualNode', function() { assert.equal(vNode.props.nodeName, 'foobar'); }); - it('should add selectorMap to root element', function() { - node.setAttribute('data-foo', 'bar'); - var vNode = new VirtualNode(node); - - assert.exists(vNode._selectorMap); - }); - it('should not add selectorMap to non-root element', function() { node.setAttribute('data-foo', 'bar'); var vNode = new VirtualNode(node, new VirtualNode(node.cloneNode())); diff --git a/test/core/utils/flattened-tree.js b/test/core/utils/flattened-tree.js index 9c458339de..6752837197 100644 --- a/test/core/utils/flattened-tree.js +++ b/test/core/utils/flattened-tree.js @@ -107,6 +107,11 @@ describe('axe.utils.getFlattenedTree', function() { assert.equal(vNode.children[1].children[0].props.nodeName, 's'); }); + it('should add selectorMap to root element', function() { + var tree = axe.utils.getFlattenedTree(); + assert.exists(tree[0]._selectorMap); + }); + if (shadowSupport.v0) { describe('shadow DOM v0', function() { beforeEach(function() { diff --git a/test/core/utils/selector-cache.js b/test/core/utils/selector-cache.js index aabbc37fa9..e6dbf17ae9 100644 --- a/test/core/utils/selector-cache.js +++ b/test/core/utils/selector-cache.js @@ -12,35 +12,31 @@ describe('utils.selector-cache', function() { vNode = new axe.VirtualNode(fixture.firstChild); }); - afterEach(function() { - fixture.innerHTML = ''; - }); - describe('cacheNodeSelectors', function() { it('should add the node to the global selector', function() { - cacheNodeSelectors(vNode); + cacheNodeSelectors(vNode, {}); assert.deepEqual(vNode._selectorMap['*'], [vNode]); }); it('should add the node to the nodeName', function() { - cacheNodeSelectors(vNode); + cacheNodeSelectors(vNode, {}); assert.deepEqual(vNode._selectorMap.div, [vNode]); }); it('should add the node to all attribute selectors', function() { - cacheNodeSelectors(vNode); + cacheNodeSelectors(vNode, {}); assert.deepEqual(vNode._selectorMap['[id]'], [vNode]); assert.deepEqual(vNode._selectorMap['[class]'], [vNode]); assert.deepEqual(vNode._selectorMap['[aria-label]'], [vNode]); }); it('should add the node to the id map', function() { - cacheNodeSelectors(vNode); + cacheNodeSelectors(vNode, {}); assert.deepEqual(vNode._selectorMap[' [idsMap]'].target, [vNode]); }); it('should not add the node to selectors it does not match', function() { - cacheNodeSelectors(vNode); + cacheNodeSelectors(vNode, {}); assert.isUndefined(vNode._selectorMap['[for]']); assert.isUndefined(vNode._selectorMap.h1); }); @@ -48,7 +44,7 @@ describe('utils.selector-cache', function() { it('should ignore non-element nodes', function() { fixture.innerHTML = 'Hello'; vNode = new axe.VirtualNode(fixture.firstChild); - cacheNodeSelectors(vNode); + cacheNodeSelectors(vNode, {}); assert.isUndefined(vNode._selectorMap); }); @@ -60,39 +56,28 @@ describe('utils.selector-cache', function() { var headingVNode; function createTree() { - var parent = document.createElement('section'); - var parentVNode = new axe.VirtualNode(parent); - tree = [parentVNode]; - var nodes = []; - for (var i = 0; i < fixture.children.length; i++) { var child = fixture.children[i]; var isShadow = child.hasAttribute('data-shadow'); - var childVNode = new axe.VirtualNode( - child, - parentVNode, - isShadow ? i : undefined - ); - parentVNode.children.push(child); - nodes.push(childVNode); + var html = child.innerHTML; + if (isShadow) { + var shadowRoot = child.attachShadow({ mode: 'open' }); + shadowRoot.innerHTML = html; + child.innerHTML = ''; + } } - return nodes; + return axe.utils.getFlattenedTree(fixture); } beforeEach(function() { - var heading = document.createElement('h1'); - headingVNode = new axe.VirtualNode(heading, vNode); + fixture.firstChild.innerHTML = + '

'; + tree = axe.utils.getFlattenedTree(fixture.firstChild); - var span = document.createElement('span'); - span.setAttribute('class', 'bar'); - span.setAttribute('id', 'notTarget'); - span.setAttribute('aria-labelledby', 'target'); - spanVNode = new axe.VirtualNode(span, headingVNode); - - vNode.children.push(headingVNode); - headingVNode.children.push(spanVNode); - tree = [vNode]; + vNode = tree[0]; + headingVNode = vNode.children[0]; + spanVNode = headingVNode.children[0]; }); it('should return undefined if the cache is not primed', function() { @@ -122,12 +107,11 @@ describe('utils.selector-cache', function() { it('should only return nodes matching shadowId when matching by id', function() { fixture.innerHTML = - '
'; - var nodes = createTree(); + '
'; + var tree = createTree(); var expression = convertSelector('#target'); - assert.deepEqual(getNodesMatchingExpression(tree, expression), [ - nodes[0] - ]); + var expected = [tree[0].children[0]]; + assert.deepEqual(getNodesMatchingExpression(tree, expression), expected); }); it('should return a list of matching nodes by class', function() { @@ -145,12 +129,18 @@ describe('utils.selector-cache', function() { assert.lengthOf(getNodesMatchingExpression(tree, expression), 0); }); + it('should return an empty array for complex selector that does not match', function() { + var expression = convertSelector('span.missingClass[id]'); + assert.lengthOf(getNodesMatchingExpression(tree, expression), 0); + }); + it('should return nodes for each expression', function() { fixture.innerHTML = - '
'; - var nodes = createTree(); - var expression = convertSelector('[role], [id]'); - assert.deepEqual(getNodesMatchingExpression(tree, expression), nodes); + '
'; + var tree = createTree(); + var expression = convertSelector('[role], [aria-label]'); + var expected = [tree[0].children[0], tree[0].children[1]]; + assert.deepEqual(getNodesMatchingExpression(tree, expression), expected); }); it('should return nodes for child combinator selector', function() { @@ -189,9 +179,10 @@ describe('utils.selector-cache', function() { it('should remove duplicates', function() { fixture.innerHTML = '
'; - var nodes = createTree(); - var expression = convertSelector('div, [aria-label]'); - assert.deepEqual(getNodesMatchingExpression(tree, expression), nodes); + var tree = createTree(); + var expression = convertSelector('div[role], [aria-label]'); + var expected = [tree[0].children[0]]; + assert.deepEqual(getNodesMatchingExpression(tree, expression), expected); }); it('should sort nodes by added order', function() { @@ -206,7 +197,7 @@ describe('utils.selector-cache', function() { '' + '
' + ''; - createTree(); + tree = createTree(); var expression = convertSelector('div, span'); var nodes = getNodesMatchingExpression(tree, expression); @@ -216,6 +207,7 @@ describe('utils.selector-cache', function() { } assert.deepEqual(ids, [ + 'fixture', 'id0', 'id1', 'id2', @@ -232,7 +224,7 @@ describe('utils.selector-cache', function() { it('should filter nodes', function() { fixture.innerHTML = '
'; - var nodes = createTree(); + var tree = createTree(); function filter(node) { return node.hasAttr('role'); @@ -242,13 +234,21 @@ describe('utils.selector-cache', function() { tree, convertSelector('div, [aria-label]') ); + var nonFilteredExpected = [ + tree[0], + tree[0].children[0], + tree[0].children[1] + ]; + var filteredNodes = getNodesMatchingExpression( tree, convertSelector('div, [aria-label]'), filter ); - assert.deepEqual(nonFilteredNodes, nodes); - assert.deepEqual(filteredNodes, [nodes[0]]); + var filteredExpected = [tree[0].children[0]]; + + assert.deepEqual(nonFilteredNodes, nonFilteredExpected); + assert.deepEqual(filteredNodes, filteredExpected); }); }); }); From 425731b9880e62d779c88a6c28b8844301c8bf3c Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Tue, 19 Apr 2022 11:53:59 -0600 Subject: [PATCH 09/12] fix test --- test/core/utils/selector-cache.js | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/test/core/utils/selector-cache.js b/test/core/utils/selector-cache.js index e6dbf17ae9..619915c210 100644 --- a/test/core/utils/selector-cache.js +++ b/test/core/utils/selector-cache.js @@ -5,6 +5,7 @@ describe('utils.selector-cache', function() { var getNodesMatchingExpression = axe._thisWillBeDeletedDoNotUse.utils.getNodesMatchingExpression; var convertSelector = axe.utils.convertSelector; + var shadowSupported = axe.testUtils.shadowSupport.v1; var vNode; beforeEach(function() { @@ -105,14 +106,20 @@ describe('utils.selector-cache', function() { assert.deepEqual(getNodesMatchingExpression(tree, expression), [vNode]); }); - it('should only return nodes matching shadowId when matching by id', function() { - fixture.innerHTML = - '
'; - var tree = createTree(); - var expression = convertSelector('#target'); - var expected = [tree[0].children[0]]; - assert.deepEqual(getNodesMatchingExpression(tree, expression), expected); - }); + (shadowSupported ? it : xit)( + 'should only return nodes matching shadowId when matching by id', + function() { + fixture.innerHTML = + '
'; + var tree = createTree(); + var expression = convertSelector('#target'); + var expected = [tree[0].children[0]]; + assert.deepEqual( + getNodesMatchingExpression(tree, expression), + expected + ); + } + ); it('should return a list of matching nodes by class', function() { var expression = convertSelector('.foo'); From dd80e7e0b5758ed4161d13f2427a13ab18269b2d Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Wed, 20 Apr 2022 09:28:18 -0600 Subject: [PATCH 10/12] remove test --- test/core/base/virtual-node/virtual-node.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/test/core/base/virtual-node/virtual-node.js b/test/core/base/virtual-node/virtual-node.js index f8bf611e72..976035e942 100644 --- a/test/core/base/virtual-node/virtual-node.js +++ b/test/core/base/virtual-node/virtual-node.js @@ -81,13 +81,6 @@ describe('VirtualNode', function() { assert.equal(vNode.props.nodeName, 'foobar'); }); - it('should not add selectorMap to non-root element', function() { - node.setAttribute('data-foo', 'bar'); - var vNode = new VirtualNode(node, new VirtualNode(node.cloneNode())); - - assert.notExists(vNode._selectorMap); - }); - describe('attr', function() { it('should return the value of the given attribute', function() { node.setAttribute('data-foo', 'bar'); From 2872fe45a50cd0009b1d7adda26f4b35b0cb93c2 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Wed, 1 Jun 2022 09:09:42 -0600 Subject: [PATCH 11/12] fixes --- lib/core/core.js | 2 ++ lib/core/utils/get-flattened-tree.js | 5 ++- lib/core/utils/selector-cache.js | 48 +++++++++++++--------------- test/core/utils/matches.js | 18 +++++++++++ test/core/utils/selector-cache.js | 41 +++++++++++++++--------- 5 files changed, 72 insertions(+), 42 deletions(-) diff --git a/lib/core/core.js b/lib/core/core.js index c3e5717fb9..2aded83b4a 100644 --- a/lib/core/core.js +++ b/lib/core/core.js @@ -50,6 +50,7 @@ import { cacheNodeSelectors, getNodesMatchingExpression } from './utils/selector-cache'; +import { convertSelector } from './utils/matches'; axe.constants = constants; axe.log = log; @@ -78,6 +79,7 @@ axe._thisWillBeDeletedDoNotUse.utils = axe._thisWillBeDeletedDoNotUse.utils || {}; axe._thisWillBeDeletedDoNotUse.utils.cacheNodeSelectors = cacheNodeSelectors; axe._thisWillBeDeletedDoNotUse.utils.getNodesMatchingExpression = getNodesMatchingExpression; +axe._thisWillBeDeletedDoNotUse.utils.convertSelector = convertSelector; axe.imports = imports; diff --git a/lib/core/utils/get-flattened-tree.js b/lib/core/utils/get-flattened-tree.js index 387182fa47..6260d23748 100644 --- a/lib/core/utils/get-flattened-tree.js +++ b/lib/core/utils/get-flattened-tree.js @@ -161,7 +161,10 @@ function getFlattenedTree(node = document.documentElement, shadowId) { // specifically pass `null` to the parent to designate the top // node of the tree. if parent === undefined then we know // we are in a disconnected tree - return flattenTree(node, shadowId, null); + var virtualTree = flattenTree(node, shadowId, null); + virtualTree[0]._selectorMap = cache.get('selectorMap'); + + return virtualTree; } export default getFlattenedTree; diff --git a/lib/core/utils/selector-cache.js b/lib/core/utils/selector-cache.js index 593cdccf5d..5f81700678 100644 --- a/lib/core/utils/selector-cache.js +++ b/lib/core/utils/selector-cache.js @@ -39,9 +39,22 @@ export function getNodesMatchingExpression(domTree, expressions, filter) { // (~3 seconds total on a benchmark site) const nodeSet = new Set(); - expressions.forEach(expression => - findMatchingNodes(expression, selectorMap, shadowId, nodeSet) - ); + expressions.forEach(expression => { + const matchingNodes = findMatchingNodes(expression, selectorMap, shadowId); + matchingNodes?.nodes?.forEach(node => { + // for complex selectors we need to verify that the node + // actually matches the entire selector since we only have + // nodes that partially match the last part of the selector + if ( + matchingNodes.isComplexSelector && + !matchesExpression(node, expression) + ) { + return; + } + + nodeSet.add(node); + }); + }); // Sets in ie11 do not work with Array.from without a polyfill //(missing `.entries`), but do have forEach @@ -60,13 +73,12 @@ export function getNodesMatchingExpression(domTree, expressions, filter) { * @param {Object} expression Selector Expression * @param {Object} selectorMap Selector map cache * @param {String} shadowId ShadowID of the root node - * @param {Set} nodeSet */ -function findMatchingNodes(expression, selectorMap, shadowId, nodeSet) { +function findMatchingNodes(expression, selectorMap, shadowId) { // use the last part of the expression to find nodes as it's more // specific. e.g. for `body h1` use `h1` and not `body` const exp = expression[expression.length - 1]; - let nodes = []; + let nodes = null; // a complex selector is one that will require using // matchesExpression to determine if it matches. these include @@ -98,7 +110,7 @@ function findMatchingNodes(expression, selectorMap, shadowId, nodeSet) { } const cachedNodes = selectorMap[exp.tag]; - nodes = nodes.length ? getSharedValues(cachedNodes, nodes) : cachedNodes; + nodes = nodes ? getSharedValues(cachedNodes, nodes) : cachedNodes; } if (exp.classes) { @@ -107,7 +119,7 @@ function findMatchingNodes(expression, selectorMap, shadowId, nodeSet) { } const cachedNodes = selectorMap['[class]']; - nodes = nodes.length ? getSharedValues(cachedNodes, nodes) : cachedNodes; + nodes = nodes ? getSharedValues(cachedNodes, nodes) : cachedNodes; } if (exp.attributes) { @@ -125,23 +137,12 @@ function findMatchingNodes(expression, selectorMap, shadowId, nodeSet) { } const cachedNodes = selectorMap[`[${attr.key}]`]; - nodes = nodes.length - ? getSharedValues(cachedNodes, nodes) - : cachedNodes; + nodes = nodes ? getSharedValues(cachedNodes, nodes) : cachedNodes; } } } - nodes.forEach(node => { - // for complex selectors we need to verify that the node - // actually matches the entire selector since we only have - // nodes that partially match the last part of the selector - if (isComplexSelector && !matchesExpression(node, expression)) { - return; - } - - nodeSet.add(node); - }); + return { nodes, isComplexSelector }; } /** @@ -188,11 +189,6 @@ export function cacheNodeSelectors(vNode, selectorMap) { return; } - // cache the selector map to the root node of the tree - if (vNode.nodeIndex === 0) { - vNode._selectorMap = selectorMap; - } - cacheSelector(vNode.props.nodeName, vNode, selectorMap); cacheSelector('*', vNode, selectorMap); diff --git a/test/core/utils/matches.js b/test/core/utils/matches.js index 16666de7c4..ad5aac90dc 100644 --- a/test/core/utils/matches.js +++ b/test/core/utils/matches.js @@ -2,6 +2,7 @@ describe('utils.matches', function() { var matches = axe.utils.matches; var fixture = document.querySelector('#fixture'); var queryFixture = axe.testUtils.queryFixture; + var convertSelector = axe._thisWillBeDeletedDoNotUse.utils.convertSelector; afterEach(function() { fixture.innerHTML = ''; @@ -322,4 +323,21 @@ describe('utils.matches', function() { }); }); }); + + describe('convertSelector', function() { + it('should set type to attrExist for attribute selector', function() { + var expression = convertSelector('[disabled]'); + assert.equal(expression[0][0].attributes[0].type, 'attrExist'); + }); + + it('should set type to attrValue for attribute value selector', function() { + var expression = convertSelector('[aria-pressed="true"]'); + assert.equal(expression[0][0].attributes[0].type, 'attrValue'); + }); + + it('should set type to attrValue for empty attribute value selector', function() { + var expression = convertSelector('[aria-pressed=""]'); + assert.equal(expression[0][0].attributes[0].type, 'attrValue'); + }); + }); }); diff --git a/test/core/utils/selector-cache.js b/test/core/utils/selector-cache.js index 619915c210..c0b7bfaf1b 100644 --- a/test/core/utils/selector-cache.js +++ b/test/core/utils/selector-cache.js @@ -15,39 +15,45 @@ describe('utils.selector-cache', function() { describe('cacheNodeSelectors', function() { it('should add the node to the global selector', function() { - cacheNodeSelectors(vNode, {}); - assert.deepEqual(vNode._selectorMap['*'], [vNode]); + var map = {}; + cacheNodeSelectors(vNode, map); + assert.deepEqual(map['*'], [vNode]); }); it('should add the node to the nodeName', function() { - cacheNodeSelectors(vNode, {}); - assert.deepEqual(vNode._selectorMap.div, [vNode]); + var map = {}; + cacheNodeSelectors(vNode, map); + assert.deepEqual(map.div, [vNode]); }); it('should add the node to all attribute selectors', function() { - cacheNodeSelectors(vNode, {}); - assert.deepEqual(vNode._selectorMap['[id]'], [vNode]); - assert.deepEqual(vNode._selectorMap['[class]'], [vNode]); - assert.deepEqual(vNode._selectorMap['[aria-label]'], [vNode]); + var map = {}; + cacheNodeSelectors(vNode, map); + assert.deepEqual(map['[id]'], [vNode]); + assert.deepEqual(map['[class]'], [vNode]); + assert.deepEqual(map['[aria-label]'], [vNode]); }); it('should add the node to the id map', function() { - cacheNodeSelectors(vNode, {}); - assert.deepEqual(vNode._selectorMap[' [idsMap]'].target, [vNode]); + var map = {}; + cacheNodeSelectors(vNode, map); + assert.deepEqual(map[' [idsMap]'].target, [vNode]); }); it('should not add the node to selectors it does not match', function() { - cacheNodeSelectors(vNode, {}); - assert.isUndefined(vNode._selectorMap['[for]']); - assert.isUndefined(vNode._selectorMap.h1); + var map = {}; + cacheNodeSelectors(vNode, map); + assert.isUndefined(map['[for]']); + assert.isUndefined(map.h1); }); it('should ignore non-element nodes', function() { + var map = {}; fixture.innerHTML = 'Hello'; vNode = new axe.VirtualNode(fixture.firstChild); - cacheNodeSelectors(vNode, {}); + cacheNodeSelectors(vNode, map); - assert.isUndefined(vNode._selectorMap); + assert.lengthOf(Object.keys(map), 0); }); }); @@ -141,6 +147,11 @@ describe('utils.selector-cache', function() { assert.lengthOf(getNodesMatchingExpression(tree, expression), 0); }); + it('should return an empty array for a non-complex selector that does not match', function() { + var expression = convertSelector('div#not-target[id]'); + assert.lengthOf(getNodesMatchingExpression(tree, expression), 0); + }); + it('should return nodes for each expression', function() { fixture.innerHTML = '
'; From e4dc6126c614dae294d75691a62f94a1eb4b8421 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Wed, 1 Jun 2022 09:14:52 -0600 Subject: [PATCH 12/12] revert rule descriptions --- doc/rule-descriptions.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/rule-descriptions.md b/doc/rule-descriptions.md index 25be6d9273..489cea0a41 100644 --- a/doc/rule-descriptions.md +++ b/doc/rule-descriptions.md @@ -70,10 +70,11 @@ ## WCAG 2.1 Level A & AA Rules -| Rule ID | Description | Impact | Tags | Issue Type | ACT Rules | -| :----------------------------------------------------------------------------------------------------------------- | :-------------------------------------------------------------------------------------------- | :------ | :-------------------------------- | :--------- | :------------------------------------------------- | -| [autocomplete-valid](https://dequeuniversity.com/rules/axe/4.4/autocomplete-valid?application=RuleDescription) | Ensure the autocomplete attribute is correct and suitable for the form field | Serious | cat.forms, wcag21aa, wcag135 | failure | [73f2c2](https://act-rules.github.io/rules/73f2c2) | -| [avoid-inline-spacing](https://dequeuniversity.com/rules/axe/4.4/avoid-inline-spacing?application=RuleDescription) | Ensure that text spacing set through style attributes can be adjusted with custom stylesheets | Serious | cat.structure, wcag21aa, wcag1412 | failure | | +| Rule ID | Description | Impact | Tags | Issue Type | ACT Rules | +| :----------------------------------------------------------------------------------------------------------------- | :-------------------------------------------------------------------------------------------- | :------ | :-------------------------------- | :---------------- | :------------------------------------------------- | +| [autocomplete-valid](https://dequeuniversity.com/rules/axe/4.4/autocomplete-valid?application=RuleDescription) | Ensure the autocomplete attribute is correct and suitable for the form field | Serious | cat.forms, wcag21aa, wcag135 | failure | [73f2c2](https://act-rules.github.io/rules/73f2c2) | +| [avoid-inline-spacing](https://dequeuniversity.com/rules/axe/4.4/avoid-inline-spacing?application=RuleDescription) | Ensure that text spacing set through style attributes can be adjusted with custom stylesheets | Serious | cat.structure, wcag21aa, wcag1412 | failure | | +| [empty-table-header](https://dequeuniversity.com/rules/axe/4.4/empty-table-header?application=RuleDescription) | Ensures table headers have discernible text | Minor | wcag131, cat.aria | needs review | | ## Best Practices Rules @@ -87,7 +88,6 @@ Rules that do not necessarily conform to WCAG success criterion but are industry | [aria-text](https://dequeuniversity.com/rules/axe/4.4/aria-text?application=RuleDescription) | Ensures "role=text" is used on elements with no focusable descendants | Serious | cat.aria, best-practice | failure, needs review | | | [aria-treeitem-name](https://dequeuniversity.com/rules/axe/4.4/aria-treeitem-name?application=RuleDescription) | Ensures every ARIA treeitem node has an accessible name | Serious | cat.aria, best-practice | failure, needs review | | | [empty-heading](https://dequeuniversity.com/rules/axe/4.4/empty-heading?application=RuleDescription) | Ensures headings have discernible text | Minor | cat.name-role-value, best-practice | failure, needs review | | -| [empty-table-header](https://dequeuniversity.com/rules/axe/4.4/empty-table-header?application=RuleDescription) | Ensures table headers have discernible text | Minor | cat.name-role-value, best-practice | failure, needs review | | | [frame-tested](https://dequeuniversity.com/rules/axe/4.4/frame-tested?application=RuleDescription) | Ensures <iframe> and <frame> elements contain the axe-core script | Critical | cat.structure, review-item, best-practice | failure, needs review | | | [frame-title-unique](https://dequeuniversity.com/rules/axe/4.4/frame-title-unique?application=RuleDescription) | Ensures <iframe> and <frame> elements contain a unique title attribute | Serious | cat.text-alternatives, best-practice | failure | | | [heading-order](https://dequeuniversity.com/rules/axe/4.4/heading-order?application=RuleDescription) | Ensures the order of headings is semantically correct | Moderate | cat.semantics, best-practice | failure, needs review | |