From 2326251312369d3b7e7035bfe3aa3dd2d66f6dca Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Mon, 22 Jun 2020 14:54:21 +0200 Subject: [PATCH 1/7] Move presentational role conflict resolation down to Role.from --- packages/alfa-aria/src/feature.ts | 13 ++++------ packages/alfa-aria/src/node.ts | 41 +------------------------------ packages/alfa-aria/src/role.ts | 34 ++++++++++++++++++++++--- 3 files changed, 37 insertions(+), 51 deletions(-) diff --git a/packages/alfa-aria/src/feature.ts b/packages/alfa-aria/src/feature.ts index 1decca197a..95b7c46abd 100644 --- a/packages/alfa-aria/src/feature.ts +++ b/packages/alfa-aria/src/feature.ts @@ -57,11 +57,7 @@ export class Feature { } export namespace Feature { - export type Aspect = []> = Mapper< - Element, - T, - A - >; + export type Aspect = []> = Mapper; export interface Status { readonly obsolete: boolean; @@ -71,7 +67,7 @@ export namespace Feature { /** * @internal */ - readonly allowPresentational?: boolean; + readonly allowPresentational: boolean; } const features = Cache.empty>(); @@ -309,9 +305,10 @@ Feature.register( Feature.register( Namespace.HTML, - Feature.of("img", (element, { allowPresentational = true }) => + Feature.of("img", (element, { allowPresentational }) => Option.of( - allowPresentational && element.attribute("alt").some((alt) => alt.value === "") + allowPresentational && + element.attribute("alt").some((alt) => alt.value === "") ? "presentation" : "img" ) diff --git a/packages/alfa-aria/src/node.ts b/packages/alfa-aria/src/node.ts index 4229eb0a1b..b70c5f9285 100644 --- a/packages/alfa-aria/src/node.ts +++ b/packages/alfa-aria/src/node.ts @@ -233,23 +233,10 @@ export namespace Node { } else { accessibleNode = Role.from(node) .flatMap((role) => { - // If the element has a presentational role, but is not allowed to - // be presentational, we fall back to its implicit role by not - // considering its explicit role. - if ( - role.some(isPresentational) && - !isAllowedPresentational(node) - ) { - return Role.from(node, { - explicit: false, - allowPresentational: false, - }); - } - return Branched.of(role); }) .flatMap((role) => { - if (role.some(isPresentational)) { + if (role.some(Role.isPresentational)) { return Branched.of(Container.of(node)); } @@ -323,29 +310,3 @@ export namespace Node { }); } } - -const isPresentational: Predicate = property( - "name", - equals("presentation", "none") -); - -/** - * Determine if an element is allowed to be presentational. - * - * @see https://w3c.github.io/aria/#conflict_resolution_presentation_none - */ -const isAllowedPresentational: Predicate = (element) => { - if (element.tabIndex().isSome()) { - return false; - } - - return Role.lookup("roletype").some((role) => { - for (const attribute of role.characteristics.supports) { - if (element.attribute(attribute).isSome()) { - return false; - } - } - - return true; - }); -}; diff --git a/packages/alfa-aria/src/role.ts b/packages/alfa-aria/src/role.ts index 99ca303c6d..0c30f99ee2 100644 --- a/packages/alfa-aria/src/role.ts +++ b/packages/alfa-aria/src/role.ts @@ -12,7 +12,7 @@ import { Feature } from "./feature"; import { Attribute } from "./attribute"; const { some } = Iterable; -const { equals, or } = Predicate; +const { equals, or, property } = Predicate; export class Role implements Equatable { public static of( @@ -270,7 +270,9 @@ export namespace Role { return feature.flatMap((feature) => feature .role(element, { - allowPresentational: options.allowPresentational, + allowPresentational: + options.allowPresentational ?? + isAllowedPresentational(element), }) .flatMap(Role.lookup) ); @@ -284,12 +286,17 @@ export namespace Role { } export namespace from { - export interface Options extends Feature.RoleOptions { + export interface Options extends Partial { readonly explicit?: boolean; readonly implicit?: boolean; } } + export const isPresentational: Predicate = property( + "name", + equals("presentation", "none") + ); + export function hasName(predicate: Predicate): Predicate; export function hasName( @@ -313,6 +320,27 @@ export namespace Role { } } +/** + * Determine if an element is allowed to be presentational. + * + * @see https://w3c.github.io/aria/#conflict_resolution_presentation_none + */ +const isAllowedPresentational: Predicate = (element) => { + if (element.tabIndex().isSome()) { + return false; + } + + return Role.lookup("roletype").some((role) => { + for (const attribute of role.characteristics.supports) { + if (element.attribute(attribute).isSome()) { + return false; + } + } + + return true; + }); +}; + import "./role/separator"; import "./role/abstract/command"; From effa90e82f274a18a7bb1b030d1722d1576682a4 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Mon, 22 Jun 2020 15:10:19 +0200 Subject: [PATCH 2/7] =?UTF-8?q?Fix=20the=20code=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/alfa-aria/src/role.ts | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/alfa-aria/src/role.ts b/packages/alfa-aria/src/role.ts index 0c30f99ee2..c61d6b01a5 100644 --- a/packages/alfa-aria/src/role.ts +++ b/packages/alfa-aria/src/role.ts @@ -12,7 +12,7 @@ import { Feature } from "./feature"; import { Attribute } from "./attribute"; const { some } = Iterable; -const { equals, or, property } = Predicate; +const { equals, or, not, property, test } = Predicate; export class Role implements Equatable { public static of( @@ -129,6 +129,7 @@ export namespace Role { /** * @see https://www.w3.org/TR/wai-aria/#roles_categorization */ + export enum Category { /** * @see https://www.w3.org/TR/wai-aria/#abstract_roles @@ -233,6 +234,8 @@ export namespace Role { ): Branched, Browser> { const role = element.attribute("role").map((attr) => attr.value.trim()); + const allowedPresentational = options.allowPresentational ?? isAllowedPresentational(element); + return ( Branched.of, Browser>( role.map((role) => role.toLowerCase()) @@ -251,15 +254,22 @@ export namespace Role { const role = Role.lookup(name); if ( + // If the role is not abstract… role.some( (role) => role.category !== Role.Category.Abstract + ) && + // …and it's not a presentational role in a forbidden context… + !( + role.some(Role.isPresentational) && + !isAllowedPresentational ) ) { + // …then we got ourselves a valid explicit role… return role; } } } - + // …otherwise, default to implicit role computation. return None; }) .orElse(() => { @@ -271,8 +281,7 @@ export namespace Role { feature .role(element, { allowPresentational: - options.allowPresentational ?? - isAllowedPresentational(element), + allowedPresentational, }) .flatMap(Role.lookup) ); @@ -292,10 +301,7 @@ export namespace Role { } } - export const isPresentational: Predicate = property( - "name", - equals("presentation", "none") - ); + export const isPresentational = hasName("presentation", "none"); export function hasName(predicate: Predicate): Predicate; From 6f6f245e899d889ddab791301168b733a355c2b5 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Mon, 22 Jun 2020 15:49:30 +0200 Subject: [PATCH 3/7] Remove unused code --- packages/alfa-aria/src/node.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/alfa-aria/src/node.ts b/packages/alfa-aria/src/node.ts index b70c5f9285..18c4207861 100644 --- a/packages/alfa-aria/src/node.ts +++ b/packages/alfa-aria/src/node.ts @@ -232,9 +232,6 @@ export namespace Node { accessibleNode = Branched.of(Container.of(node)); } else { accessibleNode = Role.from(node) - .flatMap((role) => { - return Branched.of(role); - }) .flatMap((role) => { if (role.some(Role.isPresentational)) { return Branched.of(Container.of(node)); From d9b2929b5f71898f4116cfbdb5302ab2f5e618f0 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Mon, 22 Jun 2020 15:49:42 +0200 Subject: [PATCH 4/7] Streamline comments --- packages/alfa-aria/src/role.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/alfa-aria/src/role.ts b/packages/alfa-aria/src/role.ts index c61d6b01a5..58019d4b49 100644 --- a/packages/alfa-aria/src/role.ts +++ b/packages/alfa-aria/src/role.ts @@ -254,22 +254,22 @@ export namespace Role { const role = Role.lookup(name); if ( - // If the role is not abstract… + // If the role is not abstract... role.some( (role) => role.category !== Role.Category.Abstract ) && - // …and it's not a presentational role in a forbidden context… + // ...and it's not a presentational role in a forbidden context... !( role.some(Role.isPresentational) && !isAllowedPresentational ) ) { - // …then we got ourselves a valid explicit role… + // ...then we got ourselves a valid explicit role... return role; } } } - // …otherwise, default to implicit role computation. + // ...otherwise, default to implicit role computation. return None; }) .orElse(() => { From 6fdee0b7344ddbf70df945fe01473e25a3471b08 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Mon, 22 Jun 2020 15:50:31 +0200 Subject: [PATCH 5/7] Remove useless empty line Co-authored-by: Kasper Isager --- packages/alfa-aria/src/role.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/alfa-aria/src/role.ts b/packages/alfa-aria/src/role.ts index 58019d4b49..dd0a9aaff2 100644 --- a/packages/alfa-aria/src/role.ts +++ b/packages/alfa-aria/src/role.ts @@ -129,7 +129,6 @@ export namespace Role { /** * @see https://www.w3.org/TR/wai-aria/#roles_categorization */ - export enum Category { /** * @see https://www.w3.org/TR/wai-aria/#abstract_roles From 87d28df649c842ee6f9a9e6f98e8216825da78d2 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Mon, 22 Jun 2020 15:55:10 +0200 Subject: [PATCH 6/7] Clean up --- packages/alfa-aria/src/role.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/alfa-aria/src/role.ts b/packages/alfa-aria/src/role.ts index dd0a9aaff2..64f2cd30cb 100644 --- a/packages/alfa-aria/src/role.ts +++ b/packages/alfa-aria/src/role.ts @@ -12,7 +12,7 @@ import { Feature } from "./feature"; import { Attribute } from "./attribute"; const { some } = Iterable; -const { equals, or, not, property, test } = Predicate; +const { equals, or } = Predicate; export class Role implements Equatable { public static of( From 0366d316e30106b80ebdc2cf35b868d054421f74 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Mon, 22 Jun 2020 15:56:46 +0200 Subject: [PATCH 7/7] Run prettier --- packages/alfa-aria/src/node.ts | 81 +++++++++++++++++----------------- packages/alfa-aria/src/role.ts | 6 +-- 2 files changed, 43 insertions(+), 44 deletions(-) diff --git a/packages/alfa-aria/src/node.ts b/packages/alfa-aria/src/node.ts index 18c4207861..252f32adec 100644 --- a/packages/alfa-aria/src/node.ts +++ b/packages/alfa-aria/src/node.ts @@ -231,59 +231,58 @@ export namespace Node { if (style.computed("visibility").value.value !== "visible") { accessibleNode = Branched.of(Container.of(node)); } else { - accessibleNode = Role.from(node) - .flatMap((role) => { - if (role.some(Role.isPresentational)) { - return Branched.of(Container.of(node)); - } + accessibleNode = Role.from(node).flatMap((role) => { + if (role.some(Role.isPresentational)) { + return Branched.of(Container.of(node)); + } - let attributes = Map.empty(); + let attributes = Map.empty(); - // First pass: Look up implicit attributes on the role. - if (role.isSome()) { - const queue = [role.get()]; + // First pass: Look up implicit attributes on the role. + if (role.isSome()) { + const queue = [role.get()]; - while (queue.length > 0) { - const role = queue.pop()!; + while (queue.length > 0) { + const role = queue.pop()!; - for (const [name, value] of role.characteristics.implicits) { - attributes = attributes.set(name, value); - } + for (const [name, value] of role.characteristics.implicits) { + attributes = attributes.set(name, value); + } - for (const name of role.characteristics.inherits) { - for (const role of Role.lookup(name)) { - queue.push(role); - } + for (const name of role.characteristics.inherits) { + for (const role of Role.lookup(name)) { + queue.push(role); } } } + } - // Second pass: Look up implicit attributes on the feature mapping. - for (const namespace of node.namespace) { - for (const feature of Feature.lookup(namespace, node.name)) { - attributes = attributes.concat(feature.attributes(node)); - } + // Second pass: Look up implicit attributes on the feature mapping. + for (const namespace of node.namespace) { + for (const feature of Feature.lookup(namespace, node.name)) { + attributes = attributes.concat(feature.attributes(node)); } - - // Third pass: Look up explicit `aria-*` attributes and set the - // ones that are allowed by the role. - for (const attribute of node.attributes) { - if ( - attribute.name.startsWith("aria-") && - role - .orElse(() => Role.lookup("roletype")) - .some((role) => - role.isAllowed(property("name", equals(attribute.name))) - ) - ) { - attributes = attributes.set(attribute.name, attribute.value); - } + } + + // Third pass: Look up explicit `aria-*` attributes and set the + // ones that are allowed by the role. + for (const attribute of node.attributes) { + if ( + attribute.name.startsWith("aria-") && + role + .orElse(() => Role.lookup("roletype")) + .some((role) => + role.isAllowed(property("name", equals(attribute.name))) + ) + ) { + attributes = attributes.set(attribute.name, attribute.value); } + } - return getName(node, device).map((name) => - Element.of(node, role, name, attributes) - ); - }); + return getName(node, device).map((name) => + Element.of(node, role, name, attributes) + ); + }); } } diff --git a/packages/alfa-aria/src/role.ts b/packages/alfa-aria/src/role.ts index 64f2cd30cb..2d1cd5d45b 100644 --- a/packages/alfa-aria/src/role.ts +++ b/packages/alfa-aria/src/role.ts @@ -233,7 +233,8 @@ export namespace Role { ): Branched, Browser> { const role = element.attribute("role").map((attr) => attr.value.trim()); - const allowedPresentational = options.allowPresentational ?? isAllowedPresentational(element); + const allowedPresentational = + options.allowPresentational ?? isAllowedPresentational(element); return ( Branched.of, Browser>( @@ -279,8 +280,7 @@ export namespace Role { return feature.flatMap((feature) => feature .role(element, { - allowPresentational: - allowedPresentational, + allowPresentational: allowedPresentational, }) .flatMap(Role.lookup) );