From 7d9b70cb02223f5ec4b501deef14f6df875ce029 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Fri, 19 Aug 2022 14:31:16 +0200 Subject: [PATCH 01/28] feat(new-rule): Add WCAG 2.2 target-size rule --- doc/rule-descriptions.md | 1 + lib/checks/mobile/target-offset-evaluate.js | 32 + lib/checks/mobile/target-offset.json | 14 + lib/checks/mobile/target-size-evaluate.js | 70 ++ lib/checks/mobile/target-size.json | 17 + lib/commons/dom/find-nearby-elms.js | 33 + lib/commons/dom/get-element-stack.js | 6 +- lib/commons/dom/get-rect-stack.js | 154 +---- lib/commons/dom/get-text-element-stack.js | 6 +- lib/commons/dom/index.js | 3 + lib/commons/dom/visually-sort.js | 137 ++++ lib/commons/index.js | 15 +- lib/commons/math/get-offset.js | 147 ++++ lib/commons/math/has-visual-overlap.js | 44 ++ lib/commons/math/index.js | 3 + lib/commons/math/split-rects.js | 57 ++ lib/core/constants.js | 1 + lib/rules/target-spacing.json | 12 + locales/_template.json | 18 +- test/checks/mobile/target-offset.js | 15 + test/checks/mobile/target-size.js | 90 +++ test/commons/dom/find-nearby-elms.js | 41 ++ test/commons/dom/visually-sort.js | 28 + test/commons/math/get-offset.js | 103 +++ test/commons/math/has-visual-overlap | 34 + test/commons/math/split-rects.js | 92 +++ .../full/target-size/target-size.html | 652 ++++++++++++++++++ .../full/target-size/target-size.js | 53 ++ 28 files changed, 1725 insertions(+), 153 deletions(-) create mode 100644 lib/checks/mobile/target-offset-evaluate.js create mode 100644 lib/checks/mobile/target-offset.json create mode 100644 lib/checks/mobile/target-size-evaluate.js create mode 100644 lib/checks/mobile/target-size.json create mode 100644 lib/commons/dom/find-nearby-elms.js create mode 100644 lib/commons/dom/visually-sort.js create mode 100644 lib/commons/math/get-offset.js create mode 100644 lib/commons/math/has-visual-overlap.js create mode 100644 lib/commons/math/index.js create mode 100644 lib/commons/math/split-rects.js create mode 100644 lib/rules/target-spacing.json create mode 100644 test/checks/mobile/target-offset.js create mode 100644 test/checks/mobile/target-size.js create mode 100644 test/commons/dom/find-nearby-elms.js create mode 100644 test/commons/dom/visually-sort.js create mode 100644 test/commons/math/get-offset.js create mode 100644 test/commons/math/has-visual-overlap create mode 100644 test/commons/math/split-rects.js create mode 100644 test/integration/full/target-size/target-size.html create mode 100644 test/integration/full/target-size/target-size.js diff --git a/doc/rule-descriptions.md b/doc/rule-descriptions.md index b04f237bac..8689a73aa7 100644 --- a/doc/rule-descriptions.md +++ b/doc/rule-descriptions.md @@ -136,6 +136,7 @@ Rules we are still testing and developing. They are disabled by default in axe-c | [link-in-text-block](https://dequeuniversity.com/rules/axe/4.4/link-in-text-block?application=RuleDescription) | Ensure links are distinguished from surrounding text in a way that does not rely on color | Serious | cat.color, experimental, wcag2a, wcag141 | failure, needs review | | | [p-as-heading](https://dequeuniversity.com/rules/axe/4.4/p-as-heading?application=RuleDescription) | Ensure bold, italic text and font-size is not used to style <p> elements as a heading | Serious | cat.semantics, wcag2a, wcag131, experimental | failure, needs review | | | [table-fake-caption](https://dequeuniversity.com/rules/axe/4.4/table-fake-caption?application=RuleDescription) | Ensure that tables with a caption use the <caption> element. | Serious | cat.tables, experimental, wcag2a, wcag131, section508, section508.22.g | failure | | +| [target-size](https://dequeuniversity.com/rules/axe/4.4/target-size?application=RuleDescription) | Ensure touch target have sufficient size and space | Serious | wcag22aa, sc1412, experimental, cat.sensory-and-visual-cues | failure | | | [td-has-header](https://dequeuniversity.com/rules/axe/4.4/td-has-header?application=RuleDescription) | Ensure that each non-empty data cell in a <table> larger than 3 by 3 has one or more table headers | Critical | cat.tables, experimental, wcag2a, wcag131, section508, section508.22.g | failure | | ## Deprecated Rules diff --git a/lib/checks/mobile/target-offset-evaluate.js b/lib/checks/mobile/target-offset-evaluate.js new file mode 100644 index 0000000000..91f9c5b149 --- /dev/null +++ b/lib/checks/mobile/target-offset-evaluate.js @@ -0,0 +1,32 @@ +import { findNearbyElms } from '../../commons/dom'; +import { getRole, getRoleType } from '../../commons/aria'; +import { getOffset } from '../../commons/math'; + +export default function targetOffsetEvaluate( + node, + { minSpacing = 23.95 }, + vNode +) { + let closestOffset = minSpacing; + const closeNeighbors = []; + for (const vNeighbor of findNearbyElms(vNode, minSpacing)) { + const role = getRole(vNeighbor); + if (getRoleType(role) !== 'widget') { + continue; + } + const offset = getOffset(vNode, vNeighbor); + if (offset >= minSpacing) { + continue; + } + closestOffset = Math.min(closestOffset, offset); + closeNeighbors.push(vNeighbor.actualNode); + } + + if (closeNeighbors.length === 0) { + return true; + } + + this.data({ closestOffset }); + this.relatedNodes(closeNeighbors); + return false; +} diff --git a/lib/checks/mobile/target-offset.json b/lib/checks/mobile/target-offset.json new file mode 100644 index 0000000000..ff39061520 --- /dev/null +++ b/lib/checks/mobile/target-offset.json @@ -0,0 +1,14 @@ +{ + "id": "target-offset", + "evaluate": "target-offset-evaluate", + "options": { + "minSpacing": 23.95 + }, + "metadata": { + "impact": "serious", + "messages": { + "pass": "Target is sufficiently spaced from its neighbours", + "fail": "Target is insufficiently spaced from its neighbours" + } + } +} diff --git a/lib/checks/mobile/target-size-evaluate.js b/lib/checks/mobile/target-size-evaluate.js new file mode 100644 index 0000000000..ba5c7d1214 --- /dev/null +++ b/lib/checks/mobile/target-size-evaluate.js @@ -0,0 +1,70 @@ +import { findNearbyElms } from '../../commons/dom'; +import { getRole, getRoleType } from '../../commons/aria'; +import { splitRects, hasVisualOverlap } from '../../commons/math'; + +/** + * Determine if an element has a minimum size, taking into account + * any elements that may obscure it. + */ +export default function targetSize(node, { minSpacing = 23.95 }, vNode) { + const hasMinimumSize = ({ width, height }) => { + return width >= minSpacing && height >= minSpacing; + }; + + const nodeRect = vNode.boundingClientRect; + if (!hasMinimumSize(nodeRect)) { + this.data(toDecimalSize(nodeRect)); + return false; + } + + // Handle overlapping elements; + const nearbyElms = findNearbyElms(vNode); + const obscuringElms = nearbyElms.filter(vNeighbor => { + const role = getRole(vNeighbor); + return getRoleType(role) === 'widget' && hasVisualOverlap(vNode, vNeighbor); + }); + + if (obscuringElms.length === 0) { + this.data(toDecimalSize(nodeRect)); + return true; // No obscuring elements; pass + } + + // Find areas of the target that are not obscured + const obscuringRects = obscuringElms.map( + ({ boundingClientRect: rect }) => rect + ); + const unobscuredRects = splitRects(nodeRect, obscuringRects); + + // Of the unobscured inner rects, work out the largest + const largestInnerRect = unobscuredRects.reduce((rectA, rectB) => { + const rectAisMinimum = hasMinimumSize(rectA); + const rectBisMinimum = hasMinimumSize(rectB); + // Prioritize rects that pass the minimum + if (rectAisMinimum !== rectBisMinimum) { + return rectAisMinimum ? rectA : rectB; + } + const areaA = rectA.width * rectA.height; + const areaB = rectB.width * rectB.height; + return areaA > areaB ? rectA : rectB; + }); + + if (!hasMinimumSize(largestInnerRect)) { + this.data({ + messageKey: 'obscured', + ...toDecimalSize(largestInnerRect) + }); + this.relatedNodes(obscuringElms.map(({ actualNode }) => actualNode)); + // Element is (partially?) obscured, with insufficient space + return false; + } + + this.data(toDecimalSize(largestInnerRect)); + return true; +} + +function toDecimalSize(rect) { + return { + width: Math.round(rect.width * 10) / 10, + height: Math.round(rect.height * 10) / 10 + }; +} diff --git a/lib/checks/mobile/target-size.json b/lib/checks/mobile/target-size.json new file mode 100644 index 0000000000..a5cbf27546 --- /dev/null +++ b/lib/checks/mobile/target-size.json @@ -0,0 +1,17 @@ +{ + "id": "target-size", + "evaluate": "target-size-evaluate", + "options": { + "minSpacing": 23.95 + }, + "metadata": { + "impact": "serious", + "messages": { + "pass": "Control has sufficient size (${data.width}px width, ${data.height}px height)", + "fail": { + "default": "Element has insufficient size (${data.width}px width, ${data.height}px height)", + "obscured": "Element has insufficient size because it is obscured (smallest space is ${data.width}px width by ${data.height}px height)" + } + } + } +} diff --git a/lib/commons/dom/find-nearby-elms.js b/lib/commons/dom/find-nearby-elms.js new file mode 100644 index 0000000000..6779fc34db --- /dev/null +++ b/lib/commons/dom/find-nearby-elms.js @@ -0,0 +1,33 @@ +import { createGrid } from './get-rect-stack'; + +function findNearbyElms(vNode, margin = 0) { + /*eslint no-bitwise: 0*/ + const gridSize = createGrid(); + const neighbors = []; + const rect = vNode.boundingClientRect; + const gridCells = vNode._grid.cells; + + const topRow = ((rect.top - margin) / gridSize) | 0; + const bottomRow = ((rect.bottom + margin) / gridSize) | 0; + const leftCol = ((rect.left - margin) / gridSize) | 0; + const rightCol = ((rect.right + margin) / gridSize) | 0; + + for (let row = topRow; row <= bottomRow; row++) { + for (let col = leftCol; col <= rightCol; col++) { + for (let i = 0; i <= gridCells[row][col].length; i++) { + var vNeighbour = gridCells[row][col][i]; + // Avoid duplication + if ( + vNeighbour && + vNeighbour !== vNode && + !neighbors.includes(vNeighbour) + ) { + neighbors.push(vNeighbour); + } + } + } + } + return neighbors; +} + +export default findNearbyElms; diff --git a/lib/commons/dom/get-element-stack.js b/lib/commons/dom/get-element-stack.js index 96bba028ef..00a4137805 100644 --- a/lib/commons/dom/get-element-stack.js +++ b/lib/commons/dom/get-element-stack.js @@ -1,6 +1,5 @@ import { createGrid, getRectStack } from './get-rect-stack'; import { getNodeFromTree } from '../../core/utils'; -import cache from '../../core/base/cache'; /** * Return all elements that are at the center bounding rect of the passed in node. @@ -10,10 +9,7 @@ import cache from '../../core/base/cache'; * @return {Node[]} */ function getElementStack(node) { - if (!cache.get('gridCreated')) { - createGrid(); - cache.set('gridCreated', true); - } + createGrid(); const vNode = getNodeFromTree(node); const grid = vNode._grid; diff --git a/lib/commons/dom/get-rect-stack.js b/lib/commons/dom/get-rect-stack.js index 6febf1a404..cda905a090 100644 --- a/lib/commons/dom/get-rect-stack.js +++ b/lib/commons/dom/get-rect-stack.js @@ -1,6 +1,10 @@ +/* eslint no-bitwise: 0 */ import isVisible from './is-visible'; +import visuallySort from './visually-sort'; import VirtualNode from '../../core/base/virtual-node/virtual-node'; import { getNodeFromTree, getScroll, isShadowRoot } from '../../core/utils'; +import constants from '../../core/constants'; +import cache from '../../core/base/cache'; // split the page cells to group elements by the position const gridSize = 200; // arbitrary size, increase to reduce memory use (less cells) but increase time (more nodes per grid to check collision) @@ -8,6 +12,7 @@ const gridSize = 200; // arbitrary size, increase to reduce memory use (less cel /** * Setup the 2d grid and add every element to it, even elements not * included in the flat tree + * @returns gridSize */ export function createGrid( root = document.body, @@ -17,6 +22,12 @@ export function createGrid( }, parentVNode = null ) { + // Prevent multiple calls per run + if (cache.get('gridCreated') && !parentVNode) { + return constants.gridSize; + } + cache.set('gridCreated', true); + // by not starting at the htmlElement we don't have to pass a custom // filter function into the treeWalker to filter out head elements, // which would be called for every node @@ -91,6 +102,7 @@ export function createGrid( node = treeWalker.nextNode(); } + return constants.gridSize; } export function getRectStack(grid, rect, recursed = false) { @@ -304,147 +316,6 @@ function isStackingContext(vNode, parentVNode) { return false; } -/** - * Check if a node or one of it's parents is floated. - * Floating position should be inherited from the parent tree - * @see https://github.com/dequelabs/axe-core/issues/2222 - */ -function isFloated(vNode) { - if (!vNode) { - return false; - } - - if (vNode._isFloated !== undefined) { - return vNode._isFloated; - } - - const floatStyle = vNode.getComputedStylePropertyValue('float'); - - if (floatStyle !== 'none') { - vNode._isFloated = true; - return true; - } - - const floated = isFloated(vNode.parent); - vNode._isFloated = floated; - return floated; -} - -/** - * Return the index order of how to position this element. return nodes in non-positioned, floating, positioned order - * References: - * https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Positioning/Understanding_z_index/Stacking_without_z-index - * https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Positioning/Understanding_z_index/Stacking_and_float - * https://www.w3.org/Style/css2-updates/css2/zindex.html - * @param {VirtualNode} vNode - * @return {Number} - */ -function getPositionOrder(vNode) { - // 5. the in-flow, inline-level, non-positioned descendants, including inline tables and inline blocks. - if (vNode.getComputedStylePropertyValue('display').indexOf('inline') !== -1) { - return 2; - } - - // 4. the non-positioned floats. - if (isFloated(vNode)) { - return 1; - } - - // 3. the in-flow, non-inline-level, non-positioned descendants. - return 0; -} - -/** - * Visually sort nodes based on their stack order - * References: - * https://www.w3.org/Style/css2-updates/css2/zindex.html - * @param {VirtualNode} - * @param {VirtualNode} - */ -function visuallySort(a, b) { - /*eslint no-bitwise: 0 */ - const length = Math.max(a._stackingOrder.length, b._stackingOrder.length); - - for (let i = 0; i < length; i++) { - if (typeof b._stackingOrder[i] === 'undefined') { - return -1; - } else if (typeof a._stackingOrder[i] === 'undefined') { - return 1; - } - - // 7. the child stacking contexts with positive stack levels (least positive first). - if (b._stackingOrder[i] > a._stackingOrder[i]) { - return 1; - } - - // 2. the child stacking contexts with negative stack levels (most negative first). - if (b._stackingOrder[i] < a._stackingOrder[i]) { - return -1; - } - } - - // nodes are the same stacking order - let aNode = a.actualNode; - let bNode = b.actualNode; - - // elements don't correctly calculate document position when comparing - // across shadow boundaries, so we need to compare the position of a - // shared host instead - - // elements have different hosts - if (aNode.getRootNode && aNode.getRootNode() !== bNode.getRootNode()) { - // keep track of all parent hosts and find the one both nodes share - const boundaries = []; - while (aNode) { - boundaries.push({ - root: aNode.getRootNode(), - node: aNode - }); - aNode = aNode.getRootNode().host; - } - - while ( - bNode && - !boundaries.find(boundary => boundary.root === bNode.getRootNode()) - ) { - bNode = bNode.getRootNode().host; - } - - // bNode is a node that shares a host with some part of the a parent - // shadow tree, find the aNode that shares the same host as bNode - aNode = boundaries.find( - boundary => boundary.root === bNode.getRootNode() - ).node; - - // sort child of shadow to it's host node by finding which element is - // the child of the host and sorting it before the host - if (aNode === bNode) { - return a.actualNode.getRootNode() !== aNode.getRootNode() ? -1 : 1; - } - } - - const { - DOCUMENT_POSITION_FOLLOWING, - DOCUMENT_POSITION_CONTAINS, - DOCUMENT_POSITION_CONTAINED_BY - } = window.Node; - - const docPosition = aNode.compareDocumentPosition(bNode); - const DOMOrder = docPosition & DOCUMENT_POSITION_FOLLOWING ? 1 : -1; - const isDescendant = - docPosition & DOCUMENT_POSITION_CONTAINS || - docPosition & DOCUMENT_POSITION_CONTAINED_BY; - const aPosition = getPositionOrder(a); - const bPosition = getPositionOrder(b); - - // a child of a positioned element should also be on top of the parent - if (aPosition === bPosition || isDescendant) { - return DOMOrder; - } - - return bPosition - aPosition; -} - /** * Determine the stacking order of an element. The stacking order is an array of * zIndex values for each stacking context parent. @@ -529,6 +400,7 @@ function findScrollRegionParent(vNode, parentVNode) { * @param {VirtualNode} */ function addNodeToGrid(grid, vNode) { + const gridSize = constants.gridSize; // save a reference to where this element is in the grid so we // can find it even if it's in a subgrid vNode._grid = grid; diff --git a/lib/commons/dom/get-text-element-stack.js b/lib/commons/dom/get-text-element-stack.js index ebfc9f1be8..f6e2e4d60f 100644 --- a/lib/commons/dom/get-text-element-stack.js +++ b/lib/commons/dom/get-text-element-stack.js @@ -1,7 +1,6 @@ import getElementStack from './get-element-stack'; import { createGrid, getRectStack } from './get-rect-stack'; import sanitize from '../text/sanitize'; -import cache from '../../core/base/cache'; import { getNodeFromTree } from '../../core/utils'; /** @@ -12,10 +11,7 @@ import { getNodeFromTree } from '../../core/utils'; * @return {Array} */ function getTextElementStack(node) { - if (!cache.get('gridCreated')) { - createGrid(); - cache.set('gridCreated', true); - } + createGrid(); const vNode = getNodeFromTree(node); const grid = vNode._grid; diff --git a/lib/commons/dom/index.js b/lib/commons/dom/index.js index 5f80f87ca6..adeecb7f44 100644 --- a/lib/commons/dom/index.js +++ b/lib/commons/dom/index.js @@ -6,6 +6,7 @@ export { default as findElmsInContext } from './find-elms-in-context'; export { default as findUpVirtual } from './find-up-virtual'; export { default as findUp } from './find-up'; +export { default as findNearbyElms } from './find-nearby-elms'; export { default as getComposedParent } from './get-composed-parent'; export { default as getElementByReference } from './get-element-by-reference'; export { default as getElementCoordinates } from './get-element-coordinates'; @@ -39,3 +40,5 @@ export { default as shadowElementsFromPoint } from './shadow-elements-from-point export { default as urlPropsFromAttribute } from './url-props-from-attribute'; export { default as visuallyContains } from './visually-contains'; export { default as visuallyOverlaps } from './visually-overlaps'; +export { default as visuallySort } from './visually-sort'; +export { createGrid } from './get-rect-stack'; diff --git a/lib/commons/dom/visually-sort.js b/lib/commons/dom/visually-sort.js new file mode 100644 index 0000000000..aff9ffc314 --- /dev/null +++ b/lib/commons/dom/visually-sort.js @@ -0,0 +1,137 @@ +/** + * Visually sort nodes based on their stack order + * References: + * https://www.w3.org/Style/css2-updates/css2/zindex.html + * @param {VirtualNode} + * @param {VirtualNode} + */ +export default function visuallySort(a, b) { + /*eslint no-bitwise: 0 */ + const length = Math.max(a._stackingOrder.length, b._stackingOrder.length); + + for (let i = 0; i < length; i++) { + if (typeof b._stackingOrder[i] === 'undefined') { + return -1; + } else if (typeof a._stackingOrder[i] === 'undefined') { + return 1; + } + + // 7. the child stacking contexts with positive stack levels (least positive first). + if (b._stackingOrder[i] > a._stackingOrder[i]) { + return 1; + } + + // 2. the child stacking contexts with negative stack levels (most negative first). + if (b._stackingOrder[i] < a._stackingOrder[i]) { + return -1; + } + } + + // nodes are the same stacking order + let aNode = a.actualNode; + let bNode = b.actualNode; + + // elements don't correctly calculate document position when comparing + // across shadow boundaries, so we need to compare the position of a + // shared host instead + + // elements have different hosts + if (aNode.getRootNode && aNode.getRootNode() !== bNode.getRootNode()) { + // keep track of all parent hosts and find the one both nodes share + const boundaries = []; + while (aNode) { + boundaries.push({ + root: aNode.getRootNode(), + node: aNode + }); + aNode = aNode.getRootNode().host; + } + + while ( + bNode && + !boundaries.find(boundary => boundary.root === bNode.getRootNode()) + ) { + bNode = bNode.getRootNode().host; + } + + // bNode is a node that shares a host with some part of the a parent + // shadow tree, find the aNode that shares the same host as bNode + aNode = boundaries.find( + boundary => boundary.root === bNode.getRootNode() + ).node; + + // sort child of shadow to it's host node by finding which element is + // the child of the host and sorting it before the host + if (aNode === bNode) { + return a.actualNode.getRootNode() !== aNode.getRootNode() ? -1 : 1; + } + } + + const { + DOCUMENT_POSITION_FOLLOWING, + DOCUMENT_POSITION_CONTAINS, + DOCUMENT_POSITION_CONTAINED_BY + } = window.Node; + + const docPosition = aNode.compareDocumentPosition(bNode); + const DOMOrder = docPosition & DOCUMENT_POSITION_FOLLOWING ? 1 : -1; + const isDescendant = + docPosition & DOCUMENT_POSITION_CONTAINS || + docPosition & DOCUMENT_POSITION_CONTAINED_BY; + const aPosition = getPositionOrder(a); + const bPosition = getPositionOrder(b); + + // a child of a positioned element should also be on top of the parent + if (aPosition === bPosition || isDescendant) { + return DOMOrder; + } + return bPosition - aPosition; +} + +/** + * Return the index order of how to position this element. return nodes in non-positioned, floating, positioned order + * References: + * https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Positioning/Understanding_z_index/Stacking_without_z-index + * https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Positioning/Understanding_z_index/Stacking_and_float + * https://www.w3.org/Style/css2-updates/css2/zindex.html + * @param {VirtualNode} vNode + * @return {Number} + */ +function getPositionOrder(vNode) { + // 5. the in-flow, inline-level, non-positioned descendants, including inline tables and inline blocks. + if (vNode.getComputedStylePropertyValue('display').indexOf('inline') !== -1) { + return 2; + } + // 4. the non-positioned floats. + if (isFloated(vNode)) { + return 1; + } + // 3. the in-flow, non-inline-level, non-positioned descendants. + return 0; +} + +/** + * Check if a node or one of it's parents is floated. + * Floating position should be inherited from the parent tree + * @see https://github.com/dequelabs/axe-core/issues/2222 + */ +function isFloated(vNode) { + if (!vNode) { + return false; + } + + if (vNode._isFloated !== undefined) { + return vNode._isFloated; + } + + const floatStyle = vNode.getComputedStylePropertyValue('float'); + + if (floatStyle !== 'none') { + vNode._isFloated = true; + return true; + } + + const floated = isFloated(vNode.parent); + vNode._isFloated = floated; + return floated; +} diff --git a/lib/commons/index.js b/lib/commons/index.js index 7f8779767c..cbbf9d7e4e 100644 --- a/lib/commons/index.js +++ b/lib/commons/index.js @@ -8,6 +8,7 @@ */ import * as aria from './aria'; import * as color from './color'; +import * as math from './math'; import * as dom from './dom'; import * as forms from './forms'; import matches from './matches'; @@ -21,6 +22,7 @@ var commons = { color, dom, forms, + math, matches, standards, table, @@ -28,4 +30,15 @@ var commons = { utils }; -export { aria, color, dom, forms, matches, standards, table, text, utils }; +export { + aria, + color, + dom, + forms, + math, + matches, + standards, + table, + text, + utils +}; diff --git a/lib/commons/math/get-offset.js b/lib/commons/math/get-offset.js new file mode 100644 index 0000000000..b25b81044f --- /dev/null +++ b/lib/commons/math/get-offset.js @@ -0,0 +1,147 @@ +/** + * Get the offset between node A and node B + * @param {VirtualNode} vNodeA + * @param {VirtualNode} vNodeB + * @returns {number} + */ +export default function getOffset(vNodeA, vNodeB) { + const rectA = vNodeA.boundingClientRect; + const rectB = vNodeB.boundingClientRect; + const pointA = getFarthestPoint(rectA, rectB); + const pointB = getClosestPoint(pointA, rectA, rectB); + return pointDistance(pointA, pointB); +} + +/** + * Get a point on rectA that is farthest away from rectB + * @param {Rect} rectA + * @param {Rect} rectB + * @returns {Point} + */ +function getFarthestPoint(rectA, rectB) { + const dimensionProps = [ + ['x', 'left', 'right', 'width'], + ['y', 'top', 'bottom', 'height'] + ]; + const farthestPoint = {}; + dimensionProps.forEach(([axis, start, end, diameter]) => { + if (rectB[start] < rectA[start] && rectB[end] > rectA[end]) { + farthestPoint[axis] = rectA[start] + rectA[diameter] / 2; // center | middle + return; + } + // Work out which edge of A is farthest away from the center of B + const centerB = rectB[start] + rectB[diameter] / 2; + const startDistance = Math.abs(centerB - rectA[start]); + const endDistance = Math.abs(centerB - rectA[end]); + if (startDistance >= endDistance) { + farthestPoint[axis] = rectA[start]; // left | top + } else if (startDistance < endDistance) { + farthestPoint[axis] = rectA[end]; // right | bottom + } + }); + return farthestPoint; +} + +/** + * Get a point on the adjacentRect, that is as close the point given from ownRect + * @param {Point} ownRectPoint + * @param {Rect} ownRect + * @param {Rect} adjacentRect + * @returns {Point} + */ +function getClosestPoint({ x, y }, ownRect, adjacentRect) { + if (pointInRect({ x, y }, adjacentRect)) { + // Check if there is an opposite corner inside the adjacent rectangle + const closestPoint = getCornerInAdjacentRect( + { x, y }, + ownRect, + adjacentRect + ); + if (closestPoint !== null) { + return closestPoint; + } + adjacentRect = ownRect; + } + + const { top, right, bottom, left } = adjacentRect; + // Is the adjacent rect horizontally or vertically aligned + const xAligned = x >= left && x <= right; + const yAligned = y >= top && y <= bottom; + // Find the closest edge of the adjacent rect + const closestX = Math.abs(left - x) < Math.abs(right - x) ? left : right; + const closestY = Math.abs(top - y) < Math.abs(bottom - y) ? top : bottom; + + if (!xAligned && yAligned) { + return { x: closestX, y }; // Closest horizontal point + } else if (xAligned && !yAligned) { + return { x, y: closestY }; // Closest vertical point + } else if (!xAligned && !yAligned) { + return { x: closestX, y: closestY }; // Closest diagonal corner + } + // ownRect (partially) obscures adjacentRect + if (Math.abs(x - closestX) < Math.abs(y - closestY)) { + return { x: closestX, y }; // Inside, closest edge is horizontal + } else { + return { x, y: closestY }; // Inside, closest edge is vertical + } +} + +/** + * Distance between two points + * @param {Point} pointA + * @param {Point} pointB + * @returns {number} + */ +function pointDistance(pointA, pointB) { + const xDistance = Math.abs(pointA.x - pointB.x); + const yDistance = Math.abs(pointA.y - pointB.y); + if (!xDistance || !yDistance) { + return xDistance || yDistance; // If either is 0, return the other + } + return Math.sqrt(Math.pow(xDistance, 2) + Math.pow(yDistance, 2)); +} + +/** + * Return if a point is within a rect + * @param {Point} point + * @param {Rect} rect + * @returns {boolean} + */ +function pointInRect({ x, y }, rect) { + return y >= rect.top && x <= rect.right && y <= rect.bottom && x >= rect.left; +} + +/** + * + * @param {Point} ownRectPoint + * @param {Rect} ownRect + * @param {Rect} adjacentRect + * @returns + */ +function getCornerInAdjacentRect({ x, y }, ownRect, adjacentRect) { + let closestX, closestY; + // Find the opposite corner, if it is inside the adjacent rect; + if (x === ownRect.left && ownRect.right < adjacentRect.right) { + closestX = ownRect.right; + } else if (x === ownRect.right && ownRect.left > adjacentRect.left) { + closestX = ownRect.left; + } + if (y === ownRect.top && ownRect.bottom < adjacentRect.bottom) { + closestY = ownRect.bottom; + } else if (y === ownRect.bottom && ownRect.top > adjacentRect.top) { + closestY = ownRect.top; + } + + if (!closestX && !closestY) { + return null; // opposite corners are outside the rect, or {x,y} was a center point + } else if (!closestY) { + return { x: closestX, y }; + } else if (!closestX) { + return { x, y: closestY }; + } + if (Math.abs(x - closestX) < Math.abs(y - closestY)) { + return { x: closestX, y }; + } else { + return { x, y: closestY }; + } +} diff --git a/lib/commons/math/has-visual-overlap.js b/lib/commons/math/has-visual-overlap.js new file mode 100644 index 0000000000..40bfe140ce --- /dev/null +++ b/lib/commons/math/has-visual-overlap.js @@ -0,0 +1,44 @@ +import { visuallySort } from '../dom'; + +/** + * Check if node A overlaps B + * @param {VirtualNode} vNodeA + * @param {VirtualNode} vNodeB + * @returns 0 | 1 | -1 + */ +export default function hasVisualOverlap(vNodeA, vNodeB) { + const overlapRect = getRectOverlap( + vNodeA.boundingClientRect, + vNodeB.boundingClientRect + ); + if (overlapRect === null || overlapRect.width < 1 || overlapRect.height < 1) { + return false; // No overlap + } + // Check B is rendered on top of A + return visuallySort(vNodeA, vNodeB) > 0; +} + +// Return the overlapping area of two rects, null if none +function getRectOverlap(rectA, rectB) { + const baseRect = { + top: Math.max(rectA.top, rectB.top), + left: Math.max(rectA.left, rectB.left), + bottom: Math.min(rectA.bottom, rectB.bottom), + right: Math.min(rectA.right, rectB.right) + }; + if (baseRect.top > baseRect.bottom || baseRect.left > baseRect.right) { + return null; // no overlap + } + + return computeRect(baseRect); +} + +function computeRect(baseRect) { + return { + ...baseRect, + x: baseRect.left, + y: baseRect.top, + height: baseRect.bottom - baseRect.top, + width: baseRect.right - baseRect.left + }; +} diff --git a/lib/commons/math/index.js b/lib/commons/math/index.js new file mode 100644 index 0000000000..82b296f49b --- /dev/null +++ b/lib/commons/math/index.js @@ -0,0 +1,3 @@ +export { default as getOffset } from './get-offset'; +export { default as hasVisualOverlap } from './has-visual-overlap'; +export { default as splitRects } from './split-rects'; diff --git a/lib/commons/math/split-rects.js b/lib/commons/math/split-rects.js new file mode 100644 index 0000000000..08fa7f08e0 --- /dev/null +++ b/lib/commons/math/split-rects.js @@ -0,0 +1,57 @@ +/** + * Given an outer rect, and a list of rects that overlap with it, find any rectangular + * space that does not overlap. + * @param {Rect} outerRect + * @param {Rect[]} overlapRects + * @returns uniqueRects {Rect[]} + */ +export default function splitRects(outerRect, overlapRects) { + let uniqueRects = [outerRect]; + for (const overlapRect of overlapRects) { + uniqueRects = uniqueRects.reduce((uniqueRects, inputRect) => { + return uniqueRects.concat(splitRect(inputRect, overlapRect)); + }, []); + } + return uniqueRects; +} + +// Cut the input rect along any intersecting edge of the clip rect. +function splitRect(inputRect, clipRect) { + const { top, left, bottom, right } = inputRect; + const yAligned = top < clipRect.bottom && bottom > clipRect.top; + const xAligned = left < clipRect.right && right > clipRect.left; + + const rects = []; + if (between(clipRect.top, top, bottom) && xAligned) { + // console.log('top') + rects.push({ top, left, bottom: clipRect.top, right }); + } + if (between(clipRect.right, left, right) && yAligned) { + // console.log('right', clipRect.right, left, right, top, clipRect.bottom, bottom, inputRect.top) + rects.push({ top, left: clipRect.right, bottom, right }); + } + if (between(clipRect.bottom, top, bottom) && xAligned) { + // console.log('bottom') + rects.push({ top: clipRect.bottom, right, bottom, left }); + } + if (between(clipRect.left, left, right) && yAligned) { + // console.log('left', clipRect.left, left, right) + rects.push({ top, left, bottom, right: clipRect.left }); + } + if (rects.length === 0) { + rects.push(inputRect); // No intersection + } + return rects.map(computeRect); // add x / y / width / height +} + +const between = (num, min, max) => num > min && num < max; + +function computeRect(baseRect) { + return { + ...baseRect, + x: baseRect.left, + y: baseRect.top, + height: baseRect.bottom - baseRect.top, + width: baseRect.right - baseRect.left + }; +} diff --git a/lib/core/constants.js b/lib/core/constants.js index 657f51dca5..3880c0a16e 100644 --- a/lib/core/constants.js +++ b/lib/core/constants.js @@ -27,6 +27,7 @@ const definitions = [ const constants = { helpUrlBase: 'https://dequeuniversity.com/rules/', + gridSize: 200, results: [], resultGroups: [], resultGroupMap: {}, diff --git a/lib/rules/target-spacing.json b/lib/rules/target-spacing.json new file mode 100644 index 0000000000..8c617b3598 --- /dev/null +++ b/lib/rules/target-spacing.json @@ -0,0 +1,12 @@ +{ + "id": "target-size", + "selector": "a[href], button, *[role=link]", + "tags": ["wcag22aa", "sc1412", "experimental", "cat.sensory-and-visual-cues"], + "metadata": { + "description": "Ensure touch target have sufficient size and space", + "help": "All touch targets must be 24px large, or leave sufficient space" + }, + "all": [], + "any": ["target-size", "target-offset"], + "none": [] +} diff --git a/locales/_template.json b/locales/_template.json index 454b4c1d40..c78049bb25 100644 --- a/locales/_template.json +++ b/locales/_template.json @@ -373,6 +373,10 @@ "description": "Ensure that tables with a caption use the element.", "help": "Data or header cells must not be used to give caption to a data table." }, + "target-size": { + "description": "Ensure touch target have sufficient size and space", + "help": "All touch targets must be 24px large, or leave sufficient space" + }, "td-has-header": { "description": "Ensure that each non-empty data cell in a larger than 3 by 3 has one or more table headers", "help": "Non-empty
elements in larger must have an associated table header" @@ -472,7 +476,8 @@ "pass": "Required ARIA children are present", "fail": { "singular": "Required ARIA child role not present: ${data.values}", - "plural": "Required ARIA children role not present: ${data.values}" + "plural": "Required ARIA children role not present: ${data.values}", + "unallowed": "Element has children which are not allowed (see related nodes)" }, "incomplete": { "singular": "Expecting ARIA child role to be added: ${data.values}", @@ -810,6 +815,17 @@ "pass": " tag does not disable zooming on mobile devices", "fail": "${data} on tag disables zooming on mobile devices" }, + "target-offset": { + "pass": "Target is sufficiently spaced from its neighbours", + "fail": "Target is insufficiently spaced from its neighbours" + }, + "target-size": { + "pass": "Control has sufficient size (${data.width}px width, ${data.height}px height)", + "fail": { + "default": "Element has insufficient size (${data.width}px width, ${data.height}px height)", + "obscured": "Element has insufficient size because it is obscured (smallest space is ${data.width}px width by ${data.height}px height)" + } + }, "header-present": { "pass": "Page has a heading", "fail": "Page does not have a heading" diff --git a/test/checks/mobile/target-offset.js b/test/checks/mobile/target-offset.js new file mode 100644 index 0000000000..1626636ce0 --- /dev/null +++ b/test/checks/mobile/target-offset.js @@ -0,0 +1,15 @@ +describe('target-offset tests', function () { + 'use strict'; + + var checkContext = axe.testUtils.MockCheckContext(); + // var checkSetup = axe.testUtils.checkSetup; + // var shadowCheckSetup = axe.testUtils.shadowCheckSetup; + // var shadowSupport = axe.testUtils.shadowSupport; + // var check = checks['target-offset']; + + afterEach(function () { + checkContext.reset(); + }); + + it('returns false for targets smaller than minSpacing'); +}); diff --git a/test/checks/mobile/target-size.js b/test/checks/mobile/target-size.js new file mode 100644 index 0000000000..9fd4ec10c5 --- /dev/null +++ b/test/checks/mobile/target-size.js @@ -0,0 +1,90 @@ +describe('target-size tests', function () { + 'use strict'; + + var checkContext = axe.testUtils.MockCheckContext(); + var checkSetup = axe.testUtils.checkSetup; + var shadowCheckSetup = axe.testUtils.shadowCheckSetup; + var shadowSupport = axe.testUtils.shadowSupport; + var check = checks['target-size']; + + afterEach(function () { + checkContext.reset(); + }); + + it('returns false for targets smaller than minSpacing', function () { + var checkArgs = checkSetup( + '' + ); + assert.isFalse(check.evaluate.apply(checkContext, checkArgs)); + assert.deepEqual(checkContext._data, { width: 20, height: 30 }); + }); + + it('returns true for unobscured targets larger than minSpacing', function () { + var checkArgs = checkSetup( + '' + ); + assert.isTrue(check.evaluate.apply(checkContext, checkArgs)); + assert.deepEqual(checkContext._data, { + width: 40, + height: 30 + }); + }); + + it('returns true for obscured targets with insufficient space', function () { + var checkArgs = checkSetup( + '' + + '' + ); + assert.isTrue(check.evaluate.apply(checkContext, checkArgs)); + assert.deepEqual(checkContext._data, { width: 30, height: 30 }); + }); + + it('returns false for obscured targets with insufficient space', function () { + var checkArgs = checkSetup( + '' + + '' + + '' + ); + assert.isFalse(check.evaluate.apply(checkContext, checkArgs)); + assert.deepEqual(checkContext._data, { + messageKey: 'obscured', + width: 20, + height: 30 + }); + }); + + (shadowSupport ? it : xit)('works across shadow boundaries', function () { + var checkArgs = shadowCheckSetup( + '' + + '' + + '', + '' + ); + assert.isFalse(check.evaluate.apply(checkContext, checkArgs)); + assert.deepEqual(checkContext._data, { + messageKey: 'obscured', + width: 20, + height: 30 + }); + }); +}); diff --git a/test/commons/dom/find-nearby-elms.js b/test/commons/dom/find-nearby-elms.js new file mode 100644 index 0000000000..3468ded228 --- /dev/null +++ b/test/commons/dom/find-nearby-elms.js @@ -0,0 +1,41 @@ +describe('findNearbyElms', function () { + 'use strict'; + var fixtureSetup = axe.testUtils.fixtureSetup; + var findNearbyElms = axe.commons.dom.findNearbyElms; + var fixture; + + function getIds(vNodeList) { + var ids = []; + vNodeList.forEach(function (vNode) { + if (vNode.props.id && vNode.props.id !== 'fixture') { + ids.push(vNode.props.id); + } + }); + return ids; + } + + beforeEach(function () { + fixture = fixtureSetup( + '
0
' + + '
1
' + + '
2
' + + '
3
' + + '
4
' + + '
5
' + + '
6
' + + '
7
' + + '
8
' + + '
9
' + ); + }); + + it('returns node from the same grid cell', function () { + var nearbyElms = findNearbyElms(fixture.children[1]); + assert.deepEqual(getIds(nearbyElms), ['n0', 'n2', 'n3']); + }); + + it('returns node from multiple grid cells when crossing a boundary', function () { + var nearbyElms = findNearbyElms(fixture.children[5]); + assert.deepEqual(getIds(nearbyElms), ['n3', 'n4', 'n6']); + }); +}); diff --git a/test/commons/dom/visually-sort.js b/test/commons/dom/visually-sort.js new file mode 100644 index 0000000000..fe764e9fdd --- /dev/null +++ b/test/commons/dom/visually-sort.js @@ -0,0 +1,28 @@ +// This method is mostly tested through color-contrast integrations +describe('visually-sort', function () { + 'use strict'; + + var fixtureSetup = axe.testUtils.fixtureSetup; + var visuallySort = axe.commons.dom.visuallySort; + + // visuallySort requires the grid to be set up + // For performance reasons axe-core does not do this in setup, + // so we'll need to call it ourselves. + var createGrid = axe.commons.dom.createGrid; + + it('returns 1 if B overlaps A', function () { + var rootNode = fixtureSetup('bar'); + createGrid(); + var vNodeA = rootNode.children[0]; + var vNodeB = vNodeA.children[0]; + assert.equal(visuallySort(vNodeA, vNodeB), 1); + }); + + it('returns -1 if A overlaps B', function () { + var rootNode = fixtureSetup('bar'); + createGrid(); + var vNodeB = rootNode.children[0]; + var vNodeA = vNodeB.children[0]; + assert.equal(visuallySort(vNodeA, vNodeB), -1); + }); +}); diff --git a/test/commons/math/get-offset.js b/test/commons/math/get-offset.js new file mode 100644 index 0000000000..4eff6de509 --- /dev/null +++ b/test/commons/math/get-offset.js @@ -0,0 +1,103 @@ +describe('getOffset', function () { + 'use strict'; + var fixtureSetup = axe.testUtils.fixtureSetup; + var getOffset = axe.commons.math.getOffset; + + // Return the diagonal of a square of size X, or rectangle of size X * Y + function getDiagonal(x, y) { + y = typeof y === 'number' ? y : x; + return Math.sqrt(Math.pow(x, 2) + Math.pow(y, 2)); + } + + it('returns with + spacing for horizontally adjacent elms', function () { + var fixture = fixtureSetup( + ' ' + + ' ' + ); + var nodeA = fixture.children[0]; + var nodeB = fixture.children[1]; + assert.equal(getOffset(nodeA, nodeB), 40); + assert.equal(getOffset(nodeB, nodeA), 30); + }); + + it('returns closest horizontal distance for elements horizontally aligned', function () { + var fixture = fixtureSetup( + ' ' + + ' ' + ); + var nodeA = fixture.children[0]; + var nodeB = fixture.children[1]; + assert.closeTo(getOffset(nodeA, nodeB), getDiagonal(40, 5), 0.1); + assert.equal(getOffset(nodeB, nodeA), 30); + }); + + it('returns height + spacing for vertically adjacent elms', function () { + var fixture = fixtureSetup( + ' ' + + ' ' + ); + var nodeA = fixture.children[0]; + var nodeB = fixture.children[1]; + assert.equal(getOffset(nodeA, nodeB), 40); + assert.equal(getOffset(nodeB, nodeA), 30); + }); + + it('returns closest vertical distance for elements horizontally aligned', function () { + var fixture = fixtureSetup( + ' ' + + ' ' + ); + var nodeA = fixture.children[0]; + var nodeB = fixture.children[1]; + + assert.closeTo(getOffset(nodeA, nodeB), getDiagonal(40, 10), 0.1); + assert.equal(getOffset(nodeB, nodeA), 30); + }); + + it('returns corner to corner distance for diagonal elms', function () { + var fixture = fixtureSetup( + ' ' + + ' ' + ); + var nodeA = fixture.children[0]; + var nodeB = fixture.children[1]; + assert.closeTo(getOffset(nodeA, nodeB), getDiagonal(40), 0.1); + assert.closeTo(getOffset(nodeB, nodeA), getDiagonal(30), 0.1); + }); + + it('returns the distance to the edge when elements overlap on an edge', function () { + var fixture = fixtureSetup( + '' + + ' ' + + '' + ); + var nodeA = fixture.children[0]; + var nodeB = nodeA.children[0]; + assert.equal(getOffset(nodeA, nodeB), 30); + assert.equal(getOffset(nodeB, nodeA), 30); + }); + + it('returns the shortest side of the element when an element overlaps on a corner', function () { + var fixture = fixtureSetup( + '' + + ' ' + + '' + ); + var nodeA = fixture.children[0]; + var nodeB = nodeA.children[0]; + assert.closeTo(getOffset(nodeA, nodeB), getDiagonal(30), 0.1); + assert.equal(getOffset(nodeB, nodeA), 20); + }); + + it('returns smallest diagonal if elmA fully covers elmB', function () { + var fixture = fixtureSetup( + '' + + ' ' + + '' + ); + var nodeA = fixture.children[0]; + var nodeB = nodeA.children[0]; + assert.closeTo(getOffset(nodeA, nodeB), getDiagonal(10), 0.1); + assert.equal(getOffset(nodeB, nodeA), 10); + }); +}); diff --git a/test/commons/math/has-visual-overlap b/test/commons/math/has-visual-overlap new file mode 100644 index 0000000000..f06ce628a7 --- /dev/null +++ b/test/commons/math/has-visual-overlap @@ -0,0 +1,34 @@ +describe('hasVisualOverlap', function () { + 'use strict'; + var fixtureSetup = axe.testUtils.fixtureSetup; + var hasVisualOverlap = axe.commons.math.hasVisualOverlap; + + // hasVisualOverlap requires the grid to be set up + // For performance reasons axe-core does not do this in setup, + // so we'll need to call it ourselves. + var createGrid = axe.commons.dom.createGrid; + + it('returns false if there is no overlap', function () { + var rootNode = fixtureSetup('foobar'); + createGrid(); + var vNodeA = rootNode.children[0]; + var vNodeB = rootNode.children[1]; + assert.isFalse(hasVisualOverlap(vNodeA, vNodeB)); + }); + + it('returns true if B overlaps A', function () { + var rootNode = fixtureSetup('bar'); + createGrid(); + var vNodeA = rootNode.children[0]; + var vNodeB = vNodeA.children[0]; + assert.isTrue(hasVisualOverlap(vNodeA, vNodeB)); + }); + + it('returns true A overlaps B', function () { + var rootNode = fixtureSetup('bar'); + createGrid(); + var vNodeB = rootNode.children[0]; + var vNodeA = vNodeB.children[0]; + assert.isFalse(hasVisualOverlap(vNodeA, vNodeB)); + }); +}); diff --git a/test/commons/math/split-rects.js b/test/commons/math/split-rects.js new file mode 100644 index 0000000000..714e03f24d --- /dev/null +++ b/test/commons/math/split-rects.js @@ -0,0 +1,92 @@ +describe('splitRects', function () { + var splitRects = axe.commons.math.splitRects; + function createRect(x, y, width, height) { + return { + x: x, + y: y, + width: width, + height: height, + top: y, + left: x, + bottom: y + height, + right: x + width + }; + } + + it('returns the original rect if there is no clipping rect', function () { + var rectA = createRect(0, 0, 100, 50); + var rects = splitRects(rectA, []); + assert.lengthOf(rects, 1); + assert.deepEqual(rects[0], rectA); + }); + + it('returns the original rect if there is no overlap', function () { + var rectA = createRect(0, 0, 100, 50); + var rectB = createRect(0, 50, 50, 50); + var rects = splitRects(rectA, [rectB]); + assert.lengthOf(rects, 1); + assert.deepEqual(rects[0], rectA); + }); + + describe('with one overlapping rect', function () { + it('returns one rect if overlaps covers two corners', function () { + var rectA = createRect(0, 0, 100, 50); + var rectB = createRect(40, 0, 100, 50); + var rects = splitRects(rectA, [rectB]); + assert.lengthOf(rects, 1); + assert.deepEqual(rects[0], createRect(0, 0, 40, 50)); + }); + + it('returns two rects if overlap covers one corner', function () { + var rectA = createRect(0, 0, 100, 100); + var rectB = createRect(50, 50, 50, 50); + var rects = splitRects(rectA, [rectB]); + assert.lengthOf(rects, 2); + assert.deepEqual(rects[0], createRect(0, 0, 100, 50)); + assert.deepEqual(rects[1], createRect(0, 0, 50, 100)); + }); + + it('returns three rects if overlap covers an edge, but no corner', function () { + var rectA = createRect(0, 0, 100, 150); + var rectB = createRect(50, 50, 50, 50); + var rects = splitRects(rectA, [rectB]); + assert.lengthOf(rects, 3); + assert.deepEqual(rects[0], createRect(0, 0, 100, 50)); + assert.deepEqual(rects[1], createRect(0, 100, 100, 50)); + assert.deepEqual(rects[2], createRect(0, 0, 50, 150)); + }); + + it('returns four rects if overlap sits in the middle, touching no corner', function () { + var rectA = createRect(0, 0, 150, 150); + var rectB = createRect(50, 50, 50, 50); + var rects = splitRects(rectA, [rectB]); + assert.lengthOf(rects, 4); + assert.deepEqual(rects[0], createRect(0, 0, 150, 50)); + assert.deepEqual(rects[1], createRect(100, 0, 50, 150)); + assert.deepEqual(rects[2], createRect(0, 100, 150, 50)); + assert.deepEqual(rects[3], createRect(0, 0, 50, 150)); + }); + }); + + describe('with multiple overlaps', function () { + it('can return a single rect two overlaps each cover an edge', function () { + var rectA = createRect(0, 0, 150, 50); + var rectB = createRect(0, 0, 50, 50); + var rectC = createRect(100, 0, 50, 50); + var rects = splitRects(rectA, [rectB, rectC]); + assert.lengthOf(rects, 1); + assert.deepEqual(rects[0], createRect(50, 0, 50, 50)); + }); + + it('can recursively clips regions', function () { + var rectA = createRect(0, 0, 150, 100); + var rectB = createRect(0, 50, 50, 50); + var rectC = createRect(100, 50, 50, 50); + var rects = splitRects(rectA, [rectB, rectC]); + assert.lengthOf(rects, 3); + assert.deepEqual(rects[0], createRect(0, 0, 150, 50)); + assert.deepEqual(rects[1], createRect(50, 0, 100, 50)); + assert.deepEqual(rects[2], createRect(50, 0, 50, 100)); + }); + }); +}); diff --git a/test/integration/full/target-size/target-size.html b/test/integration/full/target-size/target-size.html new file mode 100644 index 0000000000..ed45aaf798 --- /dev/null +++ b/test/integration/full/target-size/target-size.html @@ -0,0 +1,652 @@ + + + + Region Test + + + + + + + +
+ + +

Examples A1 - A4 all pass.

+
+
+ + + +
+
+ + + +
+
+ + + +
+
+ + + +
+
+ +

Examples B1 - B3 pass, B4 fails.

+
+
+ + + +
+
+ + + +
+
+ + + +
+
+ + + +
+
+ +

Examples C1 and C2 pass, C3 and C4 fail.

+
+
+ + + +
+
+ + + +
+
+ + + +
+
+ + + +
+
+ +

Example D1 passes, D2 - D4 fail.

+
+
+ + + +
+
+ + + +
+
+ + + +
+
+ + + +
+
+ +

Example E1 and E2 pass, the two outside elements of E3 and E4 fail.

+
+
+ + + +
+
+ + + +
+
+ + + +
+
+ + + +
+
+ +

Example F1 and F2 pass, the inside element of F3 and F4 fail.

+
+
+ + + +
+
+ + + +
+
+ + + +
+
+ + + +
+
+ +

Example G1 - G3 pass, the outer element of G4 fail.

+
+
+ + + +
+
+ + + +
+
+ + + +
+
+ + + +
+
+ +

Example H1 and H2 pass, the outer element of H3 and H4 fail.

+
+
+ + + +
+
+ + + +
+
+ + + +
+
+ + + + +
+
+ +

Example I1 and I2 pass, I3 and I4 fail.

+
+
+ + + +
+
+ + + +
+
+ + + +
+
+ + + +
+
+ +

Example J1 and J2 pass, J3 and J4 fail.

+
+
+ + + +
+
+ + + +
+
+ + + +
+
+ + + +
+
+ +

Example K1 - K3 pass, the middle element of K4 fails.

+
+
+ + + +
+
+ + + +
+
+ + + +
+
+ + + +
+
+ +

Example L1 and L2 pass, the outer element of L3 and L4 fail.

+
+
+ + + +
+
+
+ + + +
+
+
+ + + +
+
+ + + +
+
+ +

Example M1 - M3 pass, the outer element of M4 fail.

+
+
+ + + +
+
+ + + +
+
+ + + +
+
+ + + +
+
+ +

Example N1 - N3 pass, the outer element of N4 fail.

+
+
+ + + +
+
+ + + +
+
+ + + +
+
+ + + +
+
+ +

Example O1 and O2 pass, the inner element of O3 and O4 fail.

+
+
+ + + + +
+
+ + + + +
+
+ + + + +
+
+ + + + +
+
+ +

Example P.

+
+
+ + + +
+
+ + + +
+
+ + + +
+
+ + + +
+
+ + + + + + diff --git a/test/integration/full/target-size/target-size.js b/test/integration/full/target-size/target-size.js new file mode 100644 index 0000000000..b9652e2390 --- /dev/null +++ b/test/integration/full/target-size/target-size.js @@ -0,0 +1,53 @@ +describe('target-size test', function () { + 'use strict'; + var results; + + before(function (done) { + axe.testUtils.awaitNestedLoad(function () { + var options = { + runOnly: ['target-size'], + elementRef: true + }; + axe.run('section', options, function (err, r) { + if (err) { + done(err); + } + results = r; + // Add some highlighting for visually identifying issues. + // There are too many test cases to just do this by selector. + results.violations[0] && + results.violations[0].nodes.forEach(function (node) { + node.element.className += ' violations'; + }); + results.passes[0] && + results.passes[0].nodes.forEach(function (node) { + node.element.className += ' passes'; + }); + console.log(results); + done(); + }); + }); + }); + + it('finds all passing nodes', function () { + var passResults = results.passes[0] ? results.passes[0].nodes : []; + var passedElms = document.querySelectorAll( + 'section:not([hidden]) div:not([hidden]) .passed' + ); + passResults.forEach(function (result) { + assert.include(passedElms, result.element); + }); + assert.lengthOf(passResults, passedElms.length); + }); + + it('finds all failed nodes', function () { + var failResults = results.violations[0] ? results.violations[0].nodes : []; + var failedElms = document.querySelectorAll( + 'section:not([hidden]) div:not([hidden]) .failed' + ); + failResults.forEach(function (result) { + assert.include(failedElms, result.element); + }); + assert.lengthOf(failResults, failedElms.length); + }); +}); From 54817d7801ef35472bbce1e706b9b24c878a1a5f Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Thu, 3 Feb 2022 12:23:47 +0100 Subject: [PATCH 02/28] chore: handle rounding errors --- locales/_template.json | 2 +- test/commons/math/get-offset.js | 33 +++++++++++++++++---------------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/locales/_template.json b/locales/_template.json index c78049bb25..994d767980 100644 --- a/locales/_template.json +++ b/locales/_template.json @@ -1045,4 +1045,4 @@ } }, "incompleteFallbackMessage": "axe couldn't tell the reason. Time to break out the element inspector!" -} +} \ No newline at end of file diff --git a/test/commons/math/get-offset.js b/test/commons/math/get-offset.js index 4eff6de509..9214d6a861 100644 --- a/test/commons/math/get-offset.js +++ b/test/commons/math/get-offset.js @@ -2,6 +2,7 @@ describe('getOffset', function () { 'use strict'; var fixtureSetup = axe.testUtils.fixtureSetup; var getOffset = axe.commons.math.getOffset; + var round = 0.2; // Return the diagonal of a square of size X, or rectangle of size X * Y function getDiagonal(x, y) { @@ -16,8 +17,8 @@ describe('getOffset', function () { ); var nodeA = fixture.children[0]; var nodeB = fixture.children[1]; - assert.equal(getOffset(nodeA, nodeB), 40); - assert.equal(getOffset(nodeB, nodeA), 30); + assert.closeTo(getOffset(nodeA, nodeB), 40, round); + assert.closeTo(getOffset(nodeB, nodeA), 30, round); }); it('returns closest horizontal distance for elements horizontally aligned', function () { @@ -27,8 +28,8 @@ describe('getOffset', function () { ); var nodeA = fixture.children[0]; var nodeB = fixture.children[1]; - assert.closeTo(getOffset(nodeA, nodeB), getDiagonal(40, 5), 0.1); - assert.equal(getOffset(nodeB, nodeA), 30); + assert.closeTo(getOffset(nodeA, nodeB), getDiagonal(40, 5), round); + assert.closeTo(getOffset(nodeB, nodeA), 30, round); }); it('returns height + spacing for vertically adjacent elms', function () { @@ -38,8 +39,8 @@ describe('getOffset', function () { ); var nodeA = fixture.children[0]; var nodeB = fixture.children[1]; - assert.equal(getOffset(nodeA, nodeB), 40); - assert.equal(getOffset(nodeB, nodeA), 30); + assert.closeTo(getOffset(nodeA, nodeB), 40, round); + assert.closeTo(getOffset(nodeB, nodeA), 30, round); }); it('returns closest vertical distance for elements horizontally aligned', function () { @@ -50,8 +51,8 @@ describe('getOffset', function () { var nodeA = fixture.children[0]; var nodeB = fixture.children[1]; - assert.closeTo(getOffset(nodeA, nodeB), getDiagonal(40, 10), 0.1); - assert.equal(getOffset(nodeB, nodeA), 30); + assert.closeTo(getOffset(nodeA, nodeB), getDiagonal(40, 10), round); + assert.closeTo(getOffset(nodeB, nodeA), 30, round); }); it('returns corner to corner distance for diagonal elms', function () { @@ -61,8 +62,8 @@ describe('getOffset', function () { ); var nodeA = fixture.children[0]; var nodeB = fixture.children[1]; - assert.closeTo(getOffset(nodeA, nodeB), getDiagonal(40), 0.1); - assert.closeTo(getOffset(nodeB, nodeA), getDiagonal(30), 0.1); + assert.closeTo(getOffset(nodeA, nodeB), getDiagonal(40), round); + assert.closeTo(getOffset(nodeB, nodeA), getDiagonal(30), round); }); it('returns the distance to the edge when elements overlap on an edge', function () { @@ -73,8 +74,8 @@ describe('getOffset', function () { ); var nodeA = fixture.children[0]; var nodeB = nodeA.children[0]; - assert.equal(getOffset(nodeA, nodeB), 30); - assert.equal(getOffset(nodeB, nodeA), 30); + assert.closeTo(getOffset(nodeA, nodeB), 30, round); + assert.closeTo(getOffset(nodeB, nodeA), 30, round); }); it('returns the shortest side of the element when an element overlaps on a corner', function () { @@ -85,8 +86,8 @@ describe('getOffset', function () { ); var nodeA = fixture.children[0]; var nodeB = nodeA.children[0]; - assert.closeTo(getOffset(nodeA, nodeB), getDiagonal(30), 0.1); - assert.equal(getOffset(nodeB, nodeA), 20); + assert.closeTo(getOffset(nodeA, nodeB), getDiagonal(30), round); + assert.closeTo(getOffset(nodeB, nodeA), 20, round); }); it('returns smallest diagonal if elmA fully covers elmB', function () { @@ -97,7 +98,7 @@ describe('getOffset', function () { ); var nodeA = fixture.children[0]; var nodeB = nodeA.children[0]; - assert.closeTo(getOffset(nodeA, nodeB), getDiagonal(10), 0.1); - assert.equal(getOffset(nodeB, nodeA), 10); + assert.closeTo(getOffset(nodeA, nodeB), getDiagonal(10), round); + assert.closeTo(getOffset(nodeB, nodeA), 10, round); }); }); From 80299b2fa70e3979219ac66ab13c1ceeb4beb0e7 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Wed, 31 Aug 2022 11:45:56 +0200 Subject: [PATCH 03/28] Fix has-visual-overlap test --- test/commons/math/{has-visual-overlap => has-visual-overlap.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/commons/math/{has-visual-overlap => has-visual-overlap.js} (100%) diff --git a/test/commons/math/has-visual-overlap b/test/commons/math/has-visual-overlap.js similarity index 100% rename from test/commons/math/has-visual-overlap rename to test/commons/math/has-visual-overlap.js From 302db497449fde18aaec0b1e28da74b1dc6483ce Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Wed, 31 Aug 2022 12:03:13 +0200 Subject: [PATCH 04/28] Fix failing tests? --- lib/commons/math/get-offset.js | 2 +- test/checks/mobile/target-size.js | 25 +++++++++++++++---------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/lib/commons/math/get-offset.js b/lib/commons/math/get-offset.js index b25b81044f..27c2175d0d 100644 --- a/lib/commons/math/get-offset.js +++ b/lib/commons/math/get-offset.js @@ -116,7 +116,7 @@ function pointInRect({ x, y }, rect) { * @param {Point} ownRectPoint * @param {Rect} ownRect * @param {Rect} adjacentRect - * @returns + * @returns {Point | null} With x and y */ function getCornerInAdjacentRect({ x, y }, ownRect, adjacentRect) { let closestX, closestY; diff --git a/test/checks/mobile/target-size.js b/test/checks/mobile/target-size.js index 9fd4ec10c5..abb63b6628 100644 --- a/test/checks/mobile/target-size.js +++ b/test/checks/mobile/target-size.js @@ -4,22 +4,27 @@ describe('target-size tests', function () { var checkContext = axe.testUtils.MockCheckContext(); var checkSetup = axe.testUtils.checkSetup; var shadowCheckSetup = axe.testUtils.shadowCheckSetup; - var shadowSupport = axe.testUtils.shadowSupport; + var shadowSupport = axe.testUtils.shadowSupport.v1; + var isIE11 = axe.testUtils.isIE11; var check = checks['target-size']; afterEach(function () { checkContext.reset(); }); - it('returns false for targets smaller than minSpacing', function () { - var checkArgs = checkSetup( - '' - ); - assert.isFalse(check.evaluate.apply(checkContext, checkArgs)); - assert.deepEqual(checkContext._data, { width: 20, height: 30 }); - }); + // IE cannot count + (isIE11 ? xit : it)( + 'returns false for targets smaller than minSpacing', + function () { + var checkArgs = checkSetup( + '' + ); + assert.isFalse(check.evaluate.apply(checkContext, checkArgs)); + assert.deepEqual(checkContext._data, { width: 20, height: 30 }); + } + ); it('returns true for unobscured targets larger than minSpacing', function () { var checkArgs = checkSetup( From 0172fbec8336fffd6c961997425c209deed848ba Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Wed, 31 Aug 2022 12:09:57 +0200 Subject: [PATCH 05/28] Rename target-size.json file --- doc/rule-descriptions.md | 10 +++++----- lib/rules/{target-spacing.json => target-size.json} | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) rename lib/rules/{target-spacing.json => target-size.json} (79%) diff --git a/doc/rule-descriptions.md b/doc/rule-descriptions.md index 8689a73aa7..891a202a91 100644 --- a/doc/rule-descriptions.md +++ b/doc/rule-descriptions.md @@ -71,10 +71,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, ACT | 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, ACT | failure | [24afc2](https://act-rules.github.io/rules/24afc2), [9e45ec](https://act-rules.github.io/rules/9e45ec), [78fd32](https://act-rules.github.io/rules/78fd32) | +| 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, ACT | 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, ACT | failure | [24afc2](https://act-rules.github.io/rules/24afc2), [9e45ec](https://act-rules.github.io/rules/9e45ec), [78fd32](https://act-rules.github.io/rules/78fd32) | +| [target-size](https://dequeuniversity.com/rules/axe/4.4/target-size?application=RuleDescription) | Ensure touch target have sufficient size and space | Serious | wcag22aa, sc1412, cat.sensory-and-visual-cues | failure | | ## Best Practices Rules @@ -136,7 +137,6 @@ Rules we are still testing and developing. They are disabled by default in axe-c | [link-in-text-block](https://dequeuniversity.com/rules/axe/4.4/link-in-text-block?application=RuleDescription) | Ensure links are distinguished from surrounding text in a way that does not rely on color | Serious | cat.color, experimental, wcag2a, wcag141 | failure, needs review | | | [p-as-heading](https://dequeuniversity.com/rules/axe/4.4/p-as-heading?application=RuleDescription) | Ensure bold, italic text and font-size is not used to style <p> elements as a heading | Serious | cat.semantics, wcag2a, wcag131, experimental | failure, needs review | | | [table-fake-caption](https://dequeuniversity.com/rules/axe/4.4/table-fake-caption?application=RuleDescription) | Ensure that tables with a caption use the <caption> element. | Serious | cat.tables, experimental, wcag2a, wcag131, section508, section508.22.g | failure | | -| [target-size](https://dequeuniversity.com/rules/axe/4.4/target-size?application=RuleDescription) | Ensure touch target have sufficient size and space | Serious | wcag22aa, sc1412, experimental, cat.sensory-and-visual-cues | failure | | | [td-has-header](https://dequeuniversity.com/rules/axe/4.4/td-has-header?application=RuleDescription) | Ensure that each non-empty data cell in a <table> larger than 3 by 3 has one or more table headers | Critical | cat.tables, experimental, wcag2a, wcag131, section508, section508.22.g | failure | | ## Deprecated Rules diff --git a/lib/rules/target-spacing.json b/lib/rules/target-size.json similarity index 79% rename from lib/rules/target-spacing.json rename to lib/rules/target-size.json index 8c617b3598..2843b9dfed 100644 --- a/lib/rules/target-spacing.json +++ b/lib/rules/target-size.json @@ -1,7 +1,7 @@ { "id": "target-size", "selector": "a[href], button, *[role=link]", - "tags": ["wcag22aa", "sc1412", "experimental", "cat.sensory-and-visual-cues"], + "tags": ["wcag22aa", "sc1412", "cat.sensory-and-visual-cues"], "metadata": { "description": "Ensure touch target have sufficient size and space", "help": "All touch targets must be 24px large, or leave sufficient space" From 5b1c1a3ea382cb6154a4fa116ac433d6b8fdb86b Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Wed, 31 Aug 2022 12:31:17 +0200 Subject: [PATCH 06/28] Disable target-spacing rule on APG --- test/aria-practices/apg.spec.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/aria-practices/apg.spec.js b/test/aria-practices/apg.spec.js index a043f8d377..cc06e4b884 100644 --- a/test/aria-practices/apg.spec.js +++ b/test/aria-practices/apg.spec.js @@ -6,7 +6,7 @@ const { getWebdriver, connectToChromeDriver } = require('./run-server'); const { assert } = require('chai'); const globby = require('globby'); -describe('aria-practices', function() { +describe('aria-practices', function () { // Use path.resolve rather than require.resolve because APG has no package.json const apgPath = path.resolve(__dirname, '../../node_modules/aria-practices/'); const filePaths = globby.sync(`${apgPath}/examples/**/*.html`); @@ -36,6 +36,7 @@ describe('aria-practices', function() { const disabledRules = { '*': [ 'color-contrast', + 'target-size', 'heading-order', // w3c/aria-practices#2119 'list', // w3c/aria-practices#2118 'scrollable-region-focusable' // w3c/aria-practices#2114 From a0eaaf84309e89dfe408c1371fc34647328565b3 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Wed, 31 Aug 2022 17:53:50 +0200 Subject: [PATCH 07/28] Improve target-size messaging --- lib/checks/mobile/target-offset-evaluate.js | 30 ++++---- lib/checks/mobile/target-offset.json | 6 +- lib/checks/mobile/target-size-evaluate.js | 17 +++-- lib/checks/mobile/target-size.json | 8 +- locales/_template.json | 12 +-- test/checks/mobile/target-offset.js | 85 +++++++++++++++++++-- test/checks/mobile/target-size.js | 15 +++- 7 files changed, 134 insertions(+), 39 deletions(-) diff --git a/lib/checks/mobile/target-offset-evaluate.js b/lib/checks/mobile/target-offset-evaluate.js index 91f9c5b149..45c2ba075c 100644 --- a/lib/checks/mobile/target-offset-evaluate.js +++ b/lib/checks/mobile/target-offset-evaluate.js @@ -2,31 +2,33 @@ import { findNearbyElms } from '../../commons/dom'; import { getRole, getRoleType } from '../../commons/aria'; import { getOffset } from '../../commons/math'; -export default function targetOffsetEvaluate( - node, - { minSpacing = 23.95 }, - vNode -) { - let closestOffset = minSpacing; +const roundingMargin = 0.05; + +export default function targetOffsetEvaluate(node, options, vNode) { + const minOffset = options?.minOffset || 24; const closeNeighbors = []; - for (const vNeighbor of findNearbyElms(vNode, minSpacing)) { + let closestOffset = minOffset; + for (const vNeighbor of findNearbyElms(vNode, minOffset)) { const role = getRole(vNeighbor); if (getRoleType(role) !== 'widget') { continue; } - const offset = getOffset(vNode, vNeighbor); - if (offset >= minSpacing) { + const offset = roundToSingleDecimal(getOffset(vNode, vNeighbor)); + if (offset + roundingMargin >= minOffset) { continue; } closestOffset = Math.min(closestOffset, offset); closeNeighbors.push(vNeighbor.actualNode); } - if (closeNeighbors.length === 0) { - return true; + this.data({ closestOffset, minOffset }); + if (closeNeighbors.length > 0) { + this.relatedNodes(closeNeighbors); + return false; } + return true; +} - this.data({ closestOffset }); - this.relatedNodes(closeNeighbors); - return false; +function roundToSingleDecimal(num) { + return Math.round(num * 10) / 10; } diff --git a/lib/checks/mobile/target-offset.json b/lib/checks/mobile/target-offset.json index ff39061520..c57ca5301a 100644 --- a/lib/checks/mobile/target-offset.json +++ b/lib/checks/mobile/target-offset.json @@ -2,13 +2,13 @@ "id": "target-offset", "evaluate": "target-offset-evaluate", "options": { - "minSpacing": 23.95 + "minOffset": 24 }, "metadata": { "impact": "serious", "messages": { - "pass": "Target is sufficiently spaced from its neighbours", - "fail": "Target is insufficiently spaced from its neighbours" + "pass": "Target is sufficiently offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)", + "fail": "Target is insufficient offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)" } } } diff --git a/lib/checks/mobile/target-size-evaluate.js b/lib/checks/mobile/target-size-evaluate.js index ba5c7d1214..93f0681dcd 100644 --- a/lib/checks/mobile/target-size-evaluate.js +++ b/lib/checks/mobile/target-size-evaluate.js @@ -2,18 +2,24 @@ import { findNearbyElms } from '../../commons/dom'; import { getRole, getRoleType } from '../../commons/aria'; import { splitRects, hasVisualOverlap } from '../../commons/math'; +const roundingMargin = 0.05; + /** * Determine if an element has a minimum size, taking into account * any elements that may obscure it. */ -export default function targetSize(node, { minSpacing = 23.95 }, vNode) { +export default function targetSize(node, options, vNode) { + const minSpacing = options?.minSpacing || 24; const hasMinimumSize = ({ width, height }) => { - return width >= minSpacing && height >= minSpacing; + return ( + width + roundingMargin >= minSpacing && + height + roundingMargin >= minSpacing + ); }; const nodeRect = vNode.boundingClientRect; if (!hasMinimumSize(nodeRect)) { - this.data(toDecimalSize(nodeRect)); + this.data({ minSpacing, ...toDecimalSize(nodeRect) }); return false; } @@ -25,7 +31,7 @@ export default function targetSize(node, { minSpacing = 23.95 }, vNode) { }); if (obscuringElms.length === 0) { - this.data(toDecimalSize(nodeRect)); + this.data({ minSpacing, ...toDecimalSize(nodeRect) }); return true; // No obscuring elements; pass } @@ -51,6 +57,7 @@ export default function targetSize(node, { minSpacing = 23.95 }, vNode) { if (!hasMinimumSize(largestInnerRect)) { this.data({ messageKey: 'obscured', + minSpacing, ...toDecimalSize(largestInnerRect) }); this.relatedNodes(obscuringElms.map(({ actualNode }) => actualNode)); @@ -58,7 +65,7 @@ export default function targetSize(node, { minSpacing = 23.95 }, vNode) { return false; } - this.data(toDecimalSize(largestInnerRect)); + this.data({ minSpacing, ...toDecimalSize(largestInnerRect) }); return true; } diff --git a/lib/checks/mobile/target-size.json b/lib/checks/mobile/target-size.json index a5cbf27546..6c8df407bc 100644 --- a/lib/checks/mobile/target-size.json +++ b/lib/checks/mobile/target-size.json @@ -2,15 +2,15 @@ "id": "target-size", "evaluate": "target-size-evaluate", "options": { - "minSpacing": 23.95 + "minSpacing": 24 }, "metadata": { "impact": "serious", "messages": { - "pass": "Control has sufficient size (${data.width}px width, ${data.height}px height)", + "pass": "Control has sufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSpacing}px by ${data.minSpacing}px)", "fail": { - "default": "Element has insufficient size (${data.width}px width, ${data.height}px height)", - "obscured": "Element has insufficient size because it is obscured (smallest space is ${data.width}px width by ${data.height}px height)" + "default": "Element has insufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSpacing}px by ${data.minSpacing}px)", + "obscured": "Element has insufficient size because it is obscured (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSpacing}px by ${data.minSpacing}px)" } } } diff --git a/locales/_template.json b/locales/_template.json index 994d767980..012183081a 100644 --- a/locales/_template.json +++ b/locales/_template.json @@ -816,14 +816,14 @@ "fail": "${data} on tag disables zooming on mobile devices" }, "target-offset": { - "pass": "Target is sufficiently spaced from its neighbours", - "fail": "Target is insufficiently spaced from its neighbours" + "pass": "Target is sufficiently offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)", + "fail": "Target is insufficient offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)" }, "target-size": { - "pass": "Control has sufficient size (${data.width}px width, ${data.height}px height)", + "pass": "Control has sufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSpacing}px by ${data.minSpacing}px)", "fail": { - "default": "Element has insufficient size (${data.width}px width, ${data.height}px height)", - "obscured": "Element has insufficient size because it is obscured (smallest space is ${data.width}px width by ${data.height}px height)" + "default": "Element has insufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSpacing}px by ${data.minSpacing}px)", + "obscured": "Element has insufficient size because it is obscured (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSpacing}px by ${data.minSpacing}px)" } }, "header-present": { @@ -1045,4 +1045,4 @@ } }, "incompleteFallbackMessage": "axe couldn't tell the reason. Time to break out the element inspector!" -} \ No newline at end of file +} diff --git a/test/checks/mobile/target-offset.js b/test/checks/mobile/target-offset.js index 1626636ce0..dde72166ab 100644 --- a/test/checks/mobile/target-offset.js +++ b/test/checks/mobile/target-offset.js @@ -2,14 +2,89 @@ describe('target-offset tests', function () { 'use strict'; var checkContext = axe.testUtils.MockCheckContext(); - // var checkSetup = axe.testUtils.checkSetup; - // var shadowCheckSetup = axe.testUtils.shadowCheckSetup; - // var shadowSupport = axe.testUtils.shadowSupport; - // var check = checks['target-offset']; + var checkSetup = axe.testUtils.checkSetup; + var check = checks['target-offset']; afterEach(function () { checkContext.reset(); }); - it('returns false for targets smaller than minSpacing'); + it('returns true when there are no other nearby targets', function () { + var checkArgs = checkSetup( + '' + ); + + assert.isTrue(check.evaluate.apply(checkContext, checkArgs)); + assert.equal(checkContext._data.minOffset, 24); + assert.closeTo(checkContext._data.closestOffset, 24, 0.2); + }); + + it('returns true when the offset is 24px', function () { + var checkArgs = checkSetup( + '' + + '' + ); + + assert.isTrue(check.evaluate.apply(checkContext, checkArgs)); + assert.equal(checkContext._data.minOffset, 24); + assert.closeTo(checkContext._data.closestOffset, 24, 0.2); + }); + + it('returns false when the offset is 23px', function () { + var checkArgs = checkSetup( + '' + + '' + ); + + assert.isFalse(check.evaluate.apply(checkContext, checkArgs)); + assert.equal(checkContext._data.minOffset, 24); + assert.closeTo(checkContext._data.closestOffset, 23, 0.2); + }); + + it('ignores non-widget elements as neighbors', function () { + var checkArgs = checkSetup( + '' + + '
x
' + ); + + assert.isTrue(check.evaluate.apply(checkContext, checkArgs)); + assert.equal(checkContext._data.minOffset, 24); + assert.closeTo(checkContext._data.closestOffset, 24, 0.2); + }); + + it('sets all elements that are too close as related nodes', function () { + var checkArgs = checkSetup( + '' + + '' + + '' + ); + assert.isFalse(check.evaluate.apply(checkContext, checkArgs)); + assert.equal(checkContext._data.minOffset, 24); + assert.closeTo(checkContext._data.closestOffset, 16, 0.2); + + var relatedIds = checkContext._relatedNodes.map(function (node) { + return '#' + node.id; + }); + assert.deepEqual(relatedIds, ['#left', '#right']); + }); }); diff --git a/test/checks/mobile/target-size.js b/test/checks/mobile/target-size.js index abb63b6628..af8586e47e 100644 --- a/test/checks/mobile/target-size.js +++ b/test/checks/mobile/target-size.js @@ -22,7 +22,11 @@ describe('target-size tests', function () { '">x' ); assert.isFalse(check.evaluate.apply(checkContext, checkArgs)); - assert.deepEqual(checkContext._data, { width: 20, height: 30 }); + assert.deepEqual(checkContext._data, { + minSpacing: 24, + width: 20, + height: 30 + }); } ); @@ -34,6 +38,7 @@ describe('target-size tests', function () { ); assert.isTrue(check.evaluate.apply(checkContext, checkArgs)); assert.deepEqual(checkContext._data, { + minSpacing: 24, width: 40, height: 30 }); @@ -49,7 +54,11 @@ describe('target-size tests', function () { '">x' ); assert.isTrue(check.evaluate.apply(checkContext, checkArgs)); - assert.deepEqual(checkContext._data, { width: 30, height: 30 }); + assert.deepEqual(checkContext._data, { + minSpacing: 24, + width: 30, + height: 30 + }); }); it('returns false for obscured targets with insufficient space', function () { @@ -67,6 +76,7 @@ describe('target-size tests', function () { assert.isFalse(check.evaluate.apply(checkContext, checkArgs)); assert.deepEqual(checkContext._data, { messageKey: 'obscured', + minSpacing: 24, width: 20, height: 30 }); @@ -88,6 +98,7 @@ describe('target-size tests', function () { assert.isFalse(check.evaluate.apply(checkContext, checkArgs)); assert.deepEqual(checkContext._data, { messageKey: 'obscured', + minSpacing: 24, width: 20, height: 30 }); From c4029a544bb77e6d19e3517616f76fbcd545bf04 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Fri, 2 Sep 2022 15:04:41 +0200 Subject: [PATCH 08/28] Update is-in-text-block --- lib/commons/dom/is-in-text-block.js | 39 ++++++++----- test/commons/dom/is-in-text-block.js | 84 +++++++++++++++++++++------- 2 files changed, 89 insertions(+), 34 deletions(-) diff --git a/lib/commons/dom/is-in-text-block.js b/lib/commons/dom/is-in-text-block.js index 0e1dc2202c..52f3635e5a 100644 --- a/lib/commons/dom/is-in-text-block.js +++ b/lib/commons/dom/is-in-text-block.js @@ -33,10 +33,15 @@ function getBlockParent(node) { /** * Determines if an element is within a text block + * With `noLengthCompare` true, will return if there is any non-space text outside + * widgets. When false, compares the length of non-widget text to widget text + * * @param {Element} node [description] + * @param {Object} options Optional + * @property {Bool} noLengthCompare * @return {Boolean} [description] */ -function isInTextBlock(node) { +function isInTextBlock(node, options) { if (isBlock(node)) { // Ignore if the link is a block return false; @@ -45,7 +50,7 @@ function isInTextBlock(node) { // Find all the text part of the parent block not in a link, and all the text in a link const virtualParent = getBlockParent(node); let parentText = ''; - let linkText = ''; + let widgetText = ''; let inBrBlock = 0; // We want to ignore hidden text, and if br / hr is used, only use the section of the parent @@ -66,11 +71,14 @@ function isInTextBlock(node) { } var nodeName = (currNode.nodeName || '').toUpperCase(); + if (currNode === node) { + inBrBlock = 1; + } // BR and HR elements break the line if (['BR', 'HR'].includes(nodeName)) { if (inBrBlock === 0) { parentText = ''; - linkText = ''; + widgetText = ''; } else { inBrBlock = 2; } @@ -84,24 +92,27 @@ function isInTextBlock(node) { ) { return false; - // Don't walk links, we're only interested in what's not in them. - } else if ( - (nodeName === 'A' && currNode.href) || - (currNode.getAttribute('role') || '').toLowerCase() === 'link' - ) { - if (currNode === node) { - inBrBlock = 1; - } + // Don't walk widgets, we're only interested in what's not in them. + } else if (isWidget(currNode)) { // Grab all the text from this element, but don't walk down it's children - linkText += currNode.textContent; + widgetText += currNode.textContent; return false; } }); parentText = sanitize(parentText); - linkText = sanitize(linkText); + if (options?.noLengthCompare) { + return parentText.length !== 0; + } + + widgetText = sanitize(widgetText); + return parentText.length > widgetText.length; +} - return parentText.length > linkText.length; +function isWidget(domNode) { + const { getRoleType, getRole } = axe.commons.aria; + const role = getRole(domNode); + return ['widget', 'composite'].includes(getRoleType(role)); } export default isInTextBlock; diff --git a/test/commons/dom/is-in-text-block.js b/test/commons/dom/is-in-text-block.js index 591dd1b6b9..9366693710 100644 --- a/test/commons/dom/is-in-text-block.js +++ b/test/commons/dom/is-in-text-block.js @@ -1,15 +1,15 @@ -describe('dom.isInTextBlock', function() { +describe('dom.isInTextBlock', function () { 'use strict'; var fixture = document.getElementById('fixture'); var shadowSupport = axe.testUtils.shadowSupport; var fixtureSetup = axe.testUtils.fixtureSetup; - afterEach(function() { + afterEach(function () { fixture.innerHTML = ''; }); - it('returns true if the element is a node in a block of text', function() { + it('returns true if the element is a node in a block of text', function () { fixtureSetup( '

Some paragraph with text ' + ' link' + @@ -19,7 +19,7 @@ describe('dom.isInTextBlock', function() { assert.isTrue(axe.commons.dom.isInTextBlock(link)); }); - it('returns false if the element is a block', function() { + it('returns false if the element is a block', function () { fixtureSetup( '

Some paragraph with text ' + ' link' + @@ -29,13 +29,13 @@ describe('dom.isInTextBlock', function() { assert.isFalse(axe.commons.dom.isInTextBlock(link)); }); - it('returns false if the element has the only text in the block', function() { - fixtureSetup('

' + ' link' + '

'); + it('returns false if the element has the only text in the block', function () { + fixtureSetup('

link

'); var link = document.getElementById('link'); assert.isFalse(axe.commons.dom.isInTextBlock(link)); }); - it('returns false if there is more text in link(s) than in the rest of the block', function() { + it('returns false if there is more text in link(s) than in the rest of the block', function () { fixtureSetup( '

short text:' + ' on a link with a very long text' + @@ -45,7 +45,7 @@ describe('dom.isInTextBlock', function() { assert.isFalse(axe.commons.dom.isInTextBlock(link)); }); - it('return false if there are links along side other links', function() { + it('return false if there are links along side other links', function () { fixtureSetup( '

' + ' link' + @@ -56,7 +56,7 @@ describe('dom.isInTextBlock', function() { assert.isFalse(axe.commons.dom.isInTextBlock(link)); }); - it('ignores hidden content', function() { + it('ignores hidden content', function () { fixtureSetup( '

' + ' link' + @@ -67,7 +67,7 @@ describe('dom.isInTextBlock', function() { assert.isFalse(axe.commons.dom.isInTextBlock(link)); }); - it('ignores floated content', function() { + it('ignores floated content', function () { fixtureSetup( '

' + ' A floating text in the area' + @@ -78,7 +78,7 @@ describe('dom.isInTextBlock', function() { assert.isFalse(axe.commons.dom.isInTextBlock(link)); }); - it('ignores positioned content', function() { + it('ignores positioned content', function () { fixtureSetup( '

' + ' Some absolute potitioned text' + @@ -89,7 +89,7 @@ describe('dom.isInTextBlock', function() { assert.isFalse(axe.commons.dom.isInTextBlock(link)); }); - it('ignores none-text content', function() { + it('ignores none-text content', function () { fixtureSetup( '

' + ' Some graphical component' + @@ -100,7 +100,7 @@ describe('dom.isInTextBlock', function() { assert.isFalse(axe.commons.dom.isInTextBlock(link)); }); - it('ignore text in the block coming before a br', function() { + it('ignore text in the block coming before a br', function () { fixtureSetup( '

Some paragraph with text
' + ' link' + @@ -110,7 +110,7 @@ describe('dom.isInTextBlock', function() { assert.isFalse(axe.commons.dom.isInTextBlock(link)); }); - it('ignore text in the block coming after a br', function() { + it('ignore text in the block coming after a br', function () { fixtureSetup( '

' + ' link
' + @@ -121,7 +121,7 @@ describe('dom.isInTextBlock', function() { assert.isFalse(axe.commons.dom.isInTextBlock(link)); }); - it('ignore text in the block coming before and after a br', function() { + it('ignore text in the block coming before and after a br', function () { fixtureSetup( '

Some paragraph with text
' + ' link
' + @@ -132,7 +132,22 @@ describe('dom.isInTextBlock', function() { assert.isFalse(axe.commons.dom.isInTextBlock(link)); }); - it('treats hr elements the same as br elements', function() { + it('ignores text inside inline widgets and components', function () { + fixtureSetup( + '

' + + ' link' + + ' ' + + ' ' + + ' My query' + + ' ' + + ' ' + + '

' + ); + var link = document.getElementById('link'); + assert.isFalse(axe.commons.dom.isInTextBlock(link)); + }); + + it('treats hr elements the same as br elements', function () { fixtureSetup( '
Some paragraph with text
' + ' link
' + @@ -143,7 +158,7 @@ describe('dom.isInTextBlock', function() { assert.isFalse(axe.commons.dom.isInTextBlock(link)); }); - it('ignore comments', function() { + it('ignore comments', function () { fixtureSetup( '

' + ' link' + @@ -153,7 +168,7 @@ describe('dom.isInTextBlock', function() { assert.isFalse(axe.commons.dom.isInTextBlock(link)); }); - (shadowSupport.v1 ? it : xit)('can reach outside a shadow tree', function() { + (shadowSupport.v1 ? it : xit)('can reach outside a shadow tree', function () { var div = document.createElement('div'); div.innerHTML = 'Some paragraph with text '; var shadow = div.querySelector('span').attachShadow({ mode: 'open' }); @@ -164,7 +179,7 @@ describe('dom.isInTextBlock', function() { assert.isTrue(axe.commons.dom.isInTextBlock(link)); }); - (shadowSupport.v1 ? it : xit)('can reach into a shadow tree', function() { + (shadowSupport.v1 ? it : xit)('can reach into a shadow tree', function () { var div = document.createElement('div'); div.innerHTML = 'link'; var shadow = div.attachShadow({ mode: 'open' }); @@ -177,7 +192,7 @@ describe('dom.isInTextBlock', function() { (shadowSupport.v1 ? it : xit)( 'treats shadow DOM slots as siblings', - function() { + function () { var div = document.createElement('div'); div.innerHTML = '
'; var shadow = div.attachShadow({ mode: 'open' }); @@ -190,4 +205,33 @@ describe('dom.isInTextBlock', function() { assert.isFalse(axe.commons.dom.isInTextBlock(link)); } ); + + describe('options.noLengthCompare', function () { + it('returns true if there is any text outside the link', function () { + fixtureSetup('

amy link text is longer

'); + var link = document.getElementById('link'); + assert.isTrue( + axe.commons.dom.isInTextBlock(link, { + noLengthCompare: true + }) + ); + }); + + it('returns false if the non-widget text is only whitespace', function () { + fixtureSetup( + '

' + + ' link 1\t\n\r' + + ' link 2' + + ' link 3' + + ' link 4' + + '

' + ); + var link = document.getElementById('link'); + assert.isFalse( + axe.commons.dom.isInTextBlock(link, { + noLengthCompare: true + }) + ); + }); + }); }); From 55b037ccbdfd1bd968bd1ec90fecd39ec30621db Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Fri, 2 Sep 2022 15:05:49 +0200 Subject: [PATCH 09/28] Add target-size matches method --- doc/rule-descriptions.md | 10 +- lib/rules/target-size.json | 5 +- lib/rules/widget-not-inline-matches.js | 32 ++++++ .../rule-matches/widget-not-inline-matches.js | 100 ++++++++++++++++++ 4 files changed, 140 insertions(+), 7 deletions(-) create mode 100644 lib/rules/widget-not-inline-matches.js create mode 100644 test/rule-matches/widget-not-inline-matches.js diff --git a/doc/rule-descriptions.md b/doc/rule-descriptions.md index 891a202a91..b30aa08dbd 100644 --- a/doc/rule-descriptions.md +++ b/doc/rule-descriptions.md @@ -71,11 +71,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, ACT | 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, ACT | failure | [24afc2](https://act-rules.github.io/rules/24afc2), [9e45ec](https://act-rules.github.io/rules/9e45ec), [78fd32](https://act-rules.github.io/rules/78fd32) | -| [target-size](https://dequeuniversity.com/rules/axe/4.4/target-size?application=RuleDescription) | Ensure touch target have sufficient size and space | Serious | wcag22aa, sc1412, cat.sensory-and-visual-cues | 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, ACT | 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, ACT | failure | [24afc2](https://act-rules.github.io/rules/24afc2), [9e45ec](https://act-rules.github.io/rules/9e45ec), [78fd32](https://act-rules.github.io/rules/78fd32) | +| [target-size](https://dequeuniversity.com/rules/axe/4.4/target-size?application=RuleDescription) | Ensure touch target have sufficient size and space | Serious | wcag22aa, sc258, cat.sensory-and-visual-cues | failure | | ## Best Practices Rules diff --git a/lib/rules/target-size.json b/lib/rules/target-size.json index 2843b9dfed..add4dbe6dd 100644 --- a/lib/rules/target-size.json +++ b/lib/rules/target-size.json @@ -1,7 +1,8 @@ { "id": "target-size", - "selector": "a[href], button, *[role=link]", - "tags": ["wcag22aa", "sc1412", "cat.sensory-and-visual-cues"], + "selector": "*", + "matches": "widget-not-inline-matches", + "tags": ["wcag22aa", "sc258", "cat.sensory-and-visual-cues"], "metadata": { "description": "Ensure touch target have sufficient size and space", "help": "All touch targets must be 24px large, or leave sufficient space" diff --git a/lib/rules/widget-not-inline-matches.js b/lib/rules/widget-not-inline-matches.js new file mode 100644 index 0000000000..79312ba1b8 --- /dev/null +++ b/lib/rules/widget-not-inline-matches.js @@ -0,0 +1,32 @@ +import { getRole, getRoleType } from '../commons/aria'; +import { isFocusable, isInTextBlock } from '../commons/dom'; +import svgNamespaceMatches from './svg-namespace-matches'; + +export default function widgetNotInline(node, vNode) { + return matchesFns.every(fn => fn(node, vNode)); +} + +const matchesFns = [ + (node, vNode) => isWidgetType(vNode), + (node, vNode) => isNotAreaElement(vNode), + (node, vNode) => !svgNamespaceMatches(node, vNode), + (node, vNode) => isFocusable(vNode), + node => !isInTextBlock(node, { noLengthCompare: true }) +]; + +function isWidgetType(vNode) { + const role = getRole(vNode); + // TODO: These needs tests + if ('option' === role) { + return false; + } else if (['combobox', 'listbox'].includes(role)) { + return true; + } + + const roleType = getRoleType(role); + return roleType === 'widget'; +} + +function isNotAreaElement(vNode) { + return vNode.props.nodeName !== 'area'; +} diff --git a/test/rule-matches/widget-not-inline-matches.js b/test/rule-matches/widget-not-inline-matches.js new file mode 100644 index 0000000000..b2adb4c466 --- /dev/null +++ b/test/rule-matches/widget-not-inline-matches.js @@ -0,0 +1,100 @@ +describe('widget-not-inline-matches', function () { + 'use strict'; + var rule; + var queryFixture = axe.testUtils.queryFixture; + + beforeEach(function () { + rule = axe.utils.getRule('target-size'); + }); + + it('returns true for native widgets', function () { + var vNode = queryFixture(''); + var node = vNode.actualNode; + assert.isTrue(rule.matches(node, vNode)); + }); + + it('returns true for non-native widgets', function () { + var vNode = queryFixture( + '
' + ); + var node = vNode.actualNode; + assert.isTrue(rule.matches(node, vNode)); + }); + + it('returns false for non-widget elements', function () { + var vNode = queryFixture(''); + var node = vNode.actualNode; + assert.isFalse(rule.matches(node, vNode)); + }); + + it('returns false for non-focusable native widgets', function () { + var vNode = queryFixture(''); + var node = vNode.actualNode; + assert.isFalse(rule.matches(node, vNode)); + }); + + it('returns false for non-focusable custom widgets', function () { + var vNode = queryFixture('
'); + var node = vNode.actualNode; + assert.isFalse(rule.matches(node, vNode)); + }); + + describe('inline components', function () { + it('returns false for elements inline with text', function () { + var vNode = queryFixture( + '

Some ' + ' link' + '

' + ); + var node = vNode.actualNode; + assert.isFalse(rule.matches(node, vNode)); + }); + + it('returns false for multiple inline links', function () { + var vNode = queryFixture( + '

' + + ' link 1, ' + + ' link 2, ' + + ' link 3' + + '

' + ); + var node = vNode.actualNode; + assert.isFalse(rule.matches(node, vNode)); + }); + + it('returns true if the widget is the only element in its block parent', function () { + var vNode = queryFixture('

link

'); + var node = vNode.actualNode; + assert.isTrue(rule.matches(node, vNode)); + }); + }); + + describe('graphics (for which size may be essential)', function () { + it('returns false for area elements', function () { + var vNode = queryFixture( + 'img' + + '' + + ' map' + + '' + ); + var node = vNode.actualNode; + assert.isFalse(rule.matches(node, vNode)); + }); + + it('returns false SVG elements', function () { + var vNode = queryFixture( + '' + ); + var node = vNode.actualNode; + assert.isFalse(rule.matches(node, vNode)); + }); + + it('returns false descendants of SVG elements', function () { + var vNode = queryFixture( + '' + + ' link' + + '' + ); + var node = vNode.actualNode; + assert.isFalse(rule.matches(node, vNode)); + }); + }); +}); From 81feac624b00ab886b8e5ef9008888eb1fb446bd Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Fri, 2 Sep 2022 15:06:48 +0200 Subject: [PATCH 10/28] Add integration tests for target-size --- .../full/target-size/target-size.html | 2 +- .../rules/target-size/target-size.html | 68 +++++++++++++++++++ .../rules/target-size/target-size.json | 20 ++++++ 3 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 test/integration/rules/target-size/target-size.html create mode 100644 test/integration/rules/target-size/target-size.json diff --git a/test/integration/full/target-size/target-size.html b/test/integration/full/target-size/target-size.html index ed45aaf798..87c413b247 100644 --- a/test/integration/full/target-size/target-size.html +++ b/test/integration/full/target-size/target-size.html @@ -1,7 +1,7 @@ - Region Test + Target-size Test + /* Ensure sufficient space between tests */ + p { + margin: 30px; + } + + +

+ + + +

+ +

+ + + +

+ + +

+ x +

+ + + +

+ +

+ + +

+ The quick brown
+ fox jumped over the lazy dog. +

+ + + + Hello + + + + + + + diff --git a/test/integration/rules/target-size/target-size.json b/test/integration/rules/target-size/target-size.json new file mode 100644 index 0000000000..24cffdd9f7 --- /dev/null +++ b/test/integration/rules/target-size/target-size.json @@ -0,0 +1,20 @@ +{ + "description": "target-size test", + "rule": "target-size", + "violations": [ + ["#fail1"], + ["#fail2"], + ["#fail3"], + ["#fail4"], + ["#fail5"], + ["#fail6"] + ], + "passes": [ + ["#pass1"], + ["#pass2"], + ["#pass3"], + ["#pass4"], + ["#pass5"], + ["#pass-adjacent"] + ] +} From eb270498dbc51df0653054fddcf9dbd88c672a4f Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Fri, 2 Sep 2022 15:07:05 +0200 Subject: [PATCH 11/28] Document target-size check options --- doc/check-options.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/doc/check-options.md b/doc/check-options.md index 90a30b47d4..0f5ffb41ab 100644 --- a/doc/check-options.md +++ b/doc/check-options.md @@ -31,6 +31,8 @@ - [p-as-heading](#p-as-heading) - [avoid-inline-spacing](#avoid-inline-spacing) - [scope-value](#scope-value) + - [target-offset](#target-offset) + - [target-size](#target-size) - [region](#region) - [inline-style-property](#inline-style-property) @@ -487,6 +489,18 @@ h6:not([role]), | -------- | :-------------------------------------------------------- | :------------------------- | | `values` | 
['row', 'col', 'rowgroup', 'colgroup']
| List of valid scope values | +### target-offset + +| Option | Default | Description | +| ----------- | :------ | :--------------------------------------------------------------------------------------------------------- | +| `minOffset` | `24` | Minimum space required from the farthest edge of the target, to the closest edge of the neighboring target | + +### target-size + +| Option | Default | Description | +| --------- | :------ | :------------------------------------------------------------------------------------------------------- | +| `minSize` | `24` | Minimum width and height a component should have, that is not obscured by some other interactive element | + ### region | Option | Default | Description | From 217ac6466356491a875cf06b024cc735e3f7d009 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Fri, 2 Sep 2022 15:24:18 +0200 Subject: [PATCH 12/28] put create-grid into its own file --- lib/commons/dom/create-grid.js | 356 +++++++++++++++++++++ lib/commons/dom/find-nearby-elms.js | 2 +- lib/commons/dom/get-element-stack.js | 3 +- lib/commons/dom/get-rect-stack.js | 361 +--------------------- lib/commons/dom/get-text-element-stack.js | 3 +- lib/commons/dom/index.js | 2 +- lib/commons/dom/visually-sort.js | 2 + test/commons/dom/visually-sort.js | 7 - 8 files changed, 366 insertions(+), 370 deletions(-) create mode 100644 lib/commons/dom/create-grid.js diff --git a/lib/commons/dom/create-grid.js b/lib/commons/dom/create-grid.js new file mode 100644 index 0000000000..0d9736d6b5 --- /dev/null +++ b/lib/commons/dom/create-grid.js @@ -0,0 +1,356 @@ +/* eslint no-bitwise: 0 */ +import isVisible from './is-visible'; +import VirtualNode from '../../core/base/virtual-node/virtual-node'; +import { getNodeFromTree, getScroll, isShadowRoot } from '../../core/utils'; +import constants from '../../core/constants'; +import cache from '../../core/base/cache'; + +/** + * Setup the 2d grid and add every element to it, even elements not + * included in the flat tree + * @returns gridSize + */ +export default function createGrid( + root = document.body, + rootGrid = { + container: null, + cells: [] + }, + parentVNode = null +) { + // Prevent multiple calls per run + if (cache.get('gridCreated') && !parentVNode) { + return constants.gridSize; + } + cache.set('gridCreated', true); + + // by not starting at the htmlElement we don't have to pass a custom + // filter function into the treeWalker to filter out head elements, + // which would be called for every node + if (!parentVNode) { + let vNode = getNodeFromTree(document.documentElement); + if (!vNode) { + vNode = new VirtualNode(document.documentElement); + } + + vNode._stackingOrder = [0]; + addNodeToGrid(rootGrid, vNode); + + if (getScroll(vNode.actualNode)) { + const subGrid = { + container: vNode, + cells: [] + }; + vNode._subGrid = subGrid; + } + } + + // IE11 requires the first 3 parameters + // @see https://developer.mozilla.org/en-US/docs/Web/API/Document/createTreeWalker + const treeWalker = document.createTreeWalker( + root, + window.NodeFilter.SHOW_ELEMENT, + null, + false + ); + let node = parentVNode ? treeWalker.nextNode() : treeWalker.currentNode; + while (node) { + let vNode = getNodeFromTree(node); + + // an svg in IE11 does not have a parentElement but instead has a + // parentNode. but parentNode could be a shadow root so we need to + // verify it's in the tree first + if (node.parentElement) { + parentVNode = getNodeFromTree(node.parentElement); + } else if (node.parentNode && getNodeFromTree(node.parentNode)) { + parentVNode = getNodeFromTree(node.parentNode); + } + + if (!vNode) { + vNode = new axe.VirtualNode(node, parentVNode); + } + + vNode._stackingOrder = getStackingOrder(vNode, parentVNode); + + const scrollRegionParent = findScrollRegionParent(vNode, parentVNode); + const grid = scrollRegionParent ? scrollRegionParent._subGrid : rootGrid; + + if (getScroll(vNode.actualNode)) { + const subGrid = { + container: vNode, + cells: [] + }; + vNode._subGrid = subGrid; + } + + // filter out any elements with 0 width or height + // (we don't do this before so we can calculate stacking context + // of parents with 0 width/height) + const rect = vNode.boundingClientRect; + if (rect.width !== 0 && rect.height !== 0 && isVisible(node)) { + addNodeToGrid(grid, vNode); + } + + // add shadow root elements to the grid + if (isShadowRoot(node)) { + createGrid(node.shadowRoot, grid, vNode); + } + + node = treeWalker.nextNode(); + } + return constants.gridSize; +} + +/** + * Determine if node produces a stacking context. + * References: + * https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Positioning/Understanding_z_index/The_stacking_context + * https://github.com/gwwar/z-context/blob/master/devtools/index.js + * @param {VirtualNode} vNode + * @return {Boolean} + */ +function isStackingContext(vNode, parentVNode) { + const position = vNode.getComputedStylePropertyValue('position'); + const zIndex = vNode.getComputedStylePropertyValue('z-index'); + + // the root element (HTML) is skipped since we always start with a + // stacking order of [0] + + // position: fixed or sticky + if (position === 'fixed' || position === 'sticky') { + return true; + } + + // positioned (absolutely or relatively) with a z-index value other than "auto", + if (zIndex !== 'auto' && position !== 'static') { + return true; + } + + // elements with an opacity value less than 1. + if (vNode.getComputedStylePropertyValue('opacity') !== '1') { + return true; + } + + // elements with a transform value other than "none" + const transform = + vNode.getComputedStylePropertyValue('-webkit-transform') || + vNode.getComputedStylePropertyValue('-ms-transform') || + vNode.getComputedStylePropertyValue('transform') || + 'none'; + + if (transform !== 'none') { + return true; + } + + // elements with a mix-blend-mode value other than "normal" + const mixBlendMode = vNode.getComputedStylePropertyValue('mix-blend-mode'); + if (mixBlendMode && mixBlendMode !== 'normal') { + return true; + } + + // elements with a filter value other than "none" + const filter = vNode.getComputedStylePropertyValue('filter'); + if (filter && filter !== 'none') { + return true; + } + + // elements with a perspective value other than "none" + const perspective = vNode.getComputedStylePropertyValue('perspective'); + if (perspective && perspective !== 'none') { + return true; + } + + // element with a clip-path value other than "none" + const clipPath = vNode.getComputedStylePropertyValue('clip-path'); + if (clipPath && clipPath !== 'none') { + return true; + } + + // element with a mask value other than "none" + const mask = + vNode.getComputedStylePropertyValue('-webkit-mask') || + vNode.getComputedStylePropertyValue('mask') || + 'none'; + if (mask !== 'none') { + return true; + } + + // element with a mask-image value other than "none" + const maskImage = + vNode.getComputedStylePropertyValue('-webkit-mask-image') || + vNode.getComputedStylePropertyValue('mask-image') || + 'none'; + if (maskImage !== 'none') { + return true; + } + + // element with a mask-border value other than "none" + const maskBorder = + vNode.getComputedStylePropertyValue('-webkit-mask-border') || + vNode.getComputedStylePropertyValue('mask-border') || + 'none'; + if (maskBorder !== 'none') { + return true; + } + + // elements with isolation set to "isolate" + if (vNode.getComputedStylePropertyValue('isolation') === 'isolate') { + return true; + } + + // transform or opacity in will-change even if you don't specify values for these attributes directly + const willChange = vNode.getComputedStylePropertyValue('will-change'); + if (willChange === 'transform' || willChange === 'opacity') { + return true; + } + + // elements with -webkit-overflow-scrolling set to "touch" + if ( + vNode.getComputedStylePropertyValue('-webkit-overflow-scrolling') === + 'touch' + ) { + return true; + } + + // element with a contain value of "layout" or "paint" or a composite value + // that includes either of them (i.e. contain: strict, contain: content). + const contain = vNode.getComputedStylePropertyValue('contain'); + if (['layout', 'paint', 'strict', 'content'].includes(contain)) { + return true; + } + + // a flex item or gird item with a z-index value other than "auto", that is the parent element display: flex|inline-flex|grid|inline-grid, + if (zIndex !== 'auto' && parentVNode) { + const parentDsiplay = parentVNode.getComputedStylePropertyValue('display'); + if ( + [ + 'flex', + 'inline-flex', + 'inline flex', + 'grid', + 'inline-grid', + 'inline grid' + ].includes(parentDsiplay) + ) { + return true; + } + } + + return false; +} + +/** + * Determine the stacking order of an element. The stacking order is an array of + * zIndex values for each stacking context parent. + * @param {VirtualNode} + * @return {Number[]} + */ +function getStackingOrder(vNode, parentVNode) { + const stackingOrder = parentVNode._stackingOrder.slice(); + const zIndex = vNode.getComputedStylePropertyValue('z-index'); + const positioned = + vNode.getComputedStylePropertyValue('position') !== 'static'; + const floated = vNode.getComputedStylePropertyValue('float') !== 'none'; + + if (positioned && !['auto', '0'].includes(zIndex)) { + // if a positioned element has a z-index > 0, find the first + // true stack (not a "fake" stack created from positioned or + // floated elements without a z-index) and create a new stack at + // that point (step #5 and step #8) + // @see https://www.w3.org/Style/css2-updates/css2/zindex.html + while (stackingOrder.find(value => value % 1 !== 0)) { + const index = stackingOrder.findIndex(value => value % 1 !== 0); + stackingOrder.splice(index, 1); + } + stackingOrder[stackingOrder.length - 1] = parseInt(zIndex); + } + if (isStackingContext(vNode, parentVNode)) { + stackingOrder.push(0); + } + // if a positioned element has z-index: auto or 0 (step #8), or if + // a non-positioned floating element (step #5), treat it as its + // own stacking context + // @see https://www.w3.org/Style/css2-updates/css2/zindex.html + else if (positioned) { + // Put positioned elements above floated elements + stackingOrder.push(0.5); + } else if (floated) { + // Put floated elements above z-index: 0 + // (step #5 floating get sorted below step #8 positioned) + stackingOrder.push(0.25); + } + + return stackingOrder; +} + +/** + * Return the parent node that is a scroll region. + * @param {VirtualNode} + * @return {VirtualNode|null} + */ +function findScrollRegionParent(vNode, parentVNode) { + let scrollRegionParent = null; + const checkedNodes = [vNode]; + + while (parentVNode) { + if (getScroll(parentVNode.actualNode)) { + scrollRegionParent = parentVNode; + break; + } + + if (parentVNode._scrollRegionParent) { + scrollRegionParent = parentVNode._scrollRegionParent; + break; + } + + checkedNodes.push(parentVNode); + parentVNode = getNodeFromTree( + parentVNode.actualNode.parentElement || parentVNode.actualNode.parentNode + ); + } + + // cache result of parent scroll region so we don't have to look up the entire + // tree again for a child node + checkedNodes.forEach( + vNode => (vNode._scrollRegionParent = scrollRegionParent) + ); + return scrollRegionParent; +} + +/** + * Add a node to every cell of the grid it intersects with. + * @param {Grid} + * @param {VirtualNode} + */ +function addNodeToGrid(grid, vNode) { + const gridSize = constants.gridSize; + // save a reference to where this element is in the grid so we + // can find it even if it's in a subgrid + vNode._grid = grid; + + vNode.clientRects.forEach(rect => { + const x = rect.left; + const y = rect.top; + + // "| 0" is a faster way to do Math.floor + // @see https://jsperf.com/math-floor-vs-math-round-vs-parseint/152 + const startRow = (y / gridSize) | 0; + const startCol = (x / gridSize) | 0; + const endRow = ((y + rect.height) / gridSize) | 0; + const endCol = ((x + rect.width) / gridSize) | 0; + + grid.numCols = Math.max(grid.numCols ?? 0, endCol); + + for (let row = startRow; row <= endRow; row++) { + grid.cells[row] = grid.cells[row] || []; + + for (let col = startCol; col <= endCol; col++) { + grid.cells[row][col] = grid.cells[row][col] || []; + + if (!grid.cells[row][col].includes(vNode)) { + grid.cells[row][col].push(vNode); + } + } + } + }); +} diff --git a/lib/commons/dom/find-nearby-elms.js b/lib/commons/dom/find-nearby-elms.js index 6779fc34db..f4420afb1d 100644 --- a/lib/commons/dom/find-nearby-elms.js +++ b/lib/commons/dom/find-nearby-elms.js @@ -1,4 +1,4 @@ -import { createGrid } from './get-rect-stack'; +import createGrid from './create-grid'; function findNearbyElms(vNode, margin = 0) { /*eslint no-bitwise: 0*/ diff --git a/lib/commons/dom/get-element-stack.js b/lib/commons/dom/get-element-stack.js index 00a4137805..132a20f2bf 100644 --- a/lib/commons/dom/get-element-stack.js +++ b/lib/commons/dom/get-element-stack.js @@ -1,5 +1,6 @@ -import { createGrid, getRectStack } from './get-rect-stack'; +import { getRectStack } from './get-rect-stack'; import { getNodeFromTree } from '../../core/utils'; +import createGrid from './create-grid'; /** * Return all elements that are at the center bounding rect of the passed in node. diff --git a/lib/commons/dom/get-rect-stack.js b/lib/commons/dom/get-rect-stack.js index cda905a090..427da62366 100644 --- a/lib/commons/dom/get-rect-stack.js +++ b/lib/commons/dom/get-rect-stack.js @@ -1,109 +1,6 @@ /* eslint no-bitwise: 0 */ -import isVisible from './is-visible'; import visuallySort from './visually-sort'; -import VirtualNode from '../../core/base/virtual-node/virtual-node'; -import { getNodeFromTree, getScroll, isShadowRoot } from '../../core/utils'; import constants from '../../core/constants'; -import cache from '../../core/base/cache'; - -// split the page cells to group elements by the position -const gridSize = 200; // arbitrary size, increase to reduce memory use (less cells) but increase time (more nodes per grid to check collision) - -/** - * Setup the 2d grid and add every element to it, even elements not - * included in the flat tree - * @returns gridSize - */ -export function createGrid( - root = document.body, - rootGrid = { - container: null, - cells: [] - }, - parentVNode = null -) { - // Prevent multiple calls per run - if (cache.get('gridCreated') && !parentVNode) { - return constants.gridSize; - } - cache.set('gridCreated', true); - - // by not starting at the htmlElement we don't have to pass a custom - // filter function into the treeWalker to filter out head elements, - // which would be called for every node - if (!parentVNode) { - let vNode = getNodeFromTree(document.documentElement); - if (!vNode) { - vNode = new VirtualNode(document.documentElement); - } - - vNode._stackingOrder = [0]; - addNodeToGrid(rootGrid, vNode); - - if (getScroll(vNode.actualNode)) { - const subGrid = { - container: vNode, - cells: [] - }; - vNode._subGrid = subGrid; - } - } - - // IE11 requires the first 3 parameters - // @see https://developer.mozilla.org/en-US/docs/Web/API/Document/createTreeWalker - const treeWalker = document.createTreeWalker( - root, - window.NodeFilter.SHOW_ELEMENT, - null, - false - ); - let node = parentVNode ? treeWalker.nextNode() : treeWalker.currentNode; - while (node) { - let vNode = getNodeFromTree(node); - - // an svg in IE11 does not have a parentElement but instead has a - // parentNode. but parentNode could be a shadow root so we need to - // verify it's in the tree first - if (node.parentElement) { - parentVNode = getNodeFromTree(node.parentElement); - } else if (node.parentNode && getNodeFromTree(node.parentNode)) { - parentVNode = getNodeFromTree(node.parentNode); - } - - if (!vNode) { - vNode = new axe.VirtualNode(node, parentVNode); - } - - vNode._stackingOrder = getStackingOrder(vNode, parentVNode); - - const scrollRegionParent = findScrollRegionParent(vNode, parentVNode); - const grid = scrollRegionParent ? scrollRegionParent._subGrid : rootGrid; - - if (getScroll(vNode.actualNode)) { - const subGrid = { - container: vNode, - cells: [] - }; - vNode._subGrid = subGrid; - } - - // filter out any elements with 0 width or height - // (we don't do this before so we can calculate stacking context - // of parents with 0 width/height) - const rect = vNode.boundingClientRect; - if (rect.width !== 0 && rect.height !== 0 && isVisible(node)) { - addNodeToGrid(grid, vNode); - } - - // add shadow root elements to the grid - if (isShadowRoot(node)) { - createGrid(node.shadowRoot, grid, vNode); - } - - node = treeWalker.nextNode(); - } - return constants.gridSize; -} export function getRectStack(grid, rect, recursed = false) { // use center point of rect @@ -118,8 +15,8 @@ export function getRectStack(grid, rect, recursed = false) { // Chrome appears to round the number up and return the element while Firefox // keeps the number as is and won't return the element. In this case, we // went with pixel perfect collision rather than rounding - const row = floor(y / gridSize); - const col = floor(x / gridSize); + const row = floor(y / constants.gridSize); + const col = floor(x / constants.gridSize); // we're making an assumption that there cannot be an element in the // grid which escapes the grid bounds. For example, if the grid is 4x4 there @@ -177,257 +74,3 @@ export function getRectStack(grid, rect, recursed = false) { function floor(float) { return float | 0; } - -/** - * Determine if node produces a stacking context. - * References: - * https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Positioning/Understanding_z_index/The_stacking_context - * https://github.com/gwwar/z-context/blob/master/devtools/index.js - * @param {VirtualNode} vNode - * @return {Boolean} - */ -function isStackingContext(vNode, parentVNode) { - const position = vNode.getComputedStylePropertyValue('position'); - const zIndex = vNode.getComputedStylePropertyValue('z-index'); - - // the root element (HTML) is skipped since we always start with a - // stacking order of [0] - - // position: fixed or sticky - if (position === 'fixed' || position === 'sticky') { - return true; - } - - // positioned (absolutely or relatively) with a z-index value other than "auto", - if (zIndex !== 'auto' && position !== 'static') { - return true; - } - - // elements with an opacity value less than 1. - if (vNode.getComputedStylePropertyValue('opacity') !== '1') { - return true; - } - - // elements with a transform value other than "none" - const transform = - vNode.getComputedStylePropertyValue('-webkit-transform') || - vNode.getComputedStylePropertyValue('-ms-transform') || - vNode.getComputedStylePropertyValue('transform') || - 'none'; - - if (transform !== 'none') { - return true; - } - - // elements with a mix-blend-mode value other than "normal" - const mixBlendMode = vNode.getComputedStylePropertyValue('mix-blend-mode'); - if (mixBlendMode && mixBlendMode !== 'normal') { - return true; - } - - // elements with a filter value other than "none" - const filter = vNode.getComputedStylePropertyValue('filter'); - if (filter && filter !== 'none') { - return true; - } - - // elements with a perspective value other than "none" - const perspective = vNode.getComputedStylePropertyValue('perspective'); - if (perspective && perspective !== 'none') { - return true; - } - - // element with a clip-path value other than "none" - const clipPath = vNode.getComputedStylePropertyValue('clip-path'); - if (clipPath && clipPath !== 'none') { - return true; - } - - // element with a mask value other than "none" - const mask = - vNode.getComputedStylePropertyValue('-webkit-mask') || - vNode.getComputedStylePropertyValue('mask') || - 'none'; - if (mask !== 'none') { - return true; - } - - // element with a mask-image value other than "none" - const maskImage = - vNode.getComputedStylePropertyValue('-webkit-mask-image') || - vNode.getComputedStylePropertyValue('mask-image') || - 'none'; - if (maskImage !== 'none') { - return true; - } - - // element with a mask-border value other than "none" - const maskBorder = - vNode.getComputedStylePropertyValue('-webkit-mask-border') || - vNode.getComputedStylePropertyValue('mask-border') || - 'none'; - if (maskBorder !== 'none') { - return true; - } - - // elements with isolation set to "isolate" - if (vNode.getComputedStylePropertyValue('isolation') === 'isolate') { - return true; - } - - // transform or opacity in will-change even if you don't specify values for these attributes directly - const willChange = vNode.getComputedStylePropertyValue('will-change'); - if (willChange === 'transform' || willChange === 'opacity') { - return true; - } - - // elements with -webkit-overflow-scrolling set to "touch" - if ( - vNode.getComputedStylePropertyValue('-webkit-overflow-scrolling') === - 'touch' - ) { - return true; - } - - // element with a contain value of "layout" or "paint" or a composite value - // that includes either of them (i.e. contain: strict, contain: content). - const contain = vNode.getComputedStylePropertyValue('contain'); - if (['layout', 'paint', 'strict', 'content'].includes(contain)) { - return true; - } - - // a flex item or gird item with a z-index value other than "auto", that is the parent element display: flex|inline-flex|grid|inline-grid, - if (zIndex !== 'auto' && parentVNode) { - const parentDsiplay = parentVNode.getComputedStylePropertyValue('display'); - if ( - [ - 'flex', - 'inline-flex', - 'inline flex', - 'grid', - 'inline-grid', - 'inline grid' - ].includes(parentDsiplay) - ) { - return true; - } - } - - return false; -} - -/** - * Determine the stacking order of an element. The stacking order is an array of - * zIndex values for each stacking context parent. - * @param {VirtualNode} - * @return {Number[]} - */ -function getStackingOrder(vNode, parentVNode) { - const stackingOrder = parentVNode._stackingOrder.slice(); - const zIndex = vNode.getComputedStylePropertyValue('z-index'); - const positioned = - vNode.getComputedStylePropertyValue('position') !== 'static'; - const floated = vNode.getComputedStylePropertyValue('float') !== 'none'; - - if (positioned && !['auto', '0'].includes(zIndex)) { - // if a positioned element has a z-index > 0, find the first - // true stack (not a "fake" stack created from positioned or - // floated elements without a z-index) and create a new stack at - // that point (step #5 and step #8) - // @see https://www.w3.org/Style/css2-updates/css2/zindex.html - while (stackingOrder.find(value => value % 1 !== 0)) { - const index = stackingOrder.findIndex(value => value % 1 !== 0); - stackingOrder.splice(index, 1); - } - stackingOrder[stackingOrder.length - 1] = parseInt(zIndex); - } - if (isStackingContext(vNode, parentVNode)) { - stackingOrder.push(0); - } - // if a positioned element has z-index: auto or 0 (step #8), or if - // a non-positioned floating element (step #5), treat it as its - // own stacking context - // @see https://www.w3.org/Style/css2-updates/css2/zindex.html - else if (positioned) { - // Put positioned elements above floated elements - stackingOrder.push(0.5); - } else if (floated) { - // Put floated elements above z-index: 0 - // (step #5 floating get sorted below step #8 positioned) - stackingOrder.push(0.25); - } - - return stackingOrder; -} - -/** - * Return the parent node that is a scroll region. - * @param {VirtualNode} - * @return {VirtualNode|null} - */ -function findScrollRegionParent(vNode, parentVNode) { - let scrollRegionParent = null; - const checkedNodes = [vNode]; - - while (parentVNode) { - if (getScroll(parentVNode.actualNode)) { - scrollRegionParent = parentVNode; - break; - } - - if (parentVNode._scrollRegionParent) { - scrollRegionParent = parentVNode._scrollRegionParent; - break; - } - - checkedNodes.push(parentVNode); - parentVNode = getNodeFromTree( - parentVNode.actualNode.parentElement || parentVNode.actualNode.parentNode - ); - } - - // cache result of parent scroll region so we don't have to look up the entire - // tree again for a child node - checkedNodes.forEach( - vNode => (vNode._scrollRegionParent = scrollRegionParent) - ); - return scrollRegionParent; -} - -/** - * Add a node to every cell of the grid it intersects with. - * @param {Grid} - * @param {VirtualNode} - */ -function addNodeToGrid(grid, vNode) { - const gridSize = constants.gridSize; - // save a reference to where this element is in the grid so we - // can find it even if it's in a subgrid - vNode._grid = grid; - - vNode.clientRects.forEach(rect => { - const x = rect.left; - const y = rect.top; - - // "| 0" is a faster way to do Math.floor - // @see https://jsperf.com/math-floor-vs-math-round-vs-parseint/152 - const startRow = (y / gridSize) | 0; - const startCol = (x / gridSize) | 0; - const endRow = ((y + rect.height) / gridSize) | 0; - const endCol = ((x + rect.width) / gridSize) | 0; - - grid.numCols = Math.max(grid.numCols ?? 0, endCol); - - for (let row = startRow; row <= endRow; row++) { - grid.cells[row] = grid.cells[row] || []; - - for (let col = startCol; col <= endCol; col++) { - grid.cells[row][col] = grid.cells[row][col] || []; - - if (!grid.cells[row][col].includes(vNode)) { - grid.cells[row][col].push(vNode); - } - } - } - }); -} diff --git a/lib/commons/dom/get-text-element-stack.js b/lib/commons/dom/get-text-element-stack.js index f6e2e4d60f..641e8d15fc 100644 --- a/lib/commons/dom/get-text-element-stack.js +++ b/lib/commons/dom/get-text-element-stack.js @@ -1,5 +1,6 @@ import getElementStack from './get-element-stack'; -import { createGrid, getRectStack } from './get-rect-stack'; +import { getRectStack } from './get-rect-stack'; +import createGrid from './create-grid'; import sanitize from '../text/sanitize'; import { getNodeFromTree } from '../../core/utils'; diff --git a/lib/commons/dom/index.js b/lib/commons/dom/index.js index adeecb7f44..8d4c02ef4c 100644 --- a/lib/commons/dom/index.js +++ b/lib/commons/dom/index.js @@ -41,4 +41,4 @@ export { default as urlPropsFromAttribute } from './url-props-from-attribute'; export { default as visuallyContains } from './visually-contains'; export { default as visuallyOverlaps } from './visually-overlaps'; export { default as visuallySort } from './visually-sort'; -export { createGrid } from './get-rect-stack'; +export { default as createGrid } from './create-grid'; diff --git a/lib/commons/dom/visually-sort.js b/lib/commons/dom/visually-sort.js index aff9ffc314..9dd22bc015 100644 --- a/lib/commons/dom/visually-sort.js +++ b/lib/commons/dom/visually-sort.js @@ -1,3 +1,4 @@ +import createGrid from './create-grid'; /** * Visually sort nodes based on their stack order * References: @@ -7,6 +8,7 @@ */ export default function visuallySort(a, b) { /*eslint no-bitwise: 0 */ + createGrid(); // Because we need ._stackingOrder const length = Math.max(a._stackingOrder.length, b._stackingOrder.length); for (let i = 0; i < length; i++) { diff --git a/test/commons/dom/visually-sort.js b/test/commons/dom/visually-sort.js index fe764e9fdd..1b943d30f4 100644 --- a/test/commons/dom/visually-sort.js +++ b/test/commons/dom/visually-sort.js @@ -5,14 +5,8 @@ describe('visually-sort', function () { var fixtureSetup = axe.testUtils.fixtureSetup; var visuallySort = axe.commons.dom.visuallySort; - // visuallySort requires the grid to be set up - // For performance reasons axe-core does not do this in setup, - // so we'll need to call it ourselves. - var createGrid = axe.commons.dom.createGrid; - it('returns 1 if B overlaps A', function () { var rootNode = fixtureSetup('bar'); - createGrid(); var vNodeA = rootNode.children[0]; var vNodeB = vNodeA.children[0]; assert.equal(visuallySort(vNodeA, vNodeB), 1); @@ -20,7 +14,6 @@ describe('visually-sort', function () { it('returns -1 if A overlaps B', function () { var rootNode = fixtureSetup('bar'); - createGrid(); var vNodeB = rootNode.children[0]; var vNodeA = vNodeB.children[0]; assert.equal(visuallySort(vNodeA, vNodeB), -1); From 10b8972607abbe5a004d309ff5c0f9b6195a643c Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Fri, 2 Sep 2022 15:27:59 +0200 Subject: [PATCH 13/28] Make all links focusable --- test/integration/full/target-size/target-size.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/integration/full/target-size/target-size.js b/test/integration/full/target-size/target-size.js index b9652e2390..4061b1d78f 100644 --- a/test/integration/full/target-size/target-size.js +++ b/test/integration/full/target-size/target-size.js @@ -4,6 +4,11 @@ describe('target-size test', function () { before(function (done) { axe.testUtils.awaitNestedLoad(function () { + // Make all links focusable, otherwise the rule won't run + document.querySelectorAll('[role="link"]').forEach(function (link) { + link.setAttribute('tabindex', '0'); + }); + var options = { runOnly: ['target-size'], elementRef: true From 297524f099486ea65669868f55304574ec0474a1 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Fri, 2 Sep 2022 15:48:04 +0200 Subject: [PATCH 14/28] Simplify target-size full rest --- .../full/target-size/target-size.html | 322 +++++++++--------- .../full/target-size/target-size.js | 5 +- 2 files changed, 164 insertions(+), 163 deletions(-) diff --git a/test/integration/full/target-size/target-size.html b/test/integration/full/target-size/target-size.html index 87c413b247..02a36e9a25 100644 --- a/test/integration/full/target-size/target-size.html +++ b/test/integration/full/target-size/target-size.html @@ -257,167 +257,167 @@

Examples A1 - A4 all pass.

- - - + + +
- - - + + +
- - - + + +
- - - + + +

Examples B1 - B3 pass, B4 fails.

- - - + + +
- - - + + +
- - - + + +
- - - + + +

Examples C1 and C2 pass, C3 and C4 fail.

- - - + + +
- - - + + +
- - - + + +
- - - + + +

Example D1 passes, D2 - D4 fail.

- - - + + +
- - - + + +
- - - + + +
- - - + + +

Example E1 and E2 pass, the two outside elements of E3 and E4 fail.

- - - + + +
- - - + + +
- - - + + +
- - - + + +

Example F1 and F2 pass, the inside element of F3 and F4 fail.

- - - + + +
- - - + + +
- - - + + +
- - - + + +

Example G1 - G3 pass, the outer element of G4 fail.

- - + +
- - + +
- - + +
- - + +
@@ -425,24 +425,24 @@

Example H1 and H2 pass, the outer element of H3 and H4 fail.

- - + +
- - + +
- - + +
- - - + + +
@@ -450,23 +450,23 @@

Example I1 and I2 pass, I3 and I4 fail.

- - + +
- - + +
- - + +
- - + +
@@ -474,23 +474,23 @@

Example J1 and J2 pass, J3 and J4 fail.

- - + +
- - + +
- - + +
- - + +
@@ -498,23 +498,23 @@

Example K1 - K3 pass, the middle element of K4 fails.

- - + +
- - + +
- - + +
- - + +
@@ -522,49 +522,49 @@

Example L1 and L2 pass, the outer element of L3 and L4 fail.

- - - + + +
- - - + + +
- - - + + +
- - - + + +

Example M1 - M3 pass, the outer element of M4 fail.

- - + +
- - + +
- - + +
- - + +
@@ -572,23 +572,23 @@

Example N1 - N3 pass, the outer element of N4 fail.

- - + +
- - + +
- - + +
- - + +
@@ -596,51 +596,51 @@

Example O1 and O2 pass, the inner element of O3 and O4 fail.

- - + + - +
- - + + - +
- - + + - +
- - + + - +

Example P.

- - + +
- - + +
- - + +
- - + +
diff --git a/test/integration/full/target-size/target-size.js b/test/integration/full/target-size/target-size.js index 4061b1d78f..68287e6a8b 100644 --- a/test/integration/full/target-size/target-size.js +++ b/test/integration/full/target-size/target-size.js @@ -4,8 +4,9 @@ describe('target-size test', function () { before(function (done) { axe.testUtils.awaitNestedLoad(function () { - // Make all links focusable, otherwise the rule won't run - document.querySelectorAll('[role="link"]').forEach(function (link) { + // Add necessary markup for axe to recognize these as components: + document.querySelectorAll('section span').forEach(function (link) { + link.setAttribute('role', 'link'); link.setAttribute('tabindex', '0'); }); From a25d7a0888e918c8045443806e132fcf3f2940b0 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Fri, 2 Sep 2022 15:49:55 +0200 Subject: [PATCH 15/28] Try to fix IE issue --- test/checks/mobile/target-offset.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/checks/mobile/target-offset.js b/test/checks/mobile/target-offset.js index dde72166ab..51b74ee8ef 100644 --- a/test/checks/mobile/target-offset.js +++ b/test/checks/mobile/target-offset.js @@ -68,15 +68,15 @@ describe('target-offset tests', function () { it('sets all elements that are too close as related nodes', function () { var checkArgs = checkSetup( - '' + - '' + - '' + '">x' ); assert.isFalse(check.evaluate.apply(checkContext, checkArgs)); assert.equal(checkContext._data.minOffset, 24); From f6b97dfb6bf193cb438b1871f1d22d480f7224d6 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Fri, 2 Sep 2022 16:08:36 +0200 Subject: [PATCH 16/28] More cleanup and testing --- lib/commons/math/split-rects.js | 4 -- lib/rules/widget-not-inline-matches.js | 2 - test/checks/mobile/target-offset.js | 24 +++++----- test/commons/math/has-visual-overlap.js | 8 ---- .../rule-matches/widget-not-inline-matches.js | 48 +++++++++++++++---- 5 files changed, 52 insertions(+), 34 deletions(-) diff --git a/lib/commons/math/split-rects.js b/lib/commons/math/split-rects.js index 08fa7f08e0..779fe197b5 100644 --- a/lib/commons/math/split-rects.js +++ b/lib/commons/math/split-rects.js @@ -23,19 +23,15 @@ function splitRect(inputRect, clipRect) { const rects = []; if (between(clipRect.top, top, bottom) && xAligned) { - // console.log('top') rects.push({ top, left, bottom: clipRect.top, right }); } if (between(clipRect.right, left, right) && yAligned) { - // console.log('right', clipRect.right, left, right, top, clipRect.bottom, bottom, inputRect.top) rects.push({ top, left: clipRect.right, bottom, right }); } if (between(clipRect.bottom, top, bottom) && xAligned) { - // console.log('bottom') rects.push({ top: clipRect.bottom, right, bottom, left }); } if (between(clipRect.left, left, right) && yAligned) { - // console.log('left', clipRect.left, left, right) rects.push({ top, left, bottom, right: clipRect.left }); } if (rects.length === 0) { diff --git a/lib/rules/widget-not-inline-matches.js b/lib/rules/widget-not-inline-matches.js index 79312ba1b8..b79000fa06 100644 --- a/lib/rules/widget-not-inline-matches.js +++ b/lib/rules/widget-not-inline-matches.js @@ -16,13 +16,11 @@ const matchesFns = [ function isWidgetType(vNode) { const role = getRole(vNode); - // TODO: These needs tests if ('option' === role) { return false; } else if (['combobox', 'listbox'].includes(role)) { return true; } - const roleType = getRoleType(role); return roleType === 'widget'; } diff --git a/test/checks/mobile/target-offset.js b/test/checks/mobile/target-offset.js index 51b74ee8ef..a9440eabcd 100644 --- a/test/checks/mobile/target-offset.js +++ b/test/checks/mobile/target-offset.js @@ -11,9 +11,9 @@ describe('target-offset tests', function () { it('returns true when there are no other nearby targets', function () { var checkArgs = checkSetup( - '' + '">x' ); assert.isTrue(check.evaluate.apply(checkContext, checkArgs)); @@ -23,12 +23,12 @@ describe('target-offset tests', function () { it('returns true when the offset is 24px', function () { var checkArgs = checkSetup( - '' + - '' + '">x' ); assert.isTrue(check.evaluate.apply(checkContext, checkArgs)); @@ -38,12 +38,12 @@ describe('target-offset tests', function () { it('returns false when the offset is 23px', function () { var checkArgs = checkSetup( - '' + - '' + '">x' ); assert.isFalse(check.evaluate.apply(checkContext, checkArgs)); @@ -53,9 +53,9 @@ describe('target-offset tests', function () { it('ignores non-widget elements as neighbors', function () { var checkArgs = checkSetup( - '' + + '">x' + '
x
' diff --git a/test/commons/math/has-visual-overlap.js b/test/commons/math/has-visual-overlap.js index f06ce628a7..e73f5d27d7 100644 --- a/test/commons/math/has-visual-overlap.js +++ b/test/commons/math/has-visual-overlap.js @@ -3,14 +3,8 @@ describe('hasVisualOverlap', function () { var fixtureSetup = axe.testUtils.fixtureSetup; var hasVisualOverlap = axe.commons.math.hasVisualOverlap; - // hasVisualOverlap requires the grid to be set up - // For performance reasons axe-core does not do this in setup, - // so we'll need to call it ourselves. - var createGrid = axe.commons.dom.createGrid; - it('returns false if there is no overlap', function () { var rootNode = fixtureSetup('foobar'); - createGrid(); var vNodeA = rootNode.children[0]; var vNodeB = rootNode.children[1]; assert.isFalse(hasVisualOverlap(vNodeA, vNodeB)); @@ -18,7 +12,6 @@ describe('hasVisualOverlap', function () { it('returns true if B overlaps A', function () { var rootNode = fixtureSetup('bar'); - createGrid(); var vNodeA = rootNode.children[0]; var vNodeB = vNodeA.children[0]; assert.isTrue(hasVisualOverlap(vNodeA, vNodeB)); @@ -26,7 +19,6 @@ describe('hasVisualOverlap', function () { it('returns true A overlaps B', function () { var rootNode = fixtureSetup('bar'); - createGrid(); var vNodeB = rootNode.children[0]; var vNodeA = vNodeB.children[0]; assert.isFalse(hasVisualOverlap(vNodeA, vNodeB)); diff --git a/test/rule-matches/widget-not-inline-matches.js b/test/rule-matches/widget-not-inline-matches.js index b2adb4c466..0fbe259800 100644 --- a/test/rule-matches/widget-not-inline-matches.js +++ b/test/rule-matches/widget-not-inline-matches.js @@ -13,14 +13,6 @@ describe('widget-not-inline-matches', function () { assert.isTrue(rule.matches(node, vNode)); }); - it('returns true for non-native widgets', function () { - var vNode = queryFixture( - '
' - ); - var node = vNode.actualNode; - assert.isTrue(rule.matches(node, vNode)); - }); - it('returns false for non-widget elements', function () { var vNode = queryFixture(''); var node = vNode.actualNode; @@ -39,6 +31,46 @@ describe('widget-not-inline-matches', function () { assert.isFalse(rule.matches(node, vNode)); }); + describe('non-native components', function () { + it('returns true for a tabbable button', function () { + var vNode = queryFixture( + '
' + ); + var node = vNode.actualNode; + assert.isTrue(rule.matches(node, vNode)); + }); + + it('returns false for a non-tabbable button (widgets)', function () { + var vNode = queryFixture('
'); + var node = vNode.actualNode; + assert.isFalse(rule.matches(node, vNode)); + }); + + it('returns false for a non-tabbable button (widgets)', function () { + var vNode = queryFixture( + '
' + ); + var node = vNode.actualNode; + assert.isFalse(rule.matches(node, vNode)); + }); + + it('returns true for a listbox (component)', function () { + var vNode = queryFixture( + '
' + ); + var node = vNode.actualNode; + assert.isTrue(rule.matches(node, vNode)); + }); + + it('returns true for a combobox (component)', function () { + var vNode = queryFixture( + '
' + ); + var node = vNode.actualNode; + assert.isTrue(rule.matches(node, vNode)); + }); + }); + describe('inline components', function () { it('returns false for elements inline with text', function () { var vNode = queryFixture( From 8ab55c7f66a319f4a95cb1d51ed240c427caae31 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Fri, 2 Sep 2022 16:28:29 +0200 Subject: [PATCH 17/28] Solve oddities with IE --- .../integration/rules/target-size/target-size.html | 14 +++++--------- .../integration/rules/target-size/target-size.json | 9 +-------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/test/integration/rules/target-size/target-size.html b/test/integration/rules/target-size/target-size.html index 16878d978f..617ed9a08f 100644 --- a/test/integration/rules/target-size/target-size.html +++ b/test/integration/rules/target-size/target-size.html @@ -6,20 +6,16 @@

- +

-

- - - -

-

Date: Thu, 15 Sep 2022 12:48:41 +0200 Subject: [PATCH 18/28] Rename minSpacing option to minSize --- lib/checks/mobile/target-size-evaluate.js | 13 ++++++------- lib/checks/mobile/target-size.json | 8 ++++---- test/checks/mobile/target-size.js | 14 +++++++------- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/lib/checks/mobile/target-size-evaluate.js b/lib/checks/mobile/target-size-evaluate.js index 93f0681dcd..38ecfbcfda 100644 --- a/lib/checks/mobile/target-size-evaluate.js +++ b/lib/checks/mobile/target-size-evaluate.js @@ -9,17 +9,16 @@ const roundingMargin = 0.05; * any elements that may obscure it. */ export default function targetSize(node, options, vNode) { - const minSpacing = options?.minSpacing || 24; + const minSize = options?.minSize || 24; const hasMinimumSize = ({ width, height }) => { return ( - width + roundingMargin >= minSpacing && - height + roundingMargin >= minSpacing + width + roundingMargin >= minSize && height + roundingMargin >= minSize ); }; const nodeRect = vNode.boundingClientRect; if (!hasMinimumSize(nodeRect)) { - this.data({ minSpacing, ...toDecimalSize(nodeRect) }); + this.data({ minSize, ...toDecimalSize(nodeRect) }); return false; } @@ -31,7 +30,7 @@ export default function targetSize(node, options, vNode) { }); if (obscuringElms.length === 0) { - this.data({ minSpacing, ...toDecimalSize(nodeRect) }); + this.data({ minSize, ...toDecimalSize(nodeRect) }); return true; // No obscuring elements; pass } @@ -57,7 +56,7 @@ export default function targetSize(node, options, vNode) { if (!hasMinimumSize(largestInnerRect)) { this.data({ messageKey: 'obscured', - minSpacing, + minSize, ...toDecimalSize(largestInnerRect) }); this.relatedNodes(obscuringElms.map(({ actualNode }) => actualNode)); @@ -65,7 +64,7 @@ export default function targetSize(node, options, vNode) { return false; } - this.data({ minSpacing, ...toDecimalSize(largestInnerRect) }); + this.data({ minSize, ...toDecimalSize(largestInnerRect) }); return true; } diff --git a/lib/checks/mobile/target-size.json b/lib/checks/mobile/target-size.json index 6c8df407bc..12ed7e037a 100644 --- a/lib/checks/mobile/target-size.json +++ b/lib/checks/mobile/target-size.json @@ -2,15 +2,15 @@ "id": "target-size", "evaluate": "target-size-evaluate", "options": { - "minSpacing": 24 + "minSize": 24 }, "metadata": { "impact": "serious", "messages": { - "pass": "Control has sufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSpacing}px by ${data.minSpacing}px)", + "pass": "Control has sufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)", "fail": { - "default": "Element has insufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSpacing}px by ${data.minSpacing}px)", - "obscured": "Element has insufficient size because it is obscured (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSpacing}px by ${data.minSpacing}px)" + "default": "Element has insufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)", + "obscured": "Element has insufficient size because it is obscured (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)" } } } diff --git a/test/checks/mobile/target-size.js b/test/checks/mobile/target-size.js index af8586e47e..99388cb997 100644 --- a/test/checks/mobile/target-size.js +++ b/test/checks/mobile/target-size.js @@ -14,7 +14,7 @@ describe('target-size tests', function () { // IE cannot count (isIE11 ? xit : it)( - 'returns false for targets smaller than minSpacing', + 'returns false for targets smaller than minSize', function () { var checkArgs = checkSetup( '' - ); - assert.isFalse(check.evaluate.apply(checkContext, checkArgs)); - assert.deepEqual(checkContext._data, { - minSize: 24, - width: 20, - height: 30 - }); - } - ); + it('returns false for targets smaller than minSize', function () { + var checkArgs = checkSetup( + '' + ); + assert.isFalse(check.evaluate.apply(checkContext, checkArgs)); + assert.deepEqual(checkContext._data, { + minSize: 24, + width: 20, + height: 30 + }); + }); it('returns true for unobscured targets larger than minSize', function () { var checkArgs = checkSetup( From 368d612618ed636a15d8223697a2717f2e0d85a0 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Fri, 16 Sep 2022 13:30:26 +0200 Subject: [PATCH 20/28] Fix off screen grid issues --- lib/checks/mobile/target-offset.json | 4 +- lib/commons/dom/create-grid.js | 10 +- lib/commons/dom/find-nearby-elms.js | 44 ++++---- test/commons/dom/create-grid.js | 148 +++++++++++++++++++++++++++ test/commons/dom/find-nearby-elms.js | 67 ++++++++---- 5 files changed, 228 insertions(+), 45 deletions(-) create mode 100644 test/commons/dom/create-grid.js diff --git a/lib/checks/mobile/target-offset.json b/lib/checks/mobile/target-offset.json index c57ca5301a..14c7bd007d 100644 --- a/lib/checks/mobile/target-offset.json +++ b/lib/checks/mobile/target-offset.json @@ -7,8 +7,8 @@ "metadata": { "impact": "serious", "messages": { - "pass": "Target is sufficiently offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)", - "fail": "Target is insufficient offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)" + "pass": "Target has sufficiently offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)", + "fail": "Target has insufficient offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)" } } } diff --git a/lib/commons/dom/create-grid.js b/lib/commons/dom/create-grid.js index 0d9736d6b5..6d50c61ba7 100644 --- a/lib/commons/dom/create-grid.js +++ b/lib/commons/dom/create-grid.js @@ -324,11 +324,13 @@ function findScrollRegionParent(vNode, parentVNode) { */ function addNodeToGrid(grid, vNode) { const gridSize = constants.gridSize; - // save a reference to where this element is in the grid so we - // can find it even if it's in a subgrid - vNode._grid = grid; - vNode.clientRects.forEach(rect => { + if (rect.right <= 0 || rect.bottom <= 0) { + return; + } + // save a reference to where this element is in the grid so we + // can find it even if it's in a subgrid + vNode._grid ??= grid; const x = rect.left; const y = rect.top; diff --git a/lib/commons/dom/find-nearby-elms.js b/lib/commons/dom/find-nearby-elms.js index f4420afb1d..49e85046be 100644 --- a/lib/commons/dom/find-nearby-elms.js +++ b/lib/commons/dom/find-nearby-elms.js @@ -1,33 +1,39 @@ import createGrid from './create-grid'; -function findNearbyElms(vNode, margin = 0) { +export default function findNearbyElms(vNode, margin = 0) { /*eslint no-bitwise: 0*/ const gridSize = createGrid(); - const neighbors = []; + if (!vNode._grid?.cells?.length) { + return []; // Elements not in the grid don't have ._grid + } + const rect = vNode.boundingClientRect; const gridCells = vNode._grid.cells; + const boundaries = { + topRow: ((rect.top - margin) / gridSize) | 0, + bottomRow: ((rect.bottom + margin) / gridSize) | 0, + leftCol: ((rect.left - margin) / gridSize) | 0, + rightCol: ((rect.right + margin) / gridSize) | 0 + }; - const topRow = ((rect.top - margin) / gridSize) | 0; - const bottomRow = ((rect.bottom + margin) / gridSize) | 0; - const leftCol = ((rect.left - margin) / gridSize) | 0; - const rightCol = ((rect.right + margin) / gridSize) | 0; + const neighbors = []; + loopGridCells(gridCells, boundaries, vNeighbor => { + if (vNeighbor && vNeighbor !== vNode && !neighbors.includes(vNeighbor)) { + neighbors.push(vNeighbor); + } + }); + return neighbors; +} +function loopGridCells(gridCells, boundaries, cb) { + const { topRow, bottomRow, leftCol, rightCol } = boundaries; for (let row = topRow; row <= bottomRow; row++) { for (let col = leftCol; col <= rightCol; col++) { - for (let i = 0; i <= gridCells[row][col].length; i++) { - var vNeighbour = gridCells[row][col][i]; - // Avoid duplication - if ( - vNeighbour && - vNeighbour !== vNode && - !neighbors.includes(vNeighbour) - ) { - neighbors.push(vNeighbour); - } + // Don't loop on elements outside the grid + const length = gridCells[row]?.[col]?.length ?? -1; + for (let i = 0; i <= length; i++) { + cb(gridCells[row][col][i]); } } } - return neighbors; } - -export default findNearbyElms; diff --git a/test/commons/dom/create-grid.js b/test/commons/dom/create-grid.js new file mode 100644 index 0000000000..66ee4d73b4 --- /dev/null +++ b/test/commons/dom/create-grid.js @@ -0,0 +1,148 @@ +// Additional tests for createGrid are part of createRectStack tests, +// which is what createGrid was originally part of +describe('create-grid', function () { + var fixture; + var createGrid = axe.commons.dom.createGrid; + var fixtureSetup = axe.testUtils.fixtureSetup; + + function findPositions(grid, vNode) { + var positions = []; + grid.cells.forEach(function (rowCells, rowIndex) { + rowCells.forEach(function (cells, colIndex) { + if (cells.includes(vNode)) { + positions.push({ x: rowIndex, y: colIndex }); + } + }); + }); + return positions; + } + + it('returns the grid size', function () { + axe.setup(); + assert.equal(createGrid(), axe.constants.gridSize); + }); + + it('sets ._grid to nodes in the grid', function () { + fixture = fixtureSetup('Hello world'); + assert.isUndefined(fixture._grid); + assert.isUndefined(fixture.children[0]._grid); + + createGrid(); + assert.isDefined(fixture._grid); + assert.equal(fixture._grid, fixture.children[0]._grid); + }); + + it('adds elements to the correct cell in the grid', function () { + fixture = fixtureSetup('Hello world'); + createGrid(); + var positions = findPositions(fixture._grid, fixture.children[0]); + assert.deepEqual(positions, [{ x: 0, y: 0 }]); + }); + + it('adds large elements to multiple cell', function () { + fixture = fixtureSetup( + '' + + 'Hello world' + ); + createGrid(); + + var positions = findPositions(fixture._grid, fixture.children[0]); + assert.deepEqual(positions, [ + { x: 0, y: 0 }, + { x: 0, y: 1 }, + { x: 1, y: 0 }, + { x: 1, y: 1 } + ]); + }); + + describe('hidden elements', function () { + beforeEach(function () { + // Ensure the fixture itself is part of the grid, even if its content isn't + document + .querySelector('#fixture') + .setAttribute('style', 'min-height: 10px'); + }); + + it('does not add hidden elements', function () { + fixture = fixtureSetup('

hidden
'); + createGrid(); + var position = findPositions(fixture._grid, fixture.children[0]); + assert.isEmpty(position); + assert.isUndefined(fixture.children[0]._grid); + }); + + it('does not add off screen elements', function () { + fixture = fixtureSetup( + '
off screen
' + ); + createGrid(); + var position = findPositions(fixture._grid, fixture.children[0]); + assert.isEmpty(position); + assert.isUndefined(fixture.children[0]._grid); + }); + + it('does add partially on screen elements', function () { + fixture = fixtureSetup( + '
off screen
' + ); + createGrid(); + var position = findPositions(fixture._grid, fixture.children[0]); + assert.deepEqual(position, [{ x: 0, y: 0 }]); + }); + }); + + describe('subGrids', function () { + it('sets the .subGrid property', function () { + fixture = fixtureSetup( + '
' + + 'x' + + '
' + ); + var vOverflow = fixture.children[0]; + assert.isUndefined(vOverflow._subGrid); + createGrid(); + assert.isDefined(vOverflow._subGrid); + assert.notEqual(vOverflow._grid, vOverflow._subGrid); + }); + + it('sets the ._grid of children as the subGrid', function () { + fixture = fixtureSetup( + '
' + + 'x' + + '
' + ); + createGrid(); + var vOverflow = fixture.children[0]; + var vSpan = vOverflow.children[0]; + assert.equal(vOverflow._subGrid, vSpan._grid); + }); + + it('does not add scrollable children to the root grid', function () { + fixture = fixtureSetup( + '
' + + 'x' + + '
' + ); + createGrid(); + var vSpan = fixture.children[0].children[0]; + var position = findPositions(fixture._grid, vSpan); + assert.isEmpty(position); + }); + + it('adds scrollable children to the subGrid', function () { + fixture = fixtureSetup( + '
' + + 'x' + + '
' + ); + createGrid(); + var vOverflow = fixture.children[0]; + var vSpan = vOverflow.children[0]; + var position = findPositions(vOverflow._subGrid, vSpan); + assert.deepEqual(position, [ + { x: 0, y: 0 }, + { x: 1, y: 0 } + ]); + }); + }); +}); diff --git a/test/commons/dom/find-nearby-elms.js b/test/commons/dom/find-nearby-elms.js index 3468ded228..548ba3fc64 100644 --- a/test/commons/dom/find-nearby-elms.js +++ b/test/commons/dom/find-nearby-elms.js @@ -14,28 +14,55 @@ describe('findNearbyElms', function () { return ids; } - beforeEach(function () { - fixture = fixtureSetup( - '
0
' + - '
1
' + - '
2
' + - '
3
' + - '
4
' + - '
5
' + - '
6
' + - '
7
' + - '
8
' + - '
9
' - ); - }); + describe('in the viewport', function () { + beforeEach(function () { + fixture = fixtureSetup( + '
0
' + + '
1
' + + '
2
' + + '
3
' + + '
4
' + + '
5
' + + '
6
' + + '
7
' + + '
8
' + + '
9
' + ); + }); + + it('returns node from the same grid cell', function () { + var nearbyElms = findNearbyElms(fixture.children[1]); + assert.deepEqual(getIds(nearbyElms), ['n0', 'n2', 'n3']); + }); - it('returns node from the same grid cell', function () { - var nearbyElms = findNearbyElms(fixture.children[1]); - assert.deepEqual(getIds(nearbyElms), ['n0', 'n2', 'n3']); + it('returns node from multiple grid cells when crossing a boundary', function () { + var nearbyElms = findNearbyElms(fixture.children[5]); + assert.deepEqual(getIds(nearbyElms), ['n3', 'n4', 'n6']); + }); }); - it('returns node from multiple grid cells when crossing a boundary', function () { - var nearbyElms = findNearbyElms(fixture.children[5]); - assert.deepEqual(getIds(nearbyElms), ['n3', 'n4', 'n6']); + describe('on the edge', function () { + beforeEach(function () { + fixture = fixtureSetup( + '
0
' + + '
1
' + + '
2
' + ); + }); + + it('ignores cells outside the document boundary', function () { + var nearbyElms = findNearbyElms(fixture.children[0]); + assert.deepEqual(getIds(nearbyElms), ['n2']); + }); + + it('returns no neighbors for off-screen elements', function () { + var nearbyElms = findNearbyElms(fixture.children[1]); + assert.deepEqual(getIds(nearbyElms), []); + }); + + it('returns element partially on screen as neighbors', function () { + var nearbyElms = findNearbyElms(fixture.children[2]); + assert.deepEqual(getIds(nearbyElms), ['n0']); + }); }); }); From b4462e5f79df8d0988aeb0fa4ad41f9eda17c0e2 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Fri, 16 Sep 2022 14:28:28 +0200 Subject: [PATCH 21/28] Simplify using role types --- .../keyboard/no-focusable-content-evaluate.js | 5 +- lib/checks/mobile/target-offset-evaluate.js | 5 +- lib/checks/mobile/target-size-evaluate.js | 10 ++-- lib/commons/aria/get-role-type.js | 13 +++--- lib/commons/dom/is-in-text-block.js | 10 ++-- lib/rules/widget-not-inline-matches.js | 11 +---- lib/standards/aria-roles.js | 4 +- locales/_template.json | 10 ++-- test/commons/aria/get-role-type.js | 46 ++++++++++++++++--- .../rule-matches/widget-not-inline-matches.js | 8 ---- 10 files changed, 66 insertions(+), 56 deletions(-) diff --git a/lib/checks/keyboard/no-focusable-content-evaluate.js b/lib/checks/keyboard/no-focusable-content-evaluate.js index f72bad79d7..cb0b38cf98 100644 --- a/lib/checks/keyboard/no-focusable-content-evaluate.js +++ b/lib/checks/keyboard/no-focusable-content-evaluate.js @@ -1,5 +1,5 @@ import isFocusable from '../../commons/dom/is-focusable'; -import { getRole, getRoleType } from '../../commons/aria'; +import { getRoleType } from '../../commons/aria'; export default function noFocusableContentEvaluate(node, options, virtualNode) { if (!virtualNode.children) { @@ -41,8 +41,7 @@ function getFocusableDescendants(vNode) { const retVal = []; vNode.children.forEach(child => { - const role = getRole(child); - if (getRoleType(role) === 'widget' && isFocusable(child)) { + if (getRoleType(child) === 'widget' && isFocusable(child)) { retVal.push(child); } else { retVal.push(...getFocusableDescendants(child)); diff --git a/lib/checks/mobile/target-offset-evaluate.js b/lib/checks/mobile/target-offset-evaluate.js index 45c2ba075c..ac65277180 100644 --- a/lib/checks/mobile/target-offset-evaluate.js +++ b/lib/checks/mobile/target-offset-evaluate.js @@ -1,5 +1,5 @@ import { findNearbyElms } from '../../commons/dom'; -import { getRole, getRoleType } from '../../commons/aria'; +import { getRoleType } from '../../commons/aria'; import { getOffset } from '../../commons/math'; const roundingMargin = 0.05; @@ -9,8 +9,7 @@ export default function targetOffsetEvaluate(node, options, vNode) { const closeNeighbors = []; let closestOffset = minOffset; for (const vNeighbor of findNearbyElms(vNode, minOffset)) { - const role = getRole(vNeighbor); - if (getRoleType(role) !== 'widget') { + if (getRoleType(vNeighbor) !== 'widget') { continue; } const offset = roundToSingleDecimal(getOffset(vNode, vNeighbor)); diff --git a/lib/checks/mobile/target-size-evaluate.js b/lib/checks/mobile/target-size-evaluate.js index 38ecfbcfda..1a760d228e 100644 --- a/lib/checks/mobile/target-size-evaluate.js +++ b/lib/checks/mobile/target-size-evaluate.js @@ -1,5 +1,5 @@ import { findNearbyElms } from '../../commons/dom'; -import { getRole, getRoleType } from '../../commons/aria'; +import { getRoleType } from '../../commons/aria'; import { splitRects, hasVisualOverlap } from '../../commons/math'; const roundingMargin = 0.05; @@ -24,10 +24,10 @@ export default function targetSize(node, options, vNode) { // Handle overlapping elements; const nearbyElms = findNearbyElms(vNode); - const obscuringElms = nearbyElms.filter(vNeighbor => { - const role = getRole(vNeighbor); - return getRoleType(role) === 'widget' && hasVisualOverlap(vNode, vNeighbor); - }); + const obscuringElms = nearbyElms.filter( + vNeighbor => + getRoleType(vNeighbor) === 'widget' && hasVisualOverlap(vNode, vNeighbor) + ); if (obscuringElms.length === 0) { this.data({ minSize, ...toDecimalSize(nodeRect) }); diff --git a/lib/commons/aria/get-role-type.js b/lib/commons/aria/get-role-type.js index 84ca396ca3..1cd4c54fa3 100644 --- a/lib/commons/aria/get-role-type.js +++ b/lib/commons/aria/get-role-type.js @@ -1,21 +1,20 @@ import standards from '../../standards'; +import AbstractVirtualNode from '../../core/base/virtual-node/abstract-virtual-node'; /** * Get the "type" of role; either widget, composite, abstract, landmark or `null` * @method getRoleType * @memberof axe.commons.aria * @instance - * @param {String} role The role to check + * @param {String|Node|Element} role The role to check, or element to check the role of * @return {Mixed} String if a matching role and its type are found, otherwise `null` */ function getRoleType(role) { - const roleDef = standards.ariaRoles[role]; - - if (!roleDef) { - return null; + if (role instanceof window.Node || role instanceof AbstractVirtualNode) { + role = axe.commons.aria.getRole(role); } - - return roleDef.type; + const roleDef = standards.ariaRoles[role]; + return roleDef?.type || null; } export default getRoleType; diff --git a/lib/commons/dom/is-in-text-block.js b/lib/commons/dom/is-in-text-block.js index 52f3635e5a..f830d056af 100644 --- a/lib/commons/dom/is-in-text-block.js +++ b/lib/commons/dom/is-in-text-block.js @@ -16,6 +16,7 @@ const blockLike = [ 'grid', 'inline-block' ]; + function isBlock(elm) { var display = window.getComputedStyle(elm).getPropertyValue('display'); return blockLike.includes(display) || display.substr(0, 6) === 'table-'; @@ -42,6 +43,7 @@ function getBlockParent(node) { * @return {Boolean} [description] */ function isInTextBlock(node, options) { + const { getRoleType } = axe.commons.aria; if (isBlock(node)) { // Ignore if the link is a block return false; @@ -93,7 +95,7 @@ function isInTextBlock(node, options) { return false; // Don't walk widgets, we're only interested in what's not in them. - } else if (isWidget(currNode)) { + } else if (getRoleType(currNode) === 'widget') { // Grab all the text from this element, but don't walk down it's children widgetText += currNode.textContent; return false; @@ -109,10 +111,4 @@ function isInTextBlock(node, options) { return parentText.length > widgetText.length; } -function isWidget(domNode) { - const { getRoleType, getRole } = axe.commons.aria; - const role = getRole(domNode); - return ['widget', 'composite'].includes(getRoleType(role)); -} - export default isInTextBlock; diff --git a/lib/rules/widget-not-inline-matches.js b/lib/rules/widget-not-inline-matches.js index b79000fa06..0b1cacd836 100644 --- a/lib/rules/widget-not-inline-matches.js +++ b/lib/rules/widget-not-inline-matches.js @@ -1,4 +1,4 @@ -import { getRole, getRoleType } from '../commons/aria'; +import { getRoleType } from '../commons/aria'; import { isFocusable, isInTextBlock } from '../commons/dom'; import svgNamespaceMatches from './svg-namespace-matches'; @@ -15,14 +15,7 @@ const matchesFns = [ ]; function isWidgetType(vNode) { - const role = getRole(vNode); - if ('option' === role) { - return false; - } else if (['combobox', 'listbox'].includes(role)) { - return true; - } - const roleType = getRoleType(role); - return roleType === 'widget'; + return getRoleType(vNode) === 'widget'; } function isNotAreaElement(vNode) { diff --git a/lib/standards/aria-roles.js b/lib/standards/aria-roles.js index adbaaccfed..428e786ea0 100644 --- a/lib/standards/aria-roles.js +++ b/lib/standards/aria-roles.js @@ -116,7 +116,7 @@ const ariaRoles = { nameFromContent: true }, combobox: { - type: 'composite', + type: 'widget', requiredAttrs: ['aria-expanded', 'aria-controls'], allowedAttrs: [ 'aria-owns', @@ -283,7 +283,7 @@ const ariaRoles = { superclassRole: ['section'] }, listbox: { - type: 'composite', + type: 'widget', requiredOwned: ['group', 'option'], allowedAttrs: [ 'aria-multiselectable', diff --git a/locales/_template.json b/locales/_template.json index c40306bf8a..a0b4e6a719 100644 --- a/locales/_template.json +++ b/locales/_template.json @@ -824,14 +824,14 @@ "fail": "${data} on tag disables zooming on mobile devices" }, "target-offset": { - "pass": "Target is sufficiently offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)", - "fail": "Target is insufficient offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)" + "pass": "Target has sufficiently offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)", + "fail": "Target has insufficient offset from its closest neighbor (${data.closestOffset}px should be at least ${data.minOffset}px)" }, "target-size": { - "pass": "Control has sufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSpacing}px by ${data.minSpacing}px)", + "pass": "Control has sufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)", "fail": { - "default": "Element has insufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSpacing}px by ${data.minSpacing}px)", - "obscured": "Element has insufficient size because it is obscured (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSpacing}px by ${data.minSpacing}px)" + "default": "Element has insufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)", + "obscured": "Element has insufficient size because it is obscured (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)" } }, "header-present": { diff --git a/test/commons/aria/get-role-type.js b/test/commons/aria/get-role-type.js index 21edca6193..c1aea70db4 100644 --- a/test/commons/aria/get-role-type.js +++ b/test/commons/aria/get-role-type.js @@ -1,15 +1,17 @@ -describe('aria.getRoleType', function() { +describe('aria.getRoleType', function () { 'use strict'; + var queryFixture = axe.testUtils.queryFixture; + var getRoleType = axe.commons.aria.getRoleType; - before(function() { + before(function () { axe._load({}); }); - afterEach(function() { + afterEach(function () { axe.reset(); }); - it('should return true if role is found in the lookup table', function() { + it('should return the type from the lookup table', function () { axe.configure({ standards: { ariaRoles: { @@ -19,10 +21,40 @@ describe('aria.getRoleType', function() { } } }); - assert.equal(axe.commons.aria.getRoleType('cats'), 'stuff'); + assert.equal(getRoleType('cats'), 'stuff'); }); - it('should return null if role is not found in the lookup table', function() { - assert.isNull(axe.commons.aria.getRoleType('cats')); + it('should return null if role is not found in the lookup table', function () { + assert.isNull(getRoleType('cats')); + }); + + it('returns the type from the role of a virtual node', function () { + var vNode = queryFixture(''); + axe.configure({ + standards: { + ariaRoles: { + cats: { + type: 'stuff' + } + } + } + }); + assert.equal(getRoleType(vNode), 'stuff'); + }); + + it('returns the type from the role of a DOM node', function () { + var domNode = queryFixture( + '' + ).actualNode; + axe.configure({ + standards: { + ariaRoles: { + cats: { + type: 'stuff' + } + } + } + }); + assert.equal(getRoleType(domNode), 'stuff'); }); }); diff --git a/test/rule-matches/widget-not-inline-matches.js b/test/rule-matches/widget-not-inline-matches.js index 0fbe259800..42815f495f 100644 --- a/test/rule-matches/widget-not-inline-matches.js +++ b/test/rule-matches/widget-not-inline-matches.js @@ -46,14 +46,6 @@ describe('widget-not-inline-matches', function () { assert.isFalse(rule.matches(node, vNode)); }); - it('returns false for a non-tabbable button (widgets)', function () { - var vNode = queryFixture( - '
' - ); - var node = vNode.actualNode; - assert.isFalse(rule.matches(node, vNode)); - }); - it('returns true for a listbox (component)', function () { var vNode = queryFixture( '
' From 9e7e9b6b24586da9f8cc2dd8968930c16c522409 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Fri, 16 Sep 2022 14:34:47 +0200 Subject: [PATCH 22/28] cleanup --- test/commons/aria/get-role-type.js | 32 +++++++----------------------- 1 file changed, 7 insertions(+), 25 deletions(-) diff --git a/test/commons/aria/get-role-type.js b/test/commons/aria/get-role-type.js index c1aea70db4..f7b83566bb 100644 --- a/test/commons/aria/get-role-type.js +++ b/test/commons/aria/get-role-type.js @@ -5,13 +5,6 @@ describe('aria.getRoleType', function () { before(function () { axe._load({}); - }); - - afterEach(function () { - axe.reset(); - }); - - it('should return the type from the lookup table', function () { axe.configure({ standards: { ariaRoles: { @@ -21,6 +14,13 @@ describe('aria.getRoleType', function () { } } }); + }); + + afterEach(function () { + axe.reset(); + }); + + it('should return the type from the lookup table', function () { assert.equal(getRoleType('cats'), 'stuff'); }); @@ -30,15 +30,6 @@ describe('aria.getRoleType', function () { it('returns the type from the role of a virtual node', function () { var vNode = queryFixture(''); - axe.configure({ - standards: { - ariaRoles: { - cats: { - type: 'stuff' - } - } - } - }); assert.equal(getRoleType(vNode), 'stuff'); }); @@ -46,15 +37,6 @@ describe('aria.getRoleType', function () { var domNode = queryFixture( '' ).actualNode; - axe.configure({ - standards: { - ariaRoles: { - cats: { - type: 'stuff' - } - } - } - }); assert.equal(getRoleType(domNode), 'stuff'); }); }); From 4fefa21bfd443d40da243914501b144f634c274e Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Tue, 20 Sep 2022 14:16:20 +0200 Subject: [PATCH 23/28] Handle fully obscuring elements --- lib/checks/mobile/target-offset-evaluate.js | 4 +- lib/checks/mobile/target-size-evaluate.js | 49 ++++-- lib/checks/mobile/target-size.json | 7 +- lib/commons/math/get-offset.js | 2 +- test/checks/mobile/target-offset.js | 15 ++ test/checks/mobile/target-size.js | 163 +++++++++++++++----- 6 files changed, 189 insertions(+), 51 deletions(-) diff --git a/lib/checks/mobile/target-offset-evaluate.js b/lib/checks/mobile/target-offset-evaluate.js index ac65277180..e90c080640 100644 --- a/lib/checks/mobile/target-offset-evaluate.js +++ b/lib/checks/mobile/target-offset-evaluate.js @@ -1,4 +1,4 @@ -import { findNearbyElms } from '../../commons/dom'; +import { findNearbyElms, isFocusable } from '../../commons/dom'; import { getRoleType } from '../../commons/aria'; import { getOffset } from '../../commons/math'; @@ -9,7 +9,7 @@ export default function targetOffsetEvaluate(node, options, vNode) { const closeNeighbors = []; let closestOffset = minOffset; for (const vNeighbor of findNearbyElms(vNode, minOffset)) { - if (getRoleType(vNeighbor) !== 'widget') { + if (getRoleType(vNeighbor) !== 'widget' || !isFocusable(vNeighbor)) { continue; } const offset = roundToSingleDecimal(getOffset(vNode, vNeighbor)); diff --git a/lib/checks/mobile/target-size-evaluate.js b/lib/checks/mobile/target-size-evaluate.js index 1a760d228e..e83eccc358 100644 --- a/lib/checks/mobile/target-size-evaluate.js +++ b/lib/checks/mobile/target-size-evaluate.js @@ -1,4 +1,4 @@ -import { findNearbyElms } from '../../commons/dom'; +import { findNearbyElms, isFocusable } from '../../commons/dom'; import { getRoleType } from '../../commons/aria'; import { splitRects, hasVisualOverlap } from '../../commons/math'; @@ -10,32 +10,44 @@ const roundingMargin = 0.05; */ export default function targetSize(node, options, vNode) { const minSize = options?.minSize || 24; + const nodeRect = vNode.boundingClientRect; const hasMinimumSize = ({ width, height }) => { return ( width + roundingMargin >= minSize && height + roundingMargin >= minSize ); }; - const nodeRect = vNode.boundingClientRect; + const obscuringElms = []; + for (const vNeighbor of findNearbyElms(vNode)) { + if ( + !hasVisualOverlap(vNode, vNeighbor) || + getCssPointerEvents(vNeighbor) === 'none' + ) { + continue; + } + if (isEnclosedRect(vNode, vNeighbor)) { + this.data({ messageKey: 'obscured' }); + return true; + } + obscuringElms.push(vNeighbor); + } + if (!hasMinimumSize(nodeRect)) { this.data({ minSize, ...toDecimalSize(nodeRect) }); return false; } - // Handle overlapping elements; - const nearbyElms = findNearbyElms(vNode); - const obscuringElms = nearbyElms.filter( - vNeighbor => - getRoleType(vNeighbor) === 'widget' && hasVisualOverlap(vNode, vNeighbor) + const obscuredWidgets = obscuringElms.filter( + vNeighbor => getRoleType(vNeighbor) === 'widget' && isFocusable(vNeighbor) ); - if (obscuringElms.length === 0) { + if (obscuredWidgets.length === 0) { this.data({ minSize, ...toDecimalSize(nodeRect) }); return true; // No obscuring elements; pass } // Find areas of the target that are not obscured - const obscuringRects = obscuringElms.map( + const obscuringRects = obscuredWidgets.map( ({ boundingClientRect: rect }) => rect ); const unobscuredRects = splitRects(nodeRect, obscuringRects); @@ -55,11 +67,11 @@ export default function targetSize(node, options, vNode) { if (!hasMinimumSize(largestInnerRect)) { this.data({ - messageKey: 'obscured', + messageKey: 'partiallyObscured', minSize, ...toDecimalSize(largestInnerRect) }); - this.relatedNodes(obscuringElms.map(({ actualNode }) => actualNode)); + this.relatedNodes(obscuredWidgets.map(({ actualNode }) => actualNode)); // Element is (partially?) obscured, with insufficient space return false; } @@ -68,6 +80,21 @@ export default function targetSize(node, options, vNode) { return true; } +function isEnclosedRect(vNodeA, vNodeB) { + const rectA = vNodeA.boundingClientRect; + const rectB = vNodeB.boundingClientRect; + return ( + rectA.top >= rectB.top && + rectA.left >= rectB.left && + rectA.bottom <= rectB.bottom && + rectA.right <= rectB.right + ); +} + +function getCssPointerEvents(vNode) { + return vNode.getComputedStylePropertyValue('pointer-events'); +} + function toDecimalSize(rect) { return { width: Math.round(rect.width * 10) / 10, diff --git a/lib/checks/mobile/target-size.json b/lib/checks/mobile/target-size.json index 12ed7e037a..60d93458aa 100644 --- a/lib/checks/mobile/target-size.json +++ b/lib/checks/mobile/target-size.json @@ -7,10 +7,13 @@ "metadata": { "impact": "serious", "messages": { - "pass": "Control has sufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)", + "pass": { + "default": "Control has sufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)", + "obscured": "Control is ignored because it is fully obscured and thus not clickable" + }, "fail": { "default": "Element has insufficient size (${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)", - "obscured": "Element has insufficient size because it is obscured (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)" + "partiallyObscured": "Element has insufficient size because it is partially obscured (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px)" } } } diff --git a/lib/commons/math/get-offset.js b/lib/commons/math/get-offset.js index 27c2175d0d..aff62725ab 100644 --- a/lib/commons/math/get-offset.js +++ b/lib/commons/math/get-offset.js @@ -35,7 +35,7 @@ function getFarthestPoint(rectA, rectB) { const endDistance = Math.abs(centerB - rectA[end]); if (startDistance >= endDistance) { farthestPoint[axis] = rectA[start]; // left | top - } else if (startDistance < endDistance) { + } else { farthestPoint[axis] = rectA[end]; // right | bottom } }); diff --git a/test/checks/mobile/target-offset.js b/test/checks/mobile/target-offset.js index a9440eabcd..4552537358 100644 --- a/test/checks/mobile/target-offset.js +++ b/test/checks/mobile/target-offset.js @@ -66,6 +66,21 @@ describe('target-offset tests', function () { assert.closeTo(checkContext._data.closestOffset, 24, 0.2); }); + it('ignores non-tabbable widget elements as neighbors', function () { + var checkArgs = checkSetup( + 'x' + + '' + ); + + assert.isTrue(check.evaluate.apply(checkContext, checkArgs)); + assert.equal(checkContext._data.minOffset, 24); + assert.closeTo(checkContext._data.closestOffset, 24, 0.2); + }); + it('sets all elements that are too close as related nodes', function () { var checkArgs = checkSetup( 'x' + - '' - ); - assert.isTrue(check.evaluate.apply(checkContext, checkArgs)); - assert.deepEqual(checkContext._data, { - minSize: 24, - width: 30, - height: 30 + describe('when fully obscured', function () { + it('returns true, regardless of size', function () { + var checkArgs = checkSetup( + 'x' + + '
x
' + ); + assert.isTrue(check.evaluate.apply(checkContext, checkArgs)); + assert.deepEqual(checkContext._data, { messageKey: 'obscured' }); + }); + + it('returns true when obscured by another focusable widget', function () { + var checkArgs = checkSetup( + 'x' + + 'x' + ); + assert.isTrue(check.evaluate.apply(checkContext, checkArgs)); + assert.deepEqual(checkContext._data, { + messageKey: 'obscured', + minSize: 24, + width: 20, + height: 20 + }); + }); + + it('ignores obscuring element has pointer-events:none', function () { + var checkArgs = checkSetup( + 'x' + + 'x' + ); + assert.isFalse(check.evaluate.apply(checkContext, checkArgs)); + assert.deepEqual(checkContext._data, { + minSize: 24, + width: 20, + height: 20 + }); }); }); - it('returns false for obscured targets with insufficient space', function () { - var checkArgs = checkSetup( - '' + - '' + - '' - ); - assert.isFalse(check.evaluate.apply(checkContext, checkArgs)); - assert.deepEqual(checkContext._data, { - messageKey: 'obscured', - minSize: 24, - width: 20, - height: 30 + describe('when partially obscured', function () { + it('returns true for focusable non-widgets', function () { + var checkArgs = checkSetup( + '' + + '' + + 'x' + ); + assert.isTrue(check.evaluate.apply(checkContext, checkArgs)); + assert.deepEqual(checkContext._data, { + minSize: 24, + width: 30, + height: 30 + }); + }); + + it('returns true for non-focusable widgets', function () { + var checkArgs = checkSetup( + '' + + '' + + '' + ); + assert.isTrue(check.evaluate.apply(checkContext, checkArgs)); + assert.deepEqual(checkContext._data, { + minSize: 24, + width: 30, + height: 30 + }); + }); + + describe('by a focusable widget', function () { + it('returns true for obscured targets with insufficient space', function () { + var checkArgs = checkSetup( + '' + + '' + ); + assert.isTrue(check.evaluate.apply(checkContext, checkArgs)); + assert.deepEqual(checkContext._data, { + minSize: 24, + width: 30, + height: 30 + }); + }); + + it('returns false for obscured targets with insufficient space', function () { + var checkArgs = checkSetup( + '' + + '' + + '' + ); + assert.isFalse(check.evaluate.apply(checkContext, checkArgs)); + assert.deepEqual(checkContext._data, { + messageKey: 'partiallyObscured', + minSize: 24, + width: 20, + height: 30 + }); + }); }); }); - (shadowSupport ? it : xit)('works across shadow boundaries', function () { + it('works across shadow boundaries', function () { var checkArgs = shadowCheckSetup( '' + '' + - '' + 'x' + - '' + '' + - '' ); @@ -140,6 +150,7 @@ describe('target-size tests', function () { width: 30, height: 30 }); + assert.deepEqual(elmIds(checkContext._relatedNodes), ['#obscurer']); }); it('returns false for obscured targets with insufficient space', function () { @@ -147,10 +158,10 @@ describe('target-size tests', function () { '' + - '' + - '' ); @@ -161,6 +172,10 @@ describe('target-size tests', function () { width: 20, height: 30 }); + assert.deepEqual(elmIds(checkContext._relatedNodes), [ + '#obscurer1', + '#obscurer2' + ]); }); }); }); @@ -168,10 +183,10 @@ describe('target-size tests', function () { it('works across shadow boundaries', function () { var checkArgs = shadowCheckSetup( '' + - '' + - '', '