From dc732b01c9e8bc0241563a6fa625c4a80bdbf71b Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Fri, 21 Jun 2024 16:14:57 +0200 Subject: [PATCH 1/4] feat(new-rule): summary elements must have an accessible name --- doc/rule-descriptions.md | 1 + lib/rules/summary-interactive-matches.js | 44 +++++ lib/rules/summary-name.json | 29 ++++ locales/_template.json | 4 + .../rules/summary-name/summary-name.html | 103 +++++++++++ .../rules/summary-name/summary-name.json | 23 +++ .../integration/virtual-rules/summary-name.js | 103 +++++++++++ test/rule-matches/no-naming-method-matches.js | 6 + .../summary-interactive-matches.js | 160 ++++++++++++++++++ test/testutils.js | 17 ++ 10 files changed, 490 insertions(+) create mode 100644 lib/rules/summary-interactive-matches.js create mode 100644 lib/rules/summary-name.json create mode 100644 test/integration/rules/summary-name/summary-name.html create mode 100644 test/integration/rules/summary-name/summary-name.json create mode 100644 test/integration/virtual-rules/summary-name.js create mode 100644 test/rule-matches/summary-interactive-matches.js diff --git a/doc/rule-descriptions.md b/doc/rule-descriptions.md index 734d73b36b..319108744c 100644 --- a/doc/rule-descriptions.md +++ b/doc/rule-descriptions.md @@ -69,6 +69,7 @@ | [scrollable-region-focusable](https://dequeuniversity.com/rules/axe/4.9/scrollable-region-focusable?application=RuleDescription) | Ensure elements that have scrollable content are accessible by keyboard | Serious | cat.keyboard, wcag2a, wcag211, wcag213, TTv5, TT4.a, EN-301-549, EN-9.2.1.1, EN-9.2.1.3 | failure | [0ssw9k](https://act-rules.github.io/rules/0ssw9k) | | [select-name](https://dequeuniversity.com/rules/axe/4.9/select-name?application=RuleDescription) | Ensures select element has an accessible name | Critical | cat.forms, wcag2a, wcag412, section508, section508.22.n, TTv5, TT5.c, EN-301-549, EN-9.4.1.2, ACT | failure, needs review | [e086e5](https://act-rules.github.io/rules/e086e5) | | [server-side-image-map](https://dequeuniversity.com/rules/axe/4.9/server-side-image-map?application=RuleDescription) | Ensures that server-side image maps are not used | Minor | cat.text-alternatives, wcag2a, wcag211, section508, section508.22.f, TTv5, TT4.a, EN-301-549, EN-9.2.1.1 | needs review | | +| [summary-name](https://dequeuniversity.com/rules/axe/4.9/summary-name?application=RuleDescription) | Ensures summary elements have discernible text | Serious | cat.name-role-value, wcag2a, wcag412, section508, section508.22.a, TTv5, TT6.a, EN-301-549, EN-9.4.1.2 | failure, needs review | | | [svg-img-alt](https://dequeuniversity.com/rules/axe/4.9/svg-img-alt?application=RuleDescription) | Ensures <svg> elements with an img, graphics-document or graphics-symbol role have an accessible text | Serious | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a, TTv5, TT7.a, EN-301-549, EN-9.1.1.1, ACT | failure, needs review | [7d6734](https://act-rules.github.io/rules/7d6734) | | [td-headers-attr](https://dequeuniversity.com/rules/axe/4.9/td-headers-attr?application=RuleDescription) | Ensure that each cell in a table that uses the headers attribute refers only to other cells in that table | Serious | cat.tables, wcag2a, wcag131, section508, section508.22.g, TTv5, TT14.b, EN-301-549, EN-9.1.3.1 | failure, needs review | [a25f45](https://act-rules.github.io/rules/a25f45) | | [th-has-data-cells](https://dequeuniversity.com/rules/axe/4.9/th-has-data-cells?application=RuleDescription) | Ensure that <th> elements and elements with role=columnheader/rowheader have data cells they describe | Serious | cat.tables, wcag2a, wcag131, section508, section508.22.g, TTv5, TT14.b, EN-301-549, EN-9.1.3.1 | failure, needs review | [d0f69e](https://act-rules.github.io/rules/d0f69e) | diff --git a/lib/rules/summary-interactive-matches.js b/lib/rules/summary-interactive-matches.js new file mode 100644 index 0000000000..a3a665e198 --- /dev/null +++ b/lib/rules/summary-interactive-matches.js @@ -0,0 +1,44 @@ +import { hasContentVirtual } from '../commons/dom'; + +export default function summaryIsInteractiveMatches(_, virtualNode) { + // Summary only interactive if its real DOM parent is a details element + const parent = virtualNode.parent; + if ( + parent.props.nodeName !== 'details' || + virtualNode.shadowId !== parent.shadowId + ) { + return false; + } + if (!hasDetailsContent(parent)) { + return false; + } + // Only the first summary element is interactive + const firstSummary = parent.children.find( + child => child.props.nodeName === 'summary' + ); + if (firstSummary !== virtualNode) { + return false; + } + return true; +} + +function hasDetailsContent(vDetails) { + let summary = false; + for (const vNode of vDetails.children) { + const { nodeType, nodeValue } = vNode.props; + // Skip the first summary elm + if (!summary && vNode.props.nodeName === 'summary') { + summary = true; + continue; + } + // True for elements with content + if (nodeType === document.ELEMENT_NODE && hasContentVirtual(vNode)) { + return true; + } + // True for text nodes + if (nodeType === document.TEXT_NODE && nodeValue.trim()) { + return true; + } + } + return false; +} diff --git a/lib/rules/summary-name.json b/lib/rules/summary-name.json new file mode 100644 index 0000000000..349d9c2977 --- /dev/null +++ b/lib/rules/summary-name.json @@ -0,0 +1,29 @@ +{ + "id": "summary-name", + "impact": "serious", + "selector": "summary", + "matches": "summary-interactive-matches", + "tags": [ + "cat.name-role-value", + "wcag2a", + "wcag412", + "section508", + "section508.22.a", + "TTv5", + "TT6.a", + "EN-301-549", + "EN-9.4.1.2" + ], + "metadata": { + "description": "Ensures summary elements have discernible text", + "help": "Summary elements must have discernible text" + }, + "all": [], + "any": [ + "has-visible-text", + "aria-label", + "aria-labelledby", + "non-empty-title" + ], + "none": [] +} diff --git a/locales/_template.json b/locales/_template.json index 63b6f1d3b6..52ad9a16e1 100644 --- a/locales/_template.json +++ b/locales/_template.json @@ -373,6 +373,10 @@ "description": "Ensure all skip links have a focusable target", "help": "The skip-link target should exist and be focusable" }, + "summary-name": { + "description": "Ensures summary elements have discernible text", + "help": "Summary elements must have discernible text" + }, "svg-img-alt": { "description": "Ensures elements with an img, graphics-document or graphics-symbol role have an accessible text", "help": " elements with an img role must have an alternative text" diff --git a/test/integration/rules/summary-name/summary-name.html b/test/integration/rules/summary-name/summary-name.html new file mode 100644 index 0000000000..ca5afbe131 --- /dev/null +++ b/test/integration/rules/summary-name/summary-name.html @@ -0,0 +1,103 @@ +
+ + Hello world +
+ +
+ name + Hello world +
+ +
+ + Hello world +
+ +
+ + Hello world +
+ +
+ + Hello world +
+ +
+ + Hello world +
+ +
+ + Hello world +
+
summary label
+
+ +
+ Name + Hello world +
+ +
+ + Hello world +
+ +
+ + Conflict resolution gets this to be ignored +
+ +
+ + Conflict resolution gets this to be ignored +
+ +
+ + Conflict resolution gets this to be ignored +
+ + + +
+ + Not a valid method for giving a name +
+ +
+ + Not a valid method for giving a name +
+ + + + + +
+ +
+ +
+ +
+ + + + + +
diff --git a/test/integration/rules/summary-name/summary-name.json b/test/integration/rules/summary-name/summary-name.json new file mode 100644 index 0000000000..68a4f7721e --- /dev/null +++ b/test/integration/rules/summary-name/summary-name.json @@ -0,0 +1,23 @@ +{ + "description": "summary-name test", + "rule": "summary-name", + "violations": [ + ["#empty-fail"], + ["#aria-label-fail"], + ["#aria-labelledby-fail"], + ["#aria-labelledby-empty-fail"], + ["#presentation-role-fail"], + ["#none-role-fail"], + ["#heading-role-fail"], + ["#value-attr-fail"], + ["#alt-attr-fail"], + ["#label-elm-fail"] + ], + "passes": [ + ["#text-pass"], + ["#aria-label-pass"], + ["#aria-labelledby-pass"], + ["#combo-pass"], + ["#title-pass"] + ] +} diff --git a/test/integration/virtual-rules/summary-name.js b/test/integration/virtual-rules/summary-name.js new file mode 100644 index 0000000000..3e27ac097b --- /dev/null +++ b/test/integration/virtual-rules/summary-name.js @@ -0,0 +1,103 @@ +describe('summary-name virtual-rule', () => { + const { appendSerialChild } = axe.testUtils; + + let vDetails; + beforeEach(() => { + vDetails = new axe.SerialVirtualNode({ + nodeName: 'details', + attributes: {} + }); + appendSerialChild(vDetails, { nodeName: '#text', nodeValue: 'text' }); + }); + + it('fails without children', () => { + const vSummary = new axe.SerialVirtualNode({ + nodeName: 'summary', + attributes: {} + }); + vSummary.children = []; + appendSerialChild(vDetails, vSummary); + const results = axe.runVirtualRule('summary-name', vSummary); + console.log(results); + assert.lengthOf(results.passes, 0); + assert.lengthOf(results.violations, 1); + assert.lengthOf(results.incomplete, 0); + }); + + it('passes with text content', () => { + const vSummary = new axe.SerialVirtualNode({ + nodeName: 'summary', + attributes: {} + }); + appendSerialChild(vSummary, { nodeName: '#text', nodeValue: 'text' }); + appendSerialChild(vDetails, vSummary); + + const results = axe.runVirtualRule('summary-name', vSummary); + assert.lengthOf(results.passes, 1); + assert.lengthOf(results.violations, 0); + assert.lengthOf(results.incomplete, 0); + }); + + it('is inapplicable without siblings to summary', () => { + const vSummary = new axe.SerialVirtualNode({ + nodeName: 'summary', + attributes: {} + }); + appendSerialChild(vSummary, { nodeName: '#text', nodeValue: 'text' }); + vSummary.parent = vDetails; + vDetails.children = [vSummary]; + + const results = axe.runVirtualRule('summary-name', vSummary); + assert.lengthOf(results.passes, 0); + assert.lengthOf(results.violations, 0); + assert.lengthOf(results.incomplete, 0); + assert.lengthOf(results.inapplicable, 1); + }); + + it('passes with aria-label', () => { + const vSummary = new axe.SerialVirtualNode({ + nodeName: 'summary', + attributes: { 'aria-label': 'foobar' } + }); + appendSerialChild(vDetails, vSummary); + const results = axe.runVirtualRule('summary-name', vSummary); + assert.lengthOf(results.passes, 1); + assert.lengthOf(results.violations, 0); + assert.lengthOf(results.incomplete, 0); + }); + + it('passes with title', () => { + const vSummary = new axe.SerialVirtualNode({ + nodeName: 'summary', + attributes: { title: 'foobar' } + }); + appendSerialChild(vDetails, vSummary); + const results = axe.runVirtualRule('summary-name', vSummary); + assert.lengthOf(results.passes, 1); + assert.lengthOf(results.violations, 0); + assert.lengthOf(results.incomplete, 0); + }); + + it('incompletes with aria-labelledby', () => { + const vSummary = new axe.SerialVirtualNode({ + nodeName: 'summary', + attributes: { 'aria-labelledby': 'foobar' } + }); + appendSerialChild(vDetails, vSummary); + const results = axe.runVirtualRule('summary-name', vSummary); + assert.lengthOf(results.passes, 0); + assert.lengthOf(results.violations, 0); + assert.lengthOf(results.incomplete, 1); + }); + + it('throws without a parent', () => { + const vSummary = new axe.SerialVirtualNode({ + nodeName: 'summary', + attributes: { 'aria-labelledby': 'foobar' } + }); + vSummary.children = []; + assert.throws(() => { + axe.runVirtualRule('summary-name', vSummary); + }); + }); +}); diff --git a/test/rule-matches/no-naming-method-matches.js b/test/rule-matches/no-naming-method-matches.js index da042c5ba1..9fcee1c563 100644 --- a/test/rule-matches/no-naming-method-matches.js +++ b/test/rule-matches/no-naming-method-matches.js @@ -51,6 +51,12 @@ describe('no-naming-method-matches', function () { assert.isFalse(actual); }); + it('returns false when node is SUMMARY', function () { + const vNode = queryFixture(''); + const actual = rule.matches(null, vNode); + assert.isFalse(actual); + }); + it('returns false for INPUT of type `BUTTON`, `SUBMIT` or `RESET`', function () { ['button', 'submit', 'reset'].forEach(function (type) { const vNode = queryFixture( diff --git a/test/rule-matches/summary-interactive-matches.js b/test/rule-matches/summary-interactive-matches.js new file mode 100644 index 0000000000..5fbc2b4cd9 --- /dev/null +++ b/test/rule-matches/summary-interactive-matches.js @@ -0,0 +1,160 @@ +describe('summary-interactive-matches', () => { + const rule = axe.utils.getRule('summary-name'); + const { queryFixture, queryShadowFixture, html } = axe.testUtils; + + it('is true for an interactive summary', () => { + const vNode = queryFixture(html` +
+ Summary + text +
+ `); + assert.isTrue(rule.matches(null, vNode)); + }); + + it('is false for summary without a details parent', () => { + const vNode = queryFixture(html` + Summary + text + `); + assert.isFalse(rule.matches(null, vNode)); + }); + + it('is false for summary with a details ancestor', () => { + const vNode = queryFixture(html` +
+
+ Summary + text +
+
+ `); + assert.isFalse(rule.matches(null, vNode)); + }); + + it('is false for a non-first summary', () => { + const vNode = queryFixture(html` +
+ Summary + Summary + text +
+ `); + assert.isFalse(rule.matches(null, vNode)); + }); + + it('is false for s details parent in a different DOM tree', () => { + const vNode = queryShadowFixture( + html` +
+
+
+ `, + html` Hello World ` + ); + assert.isFalse(rule.matches(null, vNode)); + }); + + it('is true even if summary has role=none', () => { + const vNode = queryFixture(html` +
+ Summary + text +
+ `); + assert.isTrue(rule.matches(null, vNode)); + }); + + it('is true the element has a widget role', () => { + const vNode = queryFixture(html` +
+ Summary + text +
+ `); + assert.isTrue(rule.matches(null, vNode)); + }); + + it('is true the element has a non-interactive role', () => { + const vNode = queryFixture(html` +
+ Summary + text +
+ `); + assert.isTrue(rule.matches(null, vNode)); + }); + + it('is true even if summary has tabindex=-1', () => { + const vNode = queryFixture(html` +
+ Summary + text +
+ `); + assert.isTrue(rule.matches(null, vNode)); + }); + + describe('details has no content', () => { + it('is false if summary is the only child', () => { + const vNode = queryFixture(html` +
+ Summary +
+ `); + assert.isFalse(rule.matches(null, vNode)); + }); + + it('is false with a comment sibling', () => { + const vNode = queryFixture(html` +
+ Summary + +
+ `); + assert.isFalse(rule.matches(null, vNode)); + }); + + it('is false with an empty sibling elm', () => { + const vNode = queryFixture(html` +
+ Summary +
+
+ `); + assert.isFalse(rule.matches(null, vNode)); + }); + + it('is false with a hidden text element', () => { + const vNode = queryFixture(html` +
+ Summary + +
+ `); + assert.isFalse(rule.matches(null, vNode)); + }); + + it('is true if the content is a graphic', () => { + const vNode = queryFixture(html` +
+ Summary +
+
+ `); + assert.isTrue(rule.matches(null, vNode)); + }); + + it('is true if there are is a second summary with content', () => { + const vNode = queryFixture(html` +
+ Summary + actual content +
+ `); + assert.isTrue(rule.matches(null, vNode)); + }); + }); +}); diff --git a/test/testutils.js b/test/testutils.js index a9168b10fe..222ee23c4e 100644 --- a/test/testutils.js +++ b/test/testutils.js @@ -708,4 +708,21 @@ var commons; fixtureNode.appendChild(child.cloneNode(true)); } } + + /** + * Appends a child node to a parent node in a serial virtual tree. + * If the child is not an instance of `axe.SerialVirtualNode`, it will be converted to one. + * @param {axe.SerialVirtualNode} parent - The parent node to append the child to. + * @param {axe.SerialVirtualNode|any} child - The child node to append. + * @returns {axe.SerialVirtualNode} The appended child node. + */ + testUtils.appendSerialChild = function appendSerialChild(parent, child) { + if (child instanceof axe.SerialVirtualNode === false) { + child = new axe.SerialVirtualNode(child); + } + child.parent = parent; + parent.children ??= []; + parent.children.push(child); + return child; + }; })(); From 20ebabb5a0efa3a06592c14290d47980f95b2bf8 Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Mon, 24 Jun 2024 13:59:43 +0200 Subject: [PATCH 2/4] Fix failing tests --- lib/rules/summary-interactive-matches.js | 4 ++-- test/integration/full/all-rules/all-rules.html | 4 ++++ .../full/isolated-env/isolated-env.html | 4 ++++ test/integration/virtual-rules/_testUtils.js | 18 ++++++++++++++++++ test/integration/virtual-rules/summary-name.js | 4 ++-- test/testutils.js | 17 ----------------- 6 files changed, 30 insertions(+), 21 deletions(-) create mode 100644 test/integration/virtual-rules/_testUtils.js diff --git a/lib/rules/summary-interactive-matches.js b/lib/rules/summary-interactive-matches.js index a3a665e198..f8a4252238 100644 --- a/lib/rules/summary-interactive-matches.js +++ b/lib/rules/summary-interactive-matches.js @@ -32,11 +32,11 @@ function hasDetailsContent(vDetails) { continue; } // True for elements with content - if (nodeType === document.ELEMENT_NODE && hasContentVirtual(vNode)) { + if (nodeType === 1 && hasContentVirtual(vNode)) { return true; } // True for text nodes - if (nodeType === document.TEXT_NODE && nodeValue.trim()) { + if (nodeType === 3 && nodeValue.trim()) { return true; } } diff --git a/test/integration/full/all-rules/all-rules.html b/test/integration/full/all-rules/all-rules.html index fc982c3fb5..8066cb00a4 100644 --- a/test/integration/full/all-rules/all-rules.html +++ b/test/integration/full/all-rules/all-rules.html @@ -129,6 +129,10 @@

Ok

  • Hello
  • World
    • +
    + pass +

    Hello world

    +
    Large scroll area
    diff --git a/test/integration/full/isolated-env/isolated-env.html b/test/integration/full/isolated-env/isolated-env.html index 74c794e1bf..c9c60b5650 100644 --- a/test/integration/full/isolated-env/isolated-env.html +++ b/test/integration/full/isolated-env/isolated-env.html @@ -103,6 +103,10 @@

    Ok

    +
    + Hello world +

    Some text

    +