Skip to content

Commit

Permalink
SIA-R116: <summary> element has non-empty accessible name (#1728)
Browse files Browse the repository at this point in the history
* Add SIA-R116: summary element has non-empty accessible name

* Add SIA-R116: <summary> element has non-empty accessible name

* Fix implicit role of <details>

* Add changeset

* WiP on tests

* Extract isSummaryForItsParentDetail augment

* Typo

* Fix handling of summary elements

* Refactor

* Improve documentation

* Add test cases

* Fix summary name computation

* Clean up
  • Loading branch information
Jym77 authored Dec 6, 2024
1 parent 360114f commit 2882c40
Show file tree
Hide file tree
Showing 15 changed files with 388 additions and 30 deletions.
5 changes: 5 additions & 0 deletions .changeset/dry-cows-tap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@siteimprove/alfa-aria": patch
---

**Fixed:** `<summary>` elements that are not summary for their parent details are now correctly treated as `generic` role.
5 changes: 5 additions & 0 deletions .changeset/eight-ants-lick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@siteimprove/alfa-dom": minor
---

**Added:** An `Element<"summary">#isSummaryForItsParentDetails` predicate is now available.
5 changes: 5 additions & 0 deletions .changeset/eight-dolphins-taste.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@siteimprove/alfa-rules": minor
---

**Added:** SIA-R116: "`<summary>` element has non-empty accessible name" is now available.
5 changes: 5 additions & 0 deletions .changeset/five-frogs-beg.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@siteimprove/alfa-aria": patch
---

**Fixed:** `<details>` elements now correctly have an implicit role of `group`.
5 changes: 5 additions & 0 deletions .changeset/serious-dragons-clean.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@siteimprove/alfa-aria": patch
---

**Fixed:** `<summary>` elements that are summary for their parent details now correctly have their name computed from content.
34 changes: 23 additions & 11 deletions packages/alfa-aria/src/feature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,8 @@ type Features = {
* The third parameter (`name`) is called during Step 2E of name computation,
* that is after `aria-labelledby` and `aria-label`, and before content.
* The `html` wrapper adds `nameFromAttributes(element, title)` at its end.
*
* {@link https://w3c.github.io/html-aam/#accname-computation}
*/
const Features: Features = {
[Namespace.HTML]: {
Expand Down Expand Up @@ -287,6 +289,14 @@ const Features: Features = {

dd: html("definition"),

details: html("group", function* (element) {
// https://w3c.github.io/html-aam/#att-open-details
yield Attribute.of(
"aria-expanded",
element.attribute("open").isSome() ? "true" : "false",
);
}),

dfn: html("term"),

dialog: html("dialog", function* (element) {
Expand All @@ -297,17 +307,6 @@ const Features: Features = {
);
}),

details: html(
() => None,
function* (element) {
// https://w3c.github.io/html-aam/#att-open-details
yield Attribute.of(
"aria-expanded",
element.attribute("open").isSome() ? "true" : "false",
);
},
),

dt: html("term"),

fieldset: html(
Expand Down Expand Up @@ -676,6 +675,19 @@ const Features: Features = {
nameFromLabel,
),

summary: html(
(element) =>
// the type is ensured by the name.
(element as Element<"summary">).isSummaryForItsParentDetails()
? None
: Option.of(Role.of("generic")),
() => [],
(element, device, state) =>
(element as Element<"summary">).isSummaryForItsParentDetails()
? Name.fromDescendants(element, device, state)
: None,
),

table: html("table", () => [], nameFromChild(hasName("caption"))),

tbody: html("rowgroup"),
Expand Down
2 changes: 1 addition & 1 deletion packages/alfa-aria/src/name/name.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Array } from "@siteimprove/alfa-array";
import { Cache } from "@siteimprove/alfa-cache";
import type { Device } from "@siteimprove/alfa-device";
import type { Attribute} from "@siteimprove/alfa-dom";
import type { Attribute } from "@siteimprove/alfa-dom";
import { Element, Node, Query, Text } from "@siteimprove/alfa-dom";
import type { Equatable } from "@siteimprove/alfa-equatable";
import type { Iterable } from "@siteimprove/alfa-iterable";
Expand Down
86 changes: 86 additions & 0 deletions packages/alfa-aria/test/node.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -572,3 +572,89 @@ test(`.from() behaves when encountering an element with global properties where
],
});
});

test(`.from() names <summary> elements that are summary for their parent detail`, (t) => {
const summary = <summary>Opening times</summary>;

<details>
{summary}
<p>This is a website. We are available 24/7.</p>
</details>;

t.deepEqual(Node.from(summary, device).toJSON(), {
type: "element",
node: "/details[1]/summary[1]",
role: null,
name: "Opening times",
attributes: [],
children: [
{
type: "text",
node: "/details[1]/summary[1]/text()[1]",
name: "Opening times",
},
],
});
});

test(`.from() treats isolated <summary> elements as generic`, (t) => {
const summary = <summary>Opening times</summary>;

t.deepEqual(Node.from(summary, device).toJSON(), {
type: "container",
node: "/summary[1]",
role: "generic",
children: [
{
type: "text",
node: "/summary[1]/text()[1]",
name: "Opening times",
},
],
});
});

test(`.from() treats nested <summary> elements as generic`, (t) => {
const summary = <summary>Opening times</summary>;

<details>
<div>{summary}</div>
<p>This is a website. We are available 24/7.</p>
</details>;

t.deepEqual(Node.from(summary, device).toJSON(), {
type: "container",
node: "/details[1]/div[1]/summary[1]",
role: "generic",
children: [
{
type: "text",
node: "/details[1]/div[1]/summary[1]/text()[1]",
name: "Opening times",
},
],
});
});

test(`.from() treats second <summary> elements as generic`, (t) => {
const summary = <summary>Opening times</summary>;

<details>
<summary>Hello</summary>
{summary}
<p>This is a website. We are available 24/7.</p>
</details>;

t.deepEqual(Node.from(summary, device).toJSON(), {
type: "container",
node: "/details[1]/summary[2]",
role: "generic",
children: [
{
type: "text",
node: "/details[1]/summary[2]/text()[1]",
name: "Opening times",
},
],
});
});
30 changes: 28 additions & 2 deletions packages/alfa-dom/src/node/element/augment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,15 @@
*/

import { None, Some } from "@siteimprove/alfa-option";
import { Refinement } from "@siteimprove/alfa-refinement";
import { Sequence } from "@siteimprove/alfa-sequence";

import { Element } from "../element.js";
import type { InputType } from "./input-type.js";

const { isElement } = Element;
const { and } = Refinement;

declare module "../element.js" {
interface Element<N extends string> {
/**
Expand All @@ -34,6 +39,11 @@ declare module "../element.js" {
* {@link https://html.spec.whatwg.org/multipage/form-elements.html#concept-select-option-list}
*/
optionsList(this: Element<"select">): Sequence<Element<"option">>;

/**
* {@link https://html.spec.whatwg.org/multipage/#summary-for-its-parent-details}
*/
isSummaryForItsParentDetails(this: Element<"summary">): boolean;
}
}

Expand Down Expand Up @@ -92,7 +102,7 @@ Element.prototype.optionsList = function (
): Sequence<Element<"option">> {
if (this._optionsList === undefined) {
this._optionsList = this.children()
.filter(Element.isElement)
.filter(isElement)
.flatMap((child) => {
switch (child.name) {
case "option":
Expand All @@ -101,7 +111,7 @@ Element.prototype.optionsList = function (
case "optgroup":
return child
.children()
.filter(Element.isElement)
.filter(isElement)
.filter(
// We cannot really use `Element.hasName` here as it would
// create a circular dependency.
Expand All @@ -117,3 +127,19 @@ Element.prototype.optionsList = function (

return this._optionsList;
};

Element.prototype.isSummaryForItsParentDetails = function (
this: Element<"summary">,
): boolean {
// We cannot use `Element.hasName` here as it would create a circular dependency.
return this.parent()
.filter(and(Element.isElement, (parent) => parent.name === "details"))
.some((details) =>
details
.children()
.find(
and(Element.isElement, (candidate) => candidate.name === "summary"),
)
.includes(this),
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,8 @@ export function isSuggestedFocusable(element: Element): boolean {
return true;

case "summary":
return element
.parent()
.filter(Element.isElement)
.some(
(parent) =>
parent.name === "details" &&
// Checking that element is the first <summary> child of parent.
parent
.children()
.filter(Element.isElement)
// Switching on element.name does not narrow the type, so we must
// keep it as Element<string>.
.find(Element.hasName<string>("summary"))
.includes(element),
);
// The type is ensured by the switch on the name.
return (element as Element<"summary">).isSummaryForItsParentDetails();
}

return (
Expand Down
2 changes: 2 additions & 0 deletions packages/alfa-rules/src/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ import R96 from "./sia-r96/rule.js";
import R110 from "./sia-r110/rule.js";
import R111 from "./sia-r111/rule.js";
import R113 from "./sia-r113/rule.js";
import R116 from "./sia-r116/rule.js";

export {
R1,
Expand Down Expand Up @@ -180,4 +181,5 @@ export {
R110,
R111,
R113,
R116,
};
70 changes: 70 additions & 0 deletions packages/alfa-rules/src/sia-r116/rule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { Diagnostic, Rule } from "@siteimprove/alfa-act";
import { DOM, Role } from "@siteimprove/alfa-aria";
import { Element, Namespace, Node, Query } from "@siteimprove/alfa-dom";
import { Refinement } from "@siteimprove/alfa-refinement";
import { Err, Ok } from "@siteimprove/alfa-result";
import { Criterion } from "@siteimprove/alfa-wcag";
import type { Page } from "@siteimprove/alfa-web";

import { expectation } from "../common/act/expectation.js";

import { Scope, Stability } from "../tags/index.js";

const {
hasExplicitRole,
hasNonEmptyAccessibleName,
isIncludedInTheAccessibilityTree,
} = DOM;
const { hasName, hasNamespace } = Element;
const { and, not } = Refinement;
const { getElementDescendants } = Query;

export default Rule.Atomic.of<Page, Element<"summary">>({
uri: "https://alfa.siteimprove.com/rules/sia-r116",
requirements: [Criterion.of("4.1.2")],
tags: [Scope.Component, Stability.Stable],
evaluate({ device, document }) {
return {
applicability() {
return getElementDescendants(document, Node.fullTree)
.filter(and(hasNamespace(Namespace.HTML), hasName("summary")))
.filter(
and(
isIncludedInTheAccessibilityTree(device),
(summary) => summary.isSummaryForItsParentDetails(),
// If the explicit role is none/presentation but the element is
// nonetheless included in the accessibility tree, then the
// conflict triggered, and we want to keep it as target.
not(hasExplicitRole(not(Role.hasName("none", "presentation")))),
),
);
},

expectations(target) {
return {
1: expectation(
// This does not explicitly exclude the ::marker pseudo-element from
// the name. Since we currently do not handle pseudo-elements, this
// is effectively the wanted outcome.
hasNonEmptyAccessibleName(device)(target),
() => Outcomes.HasAccessibleName,
() => Outcomes.HasNoAccessibleName,
),
};
},
};
},
});

/**
* @public
*/
export namespace Outcomes {
export const HasAccessibleName = Ok.of(
Diagnostic.of(`The \`<summary>\` element has an accessible name`),
);

export const HasNoAccessibleName = Err.of(
Diagnostic.of(`The \`<summary>\` element does not have an accessible name`),
);
}
1 change: 1 addition & 0 deletions packages/alfa-rules/src/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@
"./sia-r113/rule.ts",
"./sia-r114/rule.ts",
"./sia-r115/rule.ts",
"./sia-r116/rule.ts",
"./tags/index.ts",
"./tags/stability.ts",
"./tags/scope.ts",
Expand Down
Loading

0 comments on commit 2882c40

Please sign in to comment.