From 38e07fe28fe9dbcf26c9db323ebc9002dc2140c7 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Mon, 2 Dec 2024 14:42:17 +0100 Subject: [PATCH 01/13] Add SIA-R116: summary element has non-empty accessible name --- packages/alfa-rules/src/rules.ts | 1 + packages/alfa-rules/src/sia-r116/rule.ts | 86 ++++++++++++++++++++++++ packages/alfa-rules/src/tsconfig.json | 1 + 3 files changed, 88 insertions(+) create mode 100644 packages/alfa-rules/src/sia-r116/rule.ts diff --git a/packages/alfa-rules/src/rules.ts b/packages/alfa-rules/src/rules.ts index 0e9eb2f45b..08b0cbcecb 100644 --- a/packages/alfa-rules/src/rules.ts +++ b/packages/alfa-rules/src/rules.ts @@ -180,4 +180,5 @@ export { R110, R111, R113, + R116, }; diff --git a/packages/alfa-rules/src/sia-r116/rule.ts b/packages/alfa-rules/src/sia-r116/rule.ts new file mode 100644 index 0000000000..4ea0a8837b --- /dev/null +++ b/packages/alfa-rules/src/sia-r116/rule.ts @@ -0,0 +1,86 @@ +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, isElement } = Element; +const { and, not } = Refinement; +const { getElementDescendants } = Query; + +export default Rule.Atomic.of>({ + 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), + 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(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 \`\` element has an accessible name`), + ); + + export const HasNoAccessibleName = Err.of( + Diagnostic.of(`The \`\` element does not have an accessible name`), + ); +} + +/** + * {@link + * https://html.spec.whatwg.org/multipage/#summary-for-its-parent-details} + */ +function isSummaryForItsParentDetails(summary: Element<"summary">): boolean { + return summary + .parent() + .filter(and(isElement, hasName("details"))) + .some((details) => + details + .children() + .find(and(isElement, hasName("summary"))) + .includes(summary), + ); +} diff --git a/packages/alfa-rules/src/tsconfig.json b/packages/alfa-rules/src/tsconfig.json index 801c880c39..ef406309db 100644 --- a/packages/alfa-rules/src/tsconfig.json +++ b/packages/alfa-rules/src/tsconfig.json @@ -159,6 +159,7 @@ "./sia-r111/rule.ts", "./sia-r113/rule.ts", "./sia-r114/rule.ts", + "./sia-r116/rule.ts", "./tags/index.ts", "./tags/stability.ts", "./tags/scope.ts", From 6088bfd161f24d605a30fbb5c988fd8cc1889082 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Mon, 2 Dec 2024 14:50:20 +0100 Subject: [PATCH 02/13] Add SIA-R116: element has non-empty accessible name --- .changeset/eight-dolphins-taste.md | 5 +++ packages/alfa-rules/src/rules.ts | 1 + .../alfa-rules/test/sia-r116/rule.spec.tsx | 41 +++++++++++++++++++ packages/alfa-rules/test/tsconfig.json | 3 +- 4 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 .changeset/eight-dolphins-taste.md create mode 100644 packages/alfa-rules/test/sia-r116/rule.spec.tsx diff --git a/.changeset/eight-dolphins-taste.md b/.changeset/eight-dolphins-taste.md new file mode 100644 index 0000000000..7727ae2c27 --- /dev/null +++ b/.changeset/eight-dolphins-taste.md @@ -0,0 +1,5 @@ +--- +"@siteimprove/alfa-rules": minor +--- + +**Added:** SIA-R116: "`` element has non-empty accessible name" is now available. diff --git a/packages/alfa-rules/src/rules.ts b/packages/alfa-rules/src/rules.ts index 08b0cbcecb..110546331f 100644 --- a/packages/alfa-rules/src/rules.ts +++ b/packages/alfa-rules/src/rules.ts @@ -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, diff --git a/packages/alfa-rules/test/sia-r116/rule.spec.tsx b/packages/alfa-rules/test/sia-r116/rule.spec.tsx new file mode 100644 index 0000000000..3622ed2a27 --- /dev/null +++ b/packages/alfa-rules/test/sia-r116/rule.spec.tsx @@ -0,0 +1,41 @@ +import { type Element, h } from "@siteimprove/alfa-dom"; +import { test } from "@siteimprove/alfa-test"; + +import R116, { Outcomes } from "../../dist/sia-r116/rule.js"; + +import { evaluate } from "../common/evaluate.js"; +import { failed, inapplicable, passed } from "../common/outcome.js"; + +test("evaluate() passes summary elements with an accessible name", async (t) => { + const target = (Opening times) as Element<"summary">; + + const document = h.document([ +
+ {target} +

This is a website. We are available 24/7.

+
, + ]); + + t.deepEqual(await evaluate(R116, { document }), [ + passed(R116, target, { + 1: Outcomes.HasAccessibleName, + }), + ]); +}); + +// test("evaluate() passes summary elements that are not the first children", async (t) => { +// const target = (Opening times) as Element<"summary">; +// +// const document = h.document([ +//
+//

