fix(avoid-inline-spacing): add spacing threshold (#3533)

* fix(avoid-inline-spacing): add spacing threshold

* Fix integration tests

* Improve options for inline-style-property

* IE can't count

* Add is-visible-matches method

* nudge circle

* Tweak failure messages

* Lint things

* Apply suggestions from code review

Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>

* Address feedback

* Move isMultiline to commons.dom

* pull tests that do not pass in IE

Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>

doc/check-options.md

@@ -32,6 +32,7 @@
   - [avoid-inline-spacing](#avoid-inline-spacing)
   - [scope-value](#scope-value)
   - [region](#region)
+  - [inline-style-property](#inline-style-property)
 
 ## How Checks Work
 
@@ -491,3 +492,23 @@ h6:not([role]),
 | Option          | Default                                        | Description                                                                 |
 | --------------- | :--------------------------------------------- | :-------------------------------------------------------------------------- |
 | `regionMatcher` | <pre lang=css>dialog, [role=dialog], svg</pre> | A matcher object or CSS selector to allow elements to be treated as regions |
+
+### inline-style-property-evaluate
+
+This evaluate method is used in the following checks. Default vary between checks
+
+- important-letter-spacing
+- important-word-spacing
+- important-line-height
+
+| Option           | Description                                                                   |
+| ---------------- | :---------------------------------------------------------------------------- |
+| `cssProperty`    | Which property to check the value of, for example letter-spacing or font-size |
+| `absoluteValues` | Whether or not to calculate value in pixels (true) or in em (false)           |
+| `noImportant`    | While false, the check returns `true` except if !important is used            |
+| `multiLineOnly`  | If true,                                                                      |
+| `minValue`       | Returns `false` when the value is less than `minValue`                        |
+| `maxValue`       | Returns `false` when the value is more than `maxValue`                        |
+| `normalValue`    | The value to use when `normal` is set, defaults to `0`                        |
+
+If `minValue` and `maxValue` are both undefined, the check returns `false` if the property is used with !important. If done along with `noImportant: true`, the check returns false if the property is set at all in the style attribute.

lib/checks/shared/important-letter-spacing.json

@@ -0,0 +1,15 @@
+{
+  "id": "important-letter-spacing",
+  "evaluate": "inline-style-property-evaluate",
+  "options": {
+    "cssProperty": "letter-spacing",
+    "minValue": 0.12
+  },
+  "metadata": {
+    "impact": "serious",
+    "messages": {
+      "pass": "Letter-spacing in the style attribute is not set to !important, or meets the minimum",
+      "fail": "letter-spacing in the style attribute must not use !important, or be at ${data.minValue}em (current ${data.value}em)"
+    }
+  }
+}

lib/checks/shared/important-line-height.json

@@ -0,0 +1,17 @@
+{
+  "id": "important-line-height",
+  "evaluate": "inline-style-property-evaluate",
+  "options": {
+    "multiLineOnly": true,
+    "cssProperty": "line-height",
+    "minValue": 1.5,
+    "normalValue": 1
+  },
+  "metadata": {
+    "impact": "serious",
+    "messages": {
+      "pass": "line-height in the style attribute is not set to !important, or meets the minimum",
+      "fail": "line-height in the style attribute must not use !important, or be at ${data.minValue}em (current ${data.value}em)"
+    }
+  }
+}

lib/checks/shared/important-word-spacing.json

@@ -0,0 +1,15 @@
+{
+  "id": "important-word-spacing",
+  "evaluate": "inline-style-property-evaluate",
+  "options": {
+    "cssProperty": "word-spacing",
+    "minValue": 0.16
+  },
+  "metadata": {
+    "impact": "serious",
+    "messages": {
+      "pass": "word-spacing in the style attribute is not set to !important, or meets the minimum",
+      "fail": "word-spacing in the style attribute must not use !important, or be at ${data.minValue}em (current ${data.value}em)"
+    }
+  }
+}

lib/checks/shared/inline-style-property-evaluate.js

@@ -0,0 +1,80 @@
+import { isMultiline } from '../../commons/dom';
+
+/**
+ * Check if a CSS property, !important or not is within an allowed range
+ */
+export default function inlineStyleProperty(node, options) {
+  const {
+    cssProperty,
+    absoluteValues,
+    minValue,
+    maxValue,
+    normalValue = 0,
+    noImportant,
+    multiLineOnly
+  } = options;
+  if (
+    (!noImportant &&
+      node.style.getPropertyPriority(cssProperty) !== `important`) ||
+    (multiLineOnly && !isMultiline(node))
+  ) {
+    return true;
+  }
+
+  const data = {};
+  if (typeof minValue === 'number') {
+    data.minValue = minValue;
+  }
+  if (typeof maxValue === 'number') {
+    data.maxValue = maxValue;
+  }
+
+  // These do not set the actual value to important, instead they
+  // say that it is important to use the inherited / root value.
+  // The actual value can still be modified
+  const declaredPropValue = node.style.getPropertyValue(cssProperty);
+  if (
+    ['inherit', 'unset', 'revert', 'revert-layer'].includes(declaredPropValue)
+  ) {
+    this.data({ value: declaredPropValue, ...data });
+    return true;
+  }
+
+  const value = getNumberValue(node, {
+    absoluteValues,
+    cssProperty,
+    normalValue
+  });
+  this.data({ value, ...data });
+  if (typeof value !== 'number') {
+    return undefined; // Renderer did something it shouldn't
+  }
+
+  if (
+    (typeof minValue !== 'number' || value >= minValue) &&
+    (typeof maxValue !== 'number' || value <= maxValue)
+  ) {
+    return true;
+  }
+  return false;
+}
+
+function getNumberValue(domNode, { cssProperty, absoluteValues, normalValue }) {
+  const computedStyle = window.getComputedStyle(domNode);
+  const cssPropValue = computedStyle.getPropertyValue(cssProperty);
+  if (cssPropValue === 'normal') {
+    return normalValue;
+  }
+  const parsedValue = parseFloat(cssPropValue);
+  if (absoluteValues) {
+    return parsedValue;
+  }
+
+  const fontSize = parseFloat(computedStyle.getPropertyValue('font-size'));
+  // Make the value relative to the font-size
+  const value = Math.round((parsedValue / fontSize) * 100) / 100;
+  if (isNaN(value)) {
+    return cssPropValue; // Something went wrong, return the string instead
+  }
+  return value;
+}

lib/commons/dom/index.js

@@ -26,6 +26,7 @@ export { default as isHiddenWithCSS } from './is-hidden-with-css';
 export { default as isHTML5 } from './is-html5';
 export { default as isInTextBlock } from './is-in-text-block';
 export { default as isModalOpen } from './is-modal-open';
+export { default as isMultiline } from './is-multiline';
 export { default as isNativelyFocusable } from './is-natively-focusable';
 export { default as isNode } from './is-node';
 export { default as isOffscreen } from './is-offscreen';

lib/commons/dom/is-multiline.js

@@ -0,0 +1,28 @@
+/**
+ * Returns true if content has client rects that have no vertical overlap.
+ * I.e. they are rendered on different "lines".
+ * @param {Element} domNode
+ * @param {number} margin (default: 2)
+ * @returns {number}
+ */
+export default function isMultiline(domNode, margin = 2) {
+  const range = domNode.ownerDocument.createRange();
+  range.setStart(domNode, 0);
+  range.setEnd(domNode, domNode.childNodes.length);
+  let lastLineEnd = 0;
+  let lineCount = 0;
+  for (const rect of range.getClientRects()) {
+    if (rect.height <= margin) {
+      continue;
+    }
+    if (lastLineEnd > rect.top + margin) {
+      lastLineEnd = Math.max(lastLineEnd, rect.bottom);
+    } else if (lineCount === 0) {
+      lastLineEnd = rect.bottom;
+      lineCount++;
+    } else {
+      return true;
+    }
+  }
+  return false;
+}

lib/rules/avoid-inline-spacing.json

@@ -1,13 +1,18 @@
 {
   "id": "avoid-inline-spacing",
   "selector": "[style]",
+  "matches": "is-visible-matches",
   "tags": ["cat.structure", "wcag21aa", "wcag1412", "ACT"],
   "actIds": ["24afc2", "9e45ec", "78fd32"],
   "metadata": {
     "description": "Ensure that text spacing set through style attributes can be adjusted with custom stylesheets",
     "help": "Inline text spacing must be adjustable with custom stylesheets"
   },
-  "all": ["avoid-inline-spacing"],
+  "all": [
+    "important-letter-spacing",
+    "important-word-spacing",
+    "important-line-height"
+  ],
   "any": [],
   "none": []
 }

lib/rules/is-visible-matches.js

@@ -0,0 +1,5 @@
+import { isVisible } from '../commons/dom';
+
+export default function hasVisibleTextMatches(node) {
+  return isVisible(node, false);
+}

test/act-mapping/letter-spacing-not-important.json

@@ -0,0 +1,5 @@
+{
+  "id": "24afc2",
+  "title": "Letter spacing in style attributes is not !important",
+  "axeRules": ["avoid-inline-spacing"]
+}

test/act-mapping/line-height-not-important.json

@@ -0,0 +1,5 @@
+{
+  "id": "78fd32",
+  "title": "Line height in style attributes is not !important",
+  "axeRules": ["avoid-inline-spacing"]
+}

test/act-mapping/word-spacing-not-important.json

@@ -0,0 +1,5 @@
+{
+  "id": "9e45ec",
+  "title": "Word spacing in style attributes is not !important",
+  "axeRules": ["avoid-inline-spacing"]
+}