From f542dbeacecc499856677f243c239c1bd90ababd Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Tue, 30 Jan 2024 15:48:21 +0100 Subject: [PATCH 01/11] Add breaking test --- packages/alfa-aria/test/name.spec.tsx | 85 +++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/packages/alfa-aria/test/name.spec.tsx b/packages/alfa-aria/test/name.spec.tsx index 9c396fa873..a9ba57dc3c 100644 --- a/packages/alfa-aria/test/name.spec.tsx +++ b/packages/alfa-aria/test/name.spec.tsx @@ -1746,3 +1746,88 @@ test(".from() does not recurse into content documents", (t) => { t.deepEqual(Name.from(target, Device.standard()).isNone(), true); }); + +test(".from() adds spaces when needed based on CSS display", (t) => { + const targets = [ + [ + , + "inlineno space", + ], + [ + , + "inline with space", + ], + [ + , + "block no space", + ], + [ + , + "block with space", + ], + [ + , + "block inline", + ], + [ + , + "inline block", + ], + ] as const; + h.document(targets.map(([target]) => target)); + + for (const [target, expected] of targets) { + t.equal(getName(target).value, expected); + } +}); + +test(".from() correctly add spaces in aria-labelledby traversal", (t) => { + const target = ( + + ); + h.document([target]); + + t.equal( + getName(target).value, + "List of fruits Blueberry It’s a berry and it’s blue.", + ); +}); + +test(".from() doesn't trim spaces between parts of the name", (t) => { + const target = ( + + ); + h.document([target]); + + t.equal(getName(target).value, "Hello World"); +}); From 3c06d343083acdcbbcabf854eeeb441bfd90f1e6 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 31 Jan 2024 09:11:17 +0100 Subject: [PATCH 02/11] Improve documentation --- packages/alfa-aria/src/feature.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/alfa-aria/src/feature.ts b/packages/alfa-aria/src/feature.ts index 82800ee525..c963c66fc5 100644 --- a/packages/alfa-aria/src/feature.ts +++ b/packages/alfa-aria/src/feature.ts @@ -248,11 +248,19 @@ type Features = { }; }; +/** + * @remarks + * 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. + */ const Features: Features = { [Namespace.HTML]: { a: html( ifHasAttribute("href", "link", "generic"), () => [], + // Content takes precedence over title for `` elements, so we need to + // call Name.fromDescendants() before `html` looks into `title`. (element, device, state) => Name.fromDescendants(element, device, state.visit(element)), ), From deb09e02938640622f251cd60f9e651b23f06703 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 31 Jan 2024 15:52:27 +0100 Subject: [PATCH 03/11] Move files around --- .../src/dom/predicate/has-accessible-name.ts | 2 +- packages/alfa-aria/src/name/index.ts | 2 ++ packages/alfa-aria/src/{ => name}/name.ts | 21 ++++++++--- packages/alfa-aria/src/name/predicate.ts | 1 - .../alfa-aria/src/name/predicate/has-value.ts | 2 +- .../alfa-aria/src/name/predicate/index.ts | 1 + packages/alfa-aria/src/node/element.ts | 2 +- .../alfa-aria/src/node/predicate/has-name.ts | 2 +- packages/alfa-aria/src/node/text.ts | 2 +- packages/alfa-aria/test/name.spec.tsx | 35 +++++++++++++------ packages/alfa-aria/tsconfig.json | 5 +-- 11 files changed, 52 insertions(+), 23 deletions(-) create mode 100644 packages/alfa-aria/src/name/index.ts rename packages/alfa-aria/src/{ => name}/name.ts (97%) delete mode 100644 packages/alfa-aria/src/name/predicate.ts create mode 100644 packages/alfa-aria/src/name/predicate/index.ts diff --git a/packages/alfa-aria/src/dom/predicate/has-accessible-name.ts b/packages/alfa-aria/src/dom/predicate/has-accessible-name.ts index 7b0556662b..4a9120d0b3 100644 --- a/packages/alfa-aria/src/dom/predicate/has-accessible-name.ts +++ b/packages/alfa-aria/src/dom/predicate/has-accessible-name.ts @@ -2,7 +2,7 @@ import { Device } from "@siteimprove/alfa-device"; import { Element, Text } from "@siteimprove/alfa-dom"; import { Predicate } from "@siteimprove/alfa-predicate"; -import { Name } from "../../name"; +import { Name } from "../../name/name"; import { Node } from "../../node"; import { hasName } from "../../node/predicate/has-name"; diff --git a/packages/alfa-aria/src/name/index.ts b/packages/alfa-aria/src/name/index.ts new file mode 100644 index 0000000000..1b5b78cd2d --- /dev/null +++ b/packages/alfa-aria/src/name/index.ts @@ -0,0 +1,2 @@ +export * from "./name"; +export * from "./predicate"; diff --git a/packages/alfa-aria/src/name.ts b/packages/alfa-aria/src/name/name.ts similarity index 97% rename from packages/alfa-aria/src/name.ts rename to packages/alfa-aria/src/name/name.ts index b2ac0183b3..823454ec66 100644 --- a/packages/alfa-aria/src/name.ts +++ b/packages/alfa-aria/src/name/name.ts @@ -14,12 +14,12 @@ import { Thunk } from "@siteimprove/alfa-thunk"; import * as json from "@siteimprove/alfa-json"; -import { Feature } from "./feature"; -import { Role } from "./role"; +import { Feature } from "../feature"; +import { Role } from "../role"; -import * as predicate from "./name/predicate"; +import * as predicate from "./predicate"; -import { isProgrammaticallyHidden } from "./dom/predicate/is-programmatically-hidden"; +import { isProgrammaticallyHidden } from "../dom/predicate/is-programmatically-hidden"; const { isElement } = Element; const { isText } = Text; @@ -785,6 +785,11 @@ export namespace Name { } return fromDescendants(element, device, state); + // .flatMap((name) => + // normalize(name.value) === "" + // ? None + // : Option.of(Name.of(normalize(name.value), name.source)), + // ); }, // Step 2H: Use the subtree content, if descending. @@ -845,6 +850,7 @@ export namespace Name { ), ); + // const name = flatten(names.map(([value]) => value).join("")); const name = flatten(names.map(([value]) => value).join("")).trim(); if (name === "") { @@ -943,3 +949,10 @@ export namespace Name { function flatten(string: string): string { return string.replace(/\s+/g, " "); } + +/** + * + */ +function normalize(value: string): string { + return value.trim().replace(/\s+/g, " "); +} diff --git a/packages/alfa-aria/src/name/predicate.ts b/packages/alfa-aria/src/name/predicate.ts deleted file mode 100644 index 469049063a..0000000000 --- a/packages/alfa-aria/src/name/predicate.ts +++ /dev/null @@ -1 +0,0 @@ -export * from "./predicate/has-value"; diff --git a/packages/alfa-aria/src/name/predicate/has-value.ts b/packages/alfa-aria/src/name/predicate/has-value.ts index eb6527b5b2..7f24d211c5 100644 --- a/packages/alfa-aria/src/name/predicate/has-value.ts +++ b/packages/alfa-aria/src/name/predicate/has-value.ts @@ -1,6 +1,6 @@ import { Predicate } from "@siteimprove/alfa-predicate"; -import { Name } from "../../name"; +import { Name } from "../name"; const { equals } = Predicate; diff --git a/packages/alfa-aria/src/name/predicate/index.ts b/packages/alfa-aria/src/name/predicate/index.ts new file mode 100644 index 0000000000..a0463c8024 --- /dev/null +++ b/packages/alfa-aria/src/name/predicate/index.ts @@ -0,0 +1 @@ +export * from "./has-value"; diff --git a/packages/alfa-aria/src/node/element.ts b/packages/alfa-aria/src/node/element.ts index 307563c8a0..3105472d97 100644 --- a/packages/alfa-aria/src/node/element.ts +++ b/packages/alfa-aria/src/node/element.ts @@ -6,7 +6,7 @@ import { Refinement } from "@siteimprove/alfa-refinement"; import * as dom from "@siteimprove/alfa-dom"; import { Attribute } from "../attribute"; -import { Name } from "../name"; +import { Name } from "../name/name"; import { Node } from "../node"; import { Role } from "../role"; diff --git a/packages/alfa-aria/src/node/predicate/has-name.ts b/packages/alfa-aria/src/node/predicate/has-name.ts index 22168507f8..d74d9f43cf 100644 --- a/packages/alfa-aria/src/node/predicate/has-name.ts +++ b/packages/alfa-aria/src/node/predicate/has-name.ts @@ -1,6 +1,6 @@ import { Predicate } from "@siteimprove/alfa-predicate"; -import { Name } from "../../name"; +import { Name } from "../../name/name"; import { Node } from "../../node"; /** diff --git a/packages/alfa-aria/src/node/text.ts b/packages/alfa-aria/src/node/text.ts index 1bf1b6e1b5..280f17b1c4 100644 --- a/packages/alfa-aria/src/node/text.ts +++ b/packages/alfa-aria/src/node/text.ts @@ -2,7 +2,7 @@ import { Option } from "@siteimprove/alfa-option"; import * as dom from "@siteimprove/alfa-dom"; -import { Name } from "../name"; +import { Name } from "../name/name"; import { Node } from "../node"; /** diff --git a/packages/alfa-aria/test/name.spec.tsx b/packages/alfa-aria/test/name.spec.tsx index a9ba57dc3c..47ec648885 100644 --- a/packages/alfa-aria/test/name.spec.tsx +++ b/packages/alfa-aria/test/name.spec.tsx @@ -1820,14 +1820,27 @@ test(".from() correctly add spaces in aria-labelledby traversal", (t) => { ); }); -test(".from() doesn't trim spaces between parts of the name", (t) => { - const target = ( - - ); - h.document([target]); - - t.equal(getName(target).value, "Hello World"); -}); +// test(".from() keeps spaces-only content between words", (t) => { +// const target = ( +// +// ); +// h.document([target]); +// +// t.equal(getName(target).value, "Hello World"); +// }); +// +// test(".from() doesn't trim spaces between parts of the name", (t) => { +// const target = ( +// +// ); +// h.document([target]); +// +// t.equal(getName(target).value, "Hello World"); +// }); diff --git a/packages/alfa-aria/tsconfig.json b/packages/alfa-aria/tsconfig.json index 5c4287423b..ff0b553c33 100644 --- a/packages/alfa-aria/tsconfig.json +++ b/packages/alfa-aria/tsconfig.json @@ -24,8 +24,9 @@ "src/dom/predicate/is-semantically-disabled.ts", "src/feature.ts", "src/index.ts", - "src/name.ts", - "src/name/predicate.ts", + "src/name/index.ts", + "src/name/name.ts", + "src/name/predicate/index.ts", "src/name/predicate/has-value.ts", "src/node.ts", "src/node/container.ts", From c29d23cce9e8416ca8a882e524540c7758115c14 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 31 Jan 2024 16:02:22 +0100 Subject: [PATCH 04/11] Split huge file --- packages/alfa-aria/src/feature.ts | 14 +- packages/alfa-aria/src/name/index.ts | 2 + packages/alfa-aria/src/name/name.ts | 497 +------------------------- packages/alfa-aria/src/name/source.ts | 300 ++++++++++++++++ packages/alfa-aria/src/name/state.ts | 199 +++++++++++ packages/alfa-aria/tsconfig.json | 2 + 6 files changed, 516 insertions(+), 498 deletions(-) create mode 100644 packages/alfa-aria/src/name/source.ts create mode 100644 packages/alfa-aria/src/name/state.ts diff --git a/packages/alfa-aria/src/feature.ts b/packages/alfa-aria/src/feature.ts index c963c66fc5..2321aecae9 100644 --- a/packages/alfa-aria/src/feature.ts +++ b/packages/alfa-aria/src/feature.ts @@ -11,7 +11,7 @@ import { Sequence } from "@siteimprove/alfa-sequence"; import { Cell, Table } from "@siteimprove/alfa-table"; import { Attribute } from "./attribute"; -import { Name } from "./name"; +import { Name, Source, State } from "./name"; import { Role } from "./role"; const { @@ -82,7 +82,7 @@ export namespace Feature { export type AttributesAspect = Aspect>; - export type NameAspect = Aspect, [Device, Name.State]>; + export type NameAspect = Aspect, [Device, State]>; export function from(namespace: Namespace, name: string): Option { return Option.from(Features[namespace]?.[name]).orElse(() => { @@ -143,14 +143,14 @@ const nameFromAttribute = (element: Element, ...attributes: Array) => { const nameFromChild = (predicate: Predicate) => - (element: Element, device: Device, state: Name.State) => + (element: Element, device: Device, state: State) => element .children() .filter(isElement) .find(predicate) .flatMap((child) => Name.fromDescendants(child, device, state.visit(child)).map((name) => - Name.of(name.value, [Name.Source.descendant(element, name)]), + Name.of(name.value, [Source.descendant(element, name)]), ), ); @@ -158,7 +158,7 @@ const ids = Cache.empty>(); const labels = Cache.empty>(); -const nameFromLabel = (element: Element, device: Device, state: Name.State) => { +const nameFromLabel = (element: Element, device: Device, state: State) => { const root = element.root(); const elements = getElementDescendants(root); @@ -215,10 +215,10 @@ const nameFromLabel = (element: Element, device: Device, state: Name.State) => { name, names.map(([name, element]) => { for (const attribute of element.attribute("for")) { - return Name.Source.reference(attribute, name); + return Source.reference(attribute, name); } - return Name.Source.ancestor(element, name); + return Source.ancestor(element, name); }), ), ); diff --git a/packages/alfa-aria/src/name/index.ts b/packages/alfa-aria/src/name/index.ts index 1b5b78cd2d..002277b7a4 100644 --- a/packages/alfa-aria/src/name/index.ts +++ b/packages/alfa-aria/src/name/index.ts @@ -1,2 +1,4 @@ export * from "./name"; export * from "./predicate"; +export * from "./source"; +export * from "./state"; diff --git a/packages/alfa-aria/src/name/name.ts b/packages/alfa-aria/src/name/name.ts index 823454ec66..bc8a0c7796 100644 --- a/packages/alfa-aria/src/name/name.ts +++ b/packages/alfa-aria/src/name/name.ts @@ -18,6 +18,8 @@ import { Feature } from "../feature"; import { Role } from "../role"; import * as predicate from "./predicate"; +import { Source } from "./source"; +import { State } from "./state"; import { isProgrammaticallyHidden } from "../dom/predicate/is-programmatically-hidden"; @@ -32,14 +34,14 @@ const { getElementIdMap } = Query; * @public */ export class Name implements Equatable, Serializable { - public static of(value: string, sources: Iterable = []): Name { + public static of(value: string, sources: Iterable = []): Name { return new Name(value, Array.from(sources)); } private readonly _value: string; - private readonly _sources: Array; + private readonly _sources: Array; - private constructor(value: string, sources: Array) { + private constructor(value: string, sources: Array) { this._value = value; this._sources = sources; } @@ -48,7 +50,7 @@ export class Name implements Equatable, Serializable { return this._value; } - public get source(): ReadonlyArray { + public get source(): ReadonlyArray { return this._sources; } @@ -93,486 +95,6 @@ export namespace Name { sources: Array; } - export type Source = - | Source.Data - | Source.Descendant - | Source.Ancestor - | Source.Label - | Source.Reference; - - export namespace Source { - export type JSON = - | Data.JSON - | Descendant.JSON - | Ancestor.JSON - | Label.JSON - | Reference.JSON; - - export class Data implements Equatable, Serializable { - public static of(text: Text): Data { - return new Data(text); - } - - private readonly _text: Text; - - private constructor(text: Text) { - this._text = text; - } - - public get type(): "data" { - return "data"; - } - - public get text(): Text { - return this._text; - } - - public equals(value: unknown): value is this { - return value instanceof Data && value._text.equals(this._text); - } - - public *[Symbol.iterator](): Iterator { - yield this._text; - } - - public toJSON(): Data.JSON { - return { - type: "data", - text: this._text.path(), - }; - } - } - - export namespace Data { - export interface JSON { - [key: string]: json.JSON; - type: "data"; - text: string; - } - } - - export function data(text: Text): Data { - return Data.of(text); - } - - export class Descendant - implements Equatable, Serializable - { - public static of(element: Element, name: Name): Descendant { - return new Descendant(element, name); - } - - private readonly _element: Element; - private readonly _name: Name; - - private constructor(element: Element, name: Name) { - this._element = element; - this._name = name; - } - - public get type(): "descendants" { - return "descendants"; - } - - public get element(): Element { - return this._element; - } - - public get name(): Name { - return this._name; - } - - public equals(value: unknown): value is this { - return ( - value instanceof Descendant && - value._element.equals(this._element) && - value._name.equals(this._name) - ); - } - - public *[Symbol.iterator](): Iterator { - yield this._element; - yield* this._name.sourceNodes(); - } - - public toJSON(): Descendant.JSON { - return { - type: "descendant", - element: this._element.path(Node.flatTree), - name: this._name.toJSON(), - }; - } - } - - export namespace Descendant { - export interface JSON { - [key: string]: json.JSON; - type: "descendant"; - element: string; - name: Name.JSON; - } - } - - export function descendant(element: Element, name: Name): Descendant { - return Descendant.of(element, name); - } - - export class Ancestor implements Equatable, Serializable { - public static of(element: Element, name: Name): Ancestor { - return new Ancestor(element, name); - } - - private readonly _element: Element; - private readonly _name: Name; - - private constructor(element: Element, name: Name) { - this._element = element; - this._name = name; - } - - public get type(): "ancestor" { - return "ancestor"; - } - - public get element(): Element { - return this._element; - } - - public get name(): Name { - return this._name; - } - - public equals(value: unknown): value is this { - return ( - value instanceof Ancestor && - value._element.equals(this._element) && - value._name.equals(this._name) - ); - } - - public *[Symbol.iterator](): Iterator { - yield this._element; - yield* this._name.sourceNodes(); - } - - public toJSON(): Ancestor.JSON { - return { - type: "ancestor", - element: this._element.path(), - name: this._name.toJSON(), - }; - } - } - - export namespace Ancestor { - export interface JSON { - [key: string]: json.JSON; - type: "ancestor"; - element: string; - name: Name.JSON; - } - } - - export function ancestor(element: Element, name: Name): Ancestor { - return Ancestor.of(element, name); - } - - export class Label implements Equatable, Serializable { - public static of(attribute: Attribute): Label { - return new Label(attribute); - } - - private readonly _attribute: Attribute; - - private constructor(attribute: Attribute) { - this._attribute = attribute; - } - - public get type(): "label" { - return "label"; - } - - public get attribute(): Attribute { - return this._attribute; - } - - public equals(value: unknown): value is this { - return ( - value instanceof Label && value._attribute.equals(this._attribute) - ); - } - - public *[Symbol.iterator](): Iterator { - yield this._attribute; - } - - public toJSON(): Label.JSON { - return { - type: "label", - attribute: this._attribute.path(), - }; - } - } - - export namespace Label { - export interface JSON { - [key: string]: json.JSON; - type: "label"; - attribute: string; - } - } - - export function label(attribute: Attribute): Label { - return Label.of(attribute); - } - - export class Reference implements Equatable, Serializable { - public static of(attribute: Attribute, name: Name): Reference { - return new Reference(attribute, name); - } - - private readonly _attribute: Attribute; - private readonly _name: Name; - - private constructor(attribute: Attribute, name: Name) { - this._attribute = attribute; - this._name = name; - } - - public get type(): "reference" { - return "reference"; - } - - public get attribute(): Attribute { - return this._attribute; - } - - public get name(): Name { - return this._name; - } - - public equals(value: unknown): value is this { - return ( - value instanceof Reference && value._attribute.equals(this._attribute) - ); - } - - public *[Symbol.iterator](): Iterator { - yield this._attribute; - yield* this._name.sourceNodes(); - } - - public toJSON(): Reference.JSON { - return { - type: "reference", - attribute: this._attribute.path(), - name: this._name.toJSON(), - }; - } - } - - export namespace Reference { - export interface JSON { - [key: string]: json.JSON; - type: "reference"; - attribute: string; - name: Name.JSON; - } - } - - export function reference(attribute: Attribute, name: Name): Reference { - return Reference.of(attribute, name); - } - } - - /** - * @internal - */ - export class State implements Equatable, Serializable { - private static _empty = new State([], None, None, false, false); - - public static empty(): State { - return this._empty; - } - - private readonly _visited: Array; - // which element has an aria-labelledby causing the current traversal? - private readonly _referrer: Option; - // which element was the target of aria-labelledby? - private readonly _referred: Option; - private readonly _isRecursing: boolean; - private readonly _isDescending: boolean; - - private constructor( - visited: Array, - referrer: Option, - referred: Option, - isRecursing: boolean, - isDescending: boolean, - ) { - this._visited = visited; - this._referrer = referrer; - this._referred = referred; - this._isRecursing = isRecursing; - this._isDescending = isDescending; - } - - /** - * The elements that have been seen by the name computation so far. This is - * used for detecting circular references resulting from things such as the - * `aria-labelledby` attribute and form controls that get their name from - * a containing `