This is a website. We are available 24/7.

+// {target} +//
, +// ]); +// +// t.deepEqual(await evaluate(R116, { document }), [ +// passed(R116, target, { +// 1: Outcomes.HasAccessibleName, +// }), +// ]); +// }); diff --git a/packages/alfa-rules/test/tsconfig.json b/packages/alfa-rules/test/tsconfig.json index 3a99845ba0..0897cbc765 100644 --- a/packages/alfa-rules/test/tsconfig.json +++ b/packages/alfa-rules/test/tsconfig.json @@ -118,7 +118,8 @@ "./sia-r110/rule.spec.tsx", "./sia-r111/rule.spec.tsx", "./sia-r113/rule.spec.tsx", - "./sia-r114/rule.spec.tsx" + "./sia-r114/rule.spec.tsx", + "./sia-r116/rule.spec.tsx" ], "references": [{ "path": "../src" }, { "path": "../../alfa-test" }] } From 99154a26ccc4e5c9c84a522a9d8cbec5b20ce28d Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Mon, 2 Dec 2024 15:00:17 +0100 Subject: [PATCH 03/13] Fix implicit role of
--- packages/alfa-aria/src/feature.ts | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/alfa-aria/src/feature.ts b/packages/alfa-aria/src/feature.ts index 682a9b47c7..f543a3bebe 100644 --- a/packages/alfa-aria/src/feature.ts +++ b/packages/alfa-aria/src/feature.ts @@ -287,6 +287,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) { @@ -297,17 +305,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( @@ -676,6 +673,8 @@ const Features: Features = { nameFromLabel, ), + summary: html(() => None), + table: html("table", () => [], nameFromChild(hasName("caption"))), tbody: html("rowgroup"), From 9ec186e60b366ec33a6cae02d406fd64a7f4d735 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Mon, 2 Dec 2024 15:01:04 +0100 Subject: [PATCH 04/13] Add changeset --- .changeset/five-frogs-beg.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/five-frogs-beg.md diff --git a/.changeset/five-frogs-beg.md b/.changeset/five-frogs-beg.md new file mode 100644 index 0000000000..67ff66af99 --- /dev/null +++ b/.changeset/five-frogs-beg.md @@ -0,0 +1,5 @@ +--- +"@siteimprove/alfa-aria": patch +--- + +**Fixed:** `
` elements now correctly have an implicit role of `group`. From e58ec0617c8ba9c4eb436f8d43e902286dd9feb8 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Tue, 3 Dec 2024 09:51:40 +0100 Subject: [PATCH 05/13] WiP on tests --- packages/alfa-rules/test/sia-r116/rule.spec.tsx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/alfa-rules/test/sia-r116/rule.spec.tsx b/packages/alfa-rules/test/sia-r116/rule.spec.tsx index 3622ed2a27..830952ee88 100644 --- a/packages/alfa-rules/test/sia-r116/rule.spec.tsx +++ b/packages/alfa-rules/test/sia-r116/rule.spec.tsx @@ -1,4 +1,7 @@ +/// +import { Device } from "@siteimprove/alfa-device"; import { type Element, h } from "@siteimprove/alfa-dom"; +import { Node } from "@siteimprove/alfa-aria"; import { test } from "@siteimprove/alfa-test"; import R116, { Outcomes } from "../../dist/sia-r116/rule.js"; @@ -16,6 +19,12 @@ test("evaluate() passes summary elements with an accessible name", async (t) =>
, ]); + const ariaSummary = Node.from(target, Device.standard()); + console.dir(ariaSummary.toJSON()); + + const ariaDetails = Node.from(target.parent().getUnsafe(), Device.standard()); + console.dir(ariaDetails.toJSON()); + t.deepEqual(await evaluate(R116, { document }), [ passed(R116, target, { 1: Outcomes.HasAccessibleName, From 6b89afc8db502592d5ea040e262682543885141e Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 4 Dec 2024 15:32:00 +0100 Subject: [PATCH 06/13] Extract isSummaryForItsParentDetail augment --- .changeset/eight-ants-lick.md | 5 ++++ packages/alfa-dom/src/node/element/augment.ts | 30 +++++++++++++++++-- .../predicate/is-suggested-focusable.ts | 17 ++--------- 3 files changed, 35 insertions(+), 17 deletions(-) create mode 100644 .changeset/eight-ants-lick.md diff --git a/.changeset/eight-ants-lick.md b/.changeset/eight-ants-lick.md new file mode 100644 index 0000000000..4106fb906d --- /dev/null +++ b/.changeset/eight-ants-lick.md @@ -0,0 +1,5 @@ +--- +"@siteimprove/alfa-dom": minor +--- + +**Added:** An `Element<"summary">#isSummaryForItsParentDetails` predicate is now available. diff --git a/packages/alfa-dom/src/node/element/augment.ts b/packages/alfa-dom/src/node/element/augment.ts index 6608c66a6c..9e7327411a 100644 --- a/packages/alfa-dom/src/node/element/augment.ts +++ b/packages/alfa-dom/src/node/element/augment.ts @@ -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 { /** @@ -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>; + + /** + * {@link https://html.spec.whatwg.org/multipage/#summary-for-its-parent-details} + */ + isSummaryForItsParentDetails(this: Element<"summary">): boolean; } } @@ -92,7 +102,7 @@ Element.prototype.optionsList = function ( ): Sequence> { if (this._optionsList === undefined) { this._optionsList = this.children() - .filter(Element.isElement) + .filter(isElement) .flatMap((child) => { switch (child.name) { case "option": @@ -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. @@ -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), + ); +}; diff --git a/packages/alfa-dom/src/node/element/predicate/is-suggested-focusable.ts b/packages/alfa-dom/src/node/element/predicate/is-suggested-focusable.ts index 5e0a13de8c..ad0aa34ddb 100644 --- a/packages/alfa-dom/src/node/element/predicate/is-suggested-focusable.ts +++ b/packages/alfa-dom/src/node/element/predicate/is-suggested-focusable.ts @@ -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 child of parent. - parent - .children() - .filter(Element.isElement) - // Switching on element.name does not narrow the type, so we must - // keep it as Element. - .find(Element.hasName("summary")) - .includes(element), - ); + // The type is ensured by the switch on the name. + return (element as Element<"summary">).isSummaryForItsParentDetails(); } return ( From ad0f0aa926e6cddad9861ebc7d2f697d5d385daa Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 4 Dec 2024 15:32:57 +0100 Subject: [PATCH 07/13] Typo --- packages/alfa-aria/src/name/name.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/alfa-aria/src/name/name.ts b/packages/alfa-aria/src/name/name.ts index f007d4b881..42041f977f 100644 --- a/packages/alfa-aria/src/name/name.ts +++ b/packages/alfa-aria/src/name/name.ts @@ -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"; From 12fbe683da5e632f5c4e90e825b394b9aef233fb Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 4 Dec 2024 17:09:01 +0100 Subject: [PATCH 08/13] Fix handling of summary elements --- .changeset/dry-cows-tap.md | 5 ++ .changeset/serious-dragons-clean.md | 5 ++ packages/alfa-aria/src/feature.ts | 6 +- packages/alfa-aria/src/name/name.ts | 31 +++++++++- packages/alfa-aria/test/node.spec.tsx | 86 +++++++++++++++++++++++++++ 5 files changed, 131 insertions(+), 2 deletions(-) create mode 100644 .changeset/dry-cows-tap.md create mode 100644 .changeset/serious-dragons-clean.md diff --git a/.changeset/dry-cows-tap.md b/.changeset/dry-cows-tap.md new file mode 100644 index 0000000000..265dc31df1 --- /dev/null +++ b/.changeset/dry-cows-tap.md @@ -0,0 +1,5 @@ +--- +"@siteimprove/alfa-aria": patch +--- + +**Fixed:** `` elements that are not summary for their parent details are now correctly treated as `generic` role. diff --git a/.changeset/serious-dragons-clean.md b/.changeset/serious-dragons-clean.md new file mode 100644 index 0000000000..8e39638a8d --- /dev/null +++ b/.changeset/serious-dragons-clean.md @@ -0,0 +1,5 @@ +--- +"@siteimprove/alfa-aria": patch +--- + +**Fixed:** `` elements that are summary for their parent details now correctly have their name computed from content. diff --git a/packages/alfa-aria/src/feature.ts b/packages/alfa-aria/src/feature.ts index f543a3bebe..1ba11a404f 100644 --- a/packages/alfa-aria/src/feature.ts +++ b/packages/alfa-aria/src/feature.ts @@ -673,7 +673,11 @@ const Features: Features = { nameFromLabel, ), - summary: html(() => None), + summary: html((element) => + (element as Element<"summary">).isSummaryForItsParentDetails() + ? None + : Option.of(Role.of("generic")), + ), table: html("table", () => [], nameFromChild(hasName("caption"))), diff --git a/packages/alfa-aria/src/name/name.ts b/packages/alfa-aria/src/name/name.ts index 42041f977f..54836ffe53 100644 --- a/packages/alfa-aria/src/name/name.ts +++ b/packages/alfa-aria/src/name/name.ts @@ -436,7 +436,8 @@ export namespace Name { // step produces an empty name. if ( !state.isReferencing && - !role.some((role) => role.isNamedBy("contents")) + !role.some((role) => role.isNamedBy("contents")) && + !wantsNameWithoutRole(element) ) { return None; } @@ -462,6 +463,34 @@ export namespace Name { ); } + /** + * Is this an element without an ARIA role that nonetheless should have a + * name exposed? + * + * @remarks + * This is a bit wonky and currently very specific. The `summary` element, + * and maybe a few other, does not have an implicit ARIA semantics (no implicit + * role), but still is interactive and needs a name. In the HTML AAM, it is + * listed as having a "computed role" of `html-summary` which is used for these + * cases. + * + * Since we only support ARIA role, our summary elements have no role. Thus, + * they cannot be named from content, which is a property of role. Additionally, + * the `html-summary` (pseudo-)role or its friends are not listed anywhere as + * allowing or not name from content. + * + * We have here a very ad-hoc test for the case that currently matter, and we + * can expand it as needed. Hopefully, a better solution will be found. + * + * {@link https://github.com/w3c/accname/issues/245} + */ + function wantsNameWithoutRole(element: Element): boolean { + return ( + Element.hasName("summary")(element) && + element.isSummaryForItsParentDetails() + ); + } + /** * @internal */ diff --git a/packages/alfa-aria/test/node.spec.tsx b/packages/alfa-aria/test/node.spec.tsx index 6bf8bbe5bf..87915881d3 100644 --- a/packages/alfa-aria/test/node.spec.tsx +++ b/packages/alfa-aria/test/node.spec.tsx @@ -572,3 +572,89 @@ test(`.from() behaves when encountering an element with global properties where ], }); }); + +test(`.from() names elements that are summary for their parent detail`, (t) => { + const summary = Opening times; + +
+ {summary} +

This is a website. We are available 24/7.

+
; + + 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 elements as generic`, (t) => { + const summary = Opening times; + + 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 elements as generic`, (t) => { + const summary = Opening times; + +
+
{summary}
+

This is a website. We are available 24/7.

+
; + + 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() treates second elements as generic`, (t) => { + const summary = Opening times; + +
+ Hello + {summary} +

This is a website. We are available 24/7.

+
; + + 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", + }, + ], + }); +}); From 97c47597c10bd37a2d63bcb37358ed68928fa2cd Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 4 Dec 2024 17:09:17 +0100 Subject: [PATCH 09/13] Refactor --- packages/alfa-rules/src/sia-r116/rule.ts | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/packages/alfa-rules/src/sia-r116/rule.ts b/packages/alfa-rules/src/sia-r116/rule.ts index 4ea0a8837b..7761be5b3f 100644 --- a/packages/alfa-rules/src/sia-r116/rule.ts +++ b/packages/alfa-rules/src/sia-r116/rule.ts @@ -31,7 +31,7 @@ export default Rule.Atomic.of>({ .filter( and( isIncludedInTheAccessibilityTree(device), - isSummaryForItsParentDetails, + (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. @@ -68,19 +68,3 @@ export namespace Outcomes { Diagnostic.of(`The \`\` element does not have an accessible name`), ); } - -/** - * {@link - * https://html.spec.whatwg.org/multipage/#summary-for-its-parent-details} - */ -function isSummaryForItsParentDetails(summary: Element<"summary">): boolean { - return summary - .parent() - .filter(and(isElement, hasName("details"))) - .some((details) => - details - .children() - .find(and(isElement, hasName("summary"))) - .includes(summary), - ); -} From 845136c562405bf1be5524a50d5671725296062e Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 4 Dec 2024 17:11:01 +0100 Subject: [PATCH 10/13] Improve documentation --- packages/alfa-aria/src/feature.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/alfa-aria/src/feature.ts b/packages/alfa-aria/src/feature.ts index 1ba11a404f..c45c2f78f9 100644 --- a/packages/alfa-aria/src/feature.ts +++ b/packages/alfa-aria/src/feature.ts @@ -674,6 +674,7 @@ const Features: Features = { ), summary: html((element) => + // the type is ensured by the name. (element as Element<"summary">).isSummaryForItsParentDetails() ? None : Option.of(Role.of("generic")), From eb48d87f87be3727309c2e7965380c39b4b6c635 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 4 Dec 2024 17:27:31 +0100 Subject: [PATCH 11/13] Add test cases --- packages/alfa-rules/src/sia-r116/rule.ts | 2 +- .../alfa-rules/test/sia-r116/rule.spec.tsx | 144 +++++++++++++++--- 2 files changed, 123 insertions(+), 23 deletions(-) diff --git a/packages/alfa-rules/src/sia-r116/rule.ts b/packages/alfa-rules/src/sia-r116/rule.ts index 7761be5b3f..a9d6dc541c 100644 --- a/packages/alfa-rules/src/sia-r116/rule.ts +++ b/packages/alfa-rules/src/sia-r116/rule.ts @@ -35,7 +35,7 @@ export default Rule.Atomic.of>({ // 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(Role.hasName("none", "presentation"))), + not(hasExplicitRole(not(Role.hasName("none", "presentation")))), ), ); }, diff --git a/packages/alfa-rules/test/sia-r116/rule.spec.tsx b/packages/alfa-rules/test/sia-r116/rule.spec.tsx index 830952ee88..a16d1c9e1c 100644 --- a/packages/alfa-rules/test/sia-r116/rule.spec.tsx +++ b/packages/alfa-rules/test/sia-r116/rule.spec.tsx @@ -1,4 +1,3 @@ -/// import { Device } from "@siteimprove/alfa-device"; import { type Element, h } from "@siteimprove/alfa-dom"; import { Node } from "@siteimprove/alfa-aria"; @@ -9,7 +8,26 @@ import R116, { Outcomes } from "../../dist/sia-r116/rule.js"; import { evaluate } from "../common/evaluate.js"; import { failed, inapplicable, passed } from "../common/outcome.js"; -test("evaluate() passes summary elements with an accessible name", async (t) => { +test("evaluate() passes summary elements with an accessible name from aria-label", async (t) => { + const target = ( + Opening times + ) as Element<"summary">; + + const document = h.document([ +
+ {target} +

This is a website. We are available 24/7.

+
, + ]); + + t.deepEqual(await evaluate(R116, { document }), [ + passed(R116, target, { + 1: Outcomes.HasAccessibleName, + }), + ]); +}); + +test("evaluate() passes summary elements with an accessible name from content", async (t) => { const target = (Opening times) as Element<"summary">; const document = h.document([ @@ -19,11 +37,22 @@ test("evaluate() passes summary elements with an accessible name", async (t) =>
, ]); - const ariaSummary = Node.from(target, Device.standard()); - console.dir(ariaSummary.toJSON()); + t.deepEqual(await evaluate(R116, { document }), [ + passed(R116, target, { + 1: Outcomes.HasAccessibleName, + }), + ]); +}); - const ariaDetails = Node.from(target.parent().getUnsafe(), Device.standard()); - console.dir(ariaDetails.toJSON()); +test("evaluate() passes summary elements that are not the first children", async (t) => { + const target = (Opening times) as Element<"summary">; + + const document = h.document([ +
+

This is a website. We are available 24/7.

+ {target} +
, + ]); t.deepEqual(await evaluate(R116, { document }), [ passed(R116, target, { @@ -32,19 +61,90 @@ test("evaluate() passes summary elements with an accessible name", async (t) => ]); }); -// test("evaluate() passes summary elements that are not the first children", async (t) => { -// const target = (Opening times) as Element<"summary">; -// -// const document = h.document([ -//
-//

This is a website. We are available 24/7.

-// {target} -//
, -// ]); -// -// t.deepEqual(await evaluate(R116, { document }), [ -// passed(R116, target, { -// 1: Outcomes.HasAccessibleName, -// }), -// ]); -// }); +test("evaluate() is only applicable to the first summary element child", async (t) => { + const target = (Opening times) as Element<"summary">; + + const document = h.document([ +
+ {target} + Hello +

This is a website. We are available 24/7.

+
, + ]); + + t.deepEqual(await evaluate(R116, { document }), [ + passed(R116, target, { + 1: Outcomes.HasAccessibleName, + }), + ]); +}); + +test("evaluate() fails summary elements without an accessible name", async (t) => { + const target = () as Element<"summary">; + + const document = h.document([ +
+ {target} +

This is a website. We are available 24/7.

+
, + ]); + + t.deepEqual(await evaluate(R116, { document }), [ + failed(R116, target, { + 1: Outcomes.HasNoAccessibleName, + }), + ]); +}); + +test("evaluate() applies to element where the presentational conflict triggers", async (t) => { + const target = () as Element<"summary">; + + const document = h.document([ +
+ {target} +

This is a website. We are available 24/7.

+
, + ]); + + t.deepEqual(await evaluate(R116, { document }), [ + failed(R116, target, { + 1: Outcomes.HasNoAccessibleName, + }), + ]); +}); + +test("evaluate() is inapplicable to summary elements that are not summary for their parent details", async (t) => { + const document = h.document([ + Isolated, +
+
+ Nested +
+

This is a website. We are available 24/7.

+
, + ]); + + t.deepEqual(await evaluate(R116, { document }), [inapplicable(R116)]); +}); + +test("evaluate() is inapplicable to summary elements that are not exposed", async (t) => { + const document = h.document([ +
+ Opening times +

This is a website. We are available 24/7.

+
, + ]); + + t.deepEqual(await evaluate(R116, { document }), [inapplicable(R116)]); +}); + +test("evaluate() is inapplicable to summary elements with an explicit role", async (t) => { + const document = h.document([ +
+ Opening times +

This is a website. We are available 24/7.

+
, + ]); + + t.deepEqual(await evaluate(R116, { document }), [inapplicable(R116)]); +}); From da60958f12183ef8bd5ab96db8ca0d4ebcfad8cc Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Thu, 5 Dec 2024 09:06:19 +0100 Subject: [PATCH 12/13] Fix summary name computation --- packages/alfa-aria/src/feature.ts | 18 ++++++++++++----- packages/alfa-aria/src/name/name.ts | 31 +---------------------------- 2 files changed, 14 insertions(+), 35 deletions(-) diff --git a/packages/alfa-aria/src/feature.ts b/packages/alfa-aria/src/feature.ts index c45c2f78f9..7a7731ad2d 100644 --- a/packages/alfa-aria/src/feature.ts +++ b/packages/alfa-aria/src/feature.ts @@ -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]: { @@ -673,11 +675,17 @@ 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")), + 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"))), diff --git a/packages/alfa-aria/src/name/name.ts b/packages/alfa-aria/src/name/name.ts index 54836ffe53..42041f977f 100644 --- a/packages/alfa-aria/src/name/name.ts +++ b/packages/alfa-aria/src/name/name.ts @@ -436,8 +436,7 @@ export namespace Name { // step produces an empty name. if ( !state.isReferencing && - !role.some((role) => role.isNamedBy("contents")) && - !wantsNameWithoutRole(element) + !role.some((role) => role.isNamedBy("contents")) ) { return None; } @@ -463,34 +462,6 @@ export namespace Name { ); } - /** - * Is this an element without an ARIA role that nonetheless should have a - * name exposed? - * - * @remarks - * This is a bit wonky and currently very specific. The `summary` element, - * and maybe a few other, does not have an implicit ARIA semantics (no implicit - * role), but still is interactive and needs a name. In the HTML AAM, it is - * listed as having a "computed role" of `html-summary` which is used for these - * cases. - * - * Since we only support ARIA role, our summary elements have no role. Thus, - * they cannot be named from content, which is a property of role. Additionally, - * the `html-summary` (pseudo-)role or its friends are not listed anywhere as - * allowing or not name from content. - * - * We have here a very ad-hoc test for the case that currently matter, and we - * can expand it as needed. Hopefully, a better solution will be found. - * - * {@link https://github.com/w3c/accname/issues/245} - */ - function wantsNameWithoutRole(element: Element): boolean { - return ( - Element.hasName("summary")(element) && - element.isSummaryForItsParentDetails() - ); - } - /** * @internal */ From 6d7c6809fbb4eeaad96044081e4537edad83d2b1 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Thu, 5 Dec 2024 09:34:45 +0100 Subject: [PATCH 13/13] Clean up --- packages/alfa-aria/test/node.spec.tsx | 2 +- packages/alfa-rules/src/sia-r116/rule.ts | 2 +- packages/alfa-rules/test/sia-r116/rule.spec.tsx | 2 -- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/alfa-aria/test/node.spec.tsx b/packages/alfa-aria/test/node.spec.tsx index 87915881d3..592786432e 100644 --- a/packages/alfa-aria/test/node.spec.tsx +++ b/packages/alfa-aria/test/node.spec.tsx @@ -636,7 +636,7 @@ test(`.from() treats nested elements as generic`, (t) => { }); }); -test(`.from() treates second elements as generic`, (t) => { +test(`.from() treats second elements as generic`, (t) => { const summary = Opening times;
diff --git a/packages/alfa-rules/src/sia-r116/rule.ts b/packages/alfa-rules/src/sia-r116/rule.ts index a9d6dc541c..2eb0ef5ca4 100644 --- a/packages/alfa-rules/src/sia-r116/rule.ts +++ b/packages/alfa-rules/src/sia-r116/rule.ts @@ -15,7 +15,7 @@ const { hasNonEmptyAccessibleName, isIncludedInTheAccessibilityTree, } = DOM; -const { hasName, hasNamespace, isElement } = Element; +const { hasName, hasNamespace } = Element; const { and, not } = Refinement; const { getElementDescendants } = Query; diff --git a/packages/alfa-rules/test/sia-r116/rule.spec.tsx b/packages/alfa-rules/test/sia-r116/rule.spec.tsx index a16d1c9e1c..e3cece7b69 100644 --- a/packages/alfa-rules/test/sia-r116/rule.spec.tsx +++ b/packages/alfa-rules/test/sia-r116/rule.spec.tsx @@ -1,6 +1,4 @@ -import { Device } from "@siteimprove/alfa-device"; import { type Element, h } from "@siteimprove/alfa-dom"; -import { Node } from "@siteimprove/alfa-aria"; import { test } from "@siteimprove/alfa-test"; import R116, { Outcomes } from "../../dist/sia-r116/rule.js";