Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(new-rule): summary elements must have an accessible name #4511

Merged
merged 4 commits into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/rule-descriptions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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) |
Expand Down
44 changes: 44 additions & 0 deletions lib/rules/summary-interactive-matches.js
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor

@straker straker Jun 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can details and summary every not share the same shadowId? details cannot have a shadow root attached to it and if you put a summary as slotted content to the details they'll both have the same shadowId

<custom-details>
  <summary>slotted content</summary>
</custom-details>

<script>
customElements.define('custom-details',
  class extends HTMLElement {
    constructor() {
      super();
      const shadowRoot = this.attachShadow({ mode: 'open' });
      shadowRoot.innerHTML = '<details><slot></slot></details>';
    }
  }
);
</script>
screenshot of the axe virtual tree structure showing that both the details element and summary element have the same shadowId when using the previous code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gd catch. Do have a way to see in virtual tree that two nodes are in the same tree?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if we have a way to distinguish between light DOM tree and shadow DOM tree.

) {
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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why ignore summary elements of details that don't have content? They are still interactive and screen readers read them out. They don't seem to be treated any differently if the details has text content.

<details>
  <summary></summary>
</details>
  • JAWS / Chrome - "unlabeled zero button, collapsed"
  • NVDA / Firefox - "filled right pointing small triangle, collapsed"
  • VO / Safari - "collapsed, details"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not less sure what's going on when there's nothing in the details element to expand. Its just an empty tab stop at that point. Those aren't great, but they're not an WCAG violation IMO.

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 === 1 && hasContentVirtual(vNode)) {
return true;
}
// True for text nodes
if (nodeType === 3 && nodeValue.trim()) {
return true;
}
}
return false;
}
29 changes: 29 additions & 0 deletions lib/rules/summary-name.json
Original file line number Diff line number Diff line change
@@ -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": []
}
4 changes: 4 additions & 0 deletions locales/_template.json
Original file line number Diff line number Diff line change
Expand Up @@ -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 <svg> elements with an img, graphics-document or graphics-symbol role have an accessible text",
"help": "<svg> elements with an img role must have an alternative text"
Expand Down
4 changes: 4 additions & 0 deletions test/integration/full/all-rules/all-rules.html
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,10 @@ <h2>Ok</h2>
<li>Hello</li>
<li>World</li>
</ul>
<details>
<summary>pass</summary>
<p>Hello world</p>
</details>

<div style="height: 100vh">Large scroll area</div>
<button id="end-of-page">End of page</button>
Expand Down
4 changes: 4 additions & 0 deletions test/integration/full/isolated-env/isolated-env.html
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ <h2>Ok</h2>
<button id="fail1"></button>
<span id="pass1"></span>
<button id="pass2"></button>
<details>
<summary>Hello world</summary>
<p>Some text</p>
</details>
<div aria-labelledby="fail1 pass1 pass2"></div>
<audio
id="incomplete1"
Expand Down
103 changes: 103 additions & 0 deletions test/integration/rules/summary-name/summary-name.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
<details>
<summary id="empty-fail"></summary>
Hello world
</details>

<details>
<summary id="text-pass">name</summary>
Hello world
</details>

<details>
<summary id="aria-label-pass" aria-label="Name"></summary>
Hello world
</details>

<details>
<summary id="aria-label-fail" aria-label=""></summary>
Hello world
</details>

<details>
<summary id="aria-labelledby-pass" aria-labelledby="labeldiv"></summary>
Hello world
</details>

<details>
<summary id="aria-labelledby-fail" aria-labelledby="nonexistent"></summary>
Hello world
</details>

<details>
<summary id="aria-labelledby-empty-fail" aria-labelledby="emptydiv"></summary>
Hello world
</details>
<div id="labeldiv">summary label</div>
<div id="emptydiv"></div>

<details>
<summary id="combo-pass" aria-label="Aria Name">Name</summary>
Hello world
</details>

<details>
<summary id="title-pass" title="Title"></summary>
Hello world
</details>

<details>
<summary id="presentation-role-fail" role="presentation"></summary>
Conflict resolution gets this to be ignored
</details>

<details>
<summary id="none-role-fail" role="none"></summary>
Conflict resolution gets this to be ignored
</details>

<details>
<summary id="heading-role-fail" role="heading"></summary>
Conflict resolution gets this to be ignored
</details>

<!-- Invalid naming methods -->

<details>
<summary id="value-attr-fail" value="Button Name"></summary>
Not a valid method for giving a name
</details>

<details>
<summary id="alt-attr-fail" alt="Button Name"></summary>
Not a valid method for giving a name
</details>

<label>
<details>
<summary id="label-elm-fail"></summary>
Text here
</details>
Not a valid method for giving a name
</label>

<!-- inapplicable cases -->

<details>
<summary id="no-siblings-inapplicable"></summary>
</details>

<details>
<summary id="empty-siblings-inapplicable"></summary>
<div><span> </span></div>
<!-- and a comment -->
<script>
const hello = 'world';
</script>
<style>
.hello {
world: blue;
}
</style>
<template> Skip me </template>
<noscript>And skip me</noscript>
</details>
23 changes: 23 additions & 0 deletions test/integration/rules/summary-name/summary-name.json
Original file line number Diff line number Diff line change
@@ -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"]
]
}
111 changes: 111 additions & 0 deletions test/integration/virtual-rules/summary-name.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
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;
}

describe('summary-name virtual-rule', () => {
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);
});
});
});
6 changes: 6 additions & 0 deletions test/rule-matches/no-naming-method-matches.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ describe('no-naming-method-matches', function () {
assert.isFalse(actual);
});

it('returns false when node is SUMMARY', function () {
const vNode = queryFixture('<summary id="target"></summary>');
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(
Expand Down
Loading
Loading