From 296faa3a0f54303738f5480579cbb423f5444308 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Mon, 29 Jun 2020 13:46:31 +0200 Subject: [PATCH] Update SIA R67 (#272) * Upgrade hasName to modern predicates syntax * Add predicated for marked as decorative * Update SIA R67 * Add tests for SIA R67 * Fix typo breaking presentational conflict --- packages/alfa-aria/src/role.ts | 2 +- packages/alfa-dom/src/node/attribute.ts | 40 +++++- packages/alfa-rules/src/sia-r67/rule.ts | 45 +++++-- .../alfa-rules/test/sia-r67/rule.spec.tsx | 120 ++++++++++++++++++ packages/alfa-rules/tsconfig.json | 1 + 5 files changed, 195 insertions(+), 13 deletions(-) create mode 100644 packages/alfa-rules/test/sia-r67/rule.spec.tsx diff --git a/packages/alfa-aria/src/role.ts b/packages/alfa-aria/src/role.ts index 2d1cd5d45b..e0de434825 100644 --- a/packages/alfa-aria/src/role.ts +++ b/packages/alfa-aria/src/role.ts @@ -261,7 +261,7 @@ export namespace Role { // ...and it's not a presentational role in a forbidden context... !( role.some(Role.isPresentational) && - !isAllowedPresentational + !allowedPresentational ) ) { // ...then we got ourselves a valid explicit role... diff --git a/packages/alfa-dom/src/node/attribute.ts b/packages/alfa-dom/src/node/attribute.ts index 29d2cebb80..8c1ef5b947 100644 --- a/packages/alfa-dom/src/node/attribute.ts +++ b/packages/alfa-dom/src/node/attribute.ts @@ -63,8 +63,26 @@ export class Attribute extends Node { return this._owner; } - public hasName(name: string): boolean { - return this._name === foldCase(name, this._owner); + public hasName(predicate: Predicate): boolean; + + public hasName(name: string, ...rest: Array): boolean; + + public hasName( + nameOrPredicate: string | Predicate, + ...names: Array + ): boolean { + let predicate: Predicate; + + if (typeof nameOrPredicate === "function") { + predicate = nameOrPredicate; + } else { + const namesWithCases = [nameOrPredicate, ...names].map((name) => + foldCase(name, this._owner) + ); + predicate = equals(...namesWithCases); + } + + return predicate(this._name); } /** @@ -165,6 +183,24 @@ export namespace Attribute { owner ); } + + export function hasName(predicate: Predicate): Predicate; + + export function hasName( + name: string, + ...rest: Array + ): Predicate; + + export function hasName( + nameOrPredicate: string | Predicate, + ...names: Array + ): Predicate { + if (typeof nameOrPredicate === "function") { + return attribute => attribute.hasName(nameOrPredicate); + } else { + return (attribute) => attribute.hasName(nameOrPredicate, ...names); + } + } } /** diff --git a/packages/alfa-rules/src/sia-r67/rule.ts b/packages/alfa-rules/src/sia-r67/rule.ts index 7032bbbe7a..b24cb40fee 100644 --- a/packages/alfa-rules/src/sia-r67/rule.ts +++ b/packages/alfa-rules/src/sia-r67/rule.ts @@ -1,4 +1,5 @@ import { Rule, Diagnostic } from "@siteimprove/alfa-act"; +import { Node, Role } from "@siteimprove/alfa-aria"; import { Element, Namespace } from "@siteimprove/alfa-dom"; import { Predicate } from "@siteimprove/alfa-predicate"; import { Err, Ok } from "@siteimprove/alfa-result"; @@ -7,10 +8,9 @@ import { Page } from "@siteimprove/alfa-web"; import { expectation } from "../common/expectation"; import { hasAccessibleName } from "../common/predicate/has-accessible-name"; -import { isDecorative } from "../common/predicate/is-decorative"; const { isElement, hasName, hasNamespace } = Element; -const { and } = Predicate; +const { and, or } = Predicate; export default Rule.Atomic.of({ uri: "https://siteimprove.github.io/sanshikan/rules/sia-r67.html", @@ -22,7 +22,13 @@ export default Rule.Atomic.of({ .filter( and( isElement, - and(hasNamespace(Namespace.HTML), hasName("img"), isDecorative) + and( + or( + and(hasNamespace(Namespace.HTML), hasName("img")), + and(hasNamespace(Namespace.SVG), hasName("svg")) + ), + isMarkedAsDecorative + ) ) ); }, @@ -30,9 +36,16 @@ export default Rule.Atomic.of({ expectations(target) { return { 1: expectation( - hasAccessibleName(device)(target), - () => Outcomes.HasName, - () => Outcomes.HasNoName + Node.from(target, device).every((accNode) => { + const role = accNode.role(); + + return ( + role.isNone() || + Role.hasName("none", "presentation")(role.get()) + ); + }), + () => Outcomes.IsNotExposed, + () => Outcomes.IsExposed ), }; }, @@ -41,15 +54,27 @@ export default Rule.Atomic.of({ }); export namespace Outcomes { - export const HasNoName = Ok.of( + export const IsNotExposed = Ok.of( Diagnostic.of( - `The image is marked as decorative and does not have an accessible name` + `The element is marked as decorative and is not exposed` ) ); - export const HasName = Err.of( + export const IsExposed = Err.of( Diagnostic.of( - `The image is marked as decorative but has an accessible name` + `The element is marked as decorative but is exposed` ) ); } + +/** + * Check if an element is marked as decorative by looking at its role but without conflict resolution. + * If the result is "none" or "presentation", then the element is marked as decorative. + */ +function isMarkedAsDecorative(element: Element): boolean { + return ( + Role.from(element, { allowPresentational: true }) + // Element is marked as decorative if at least one browser thinks so. + .some((r) => r.some(Role.hasName("none", "presentation"))) + ); +} diff --git a/packages/alfa-rules/test/sia-r67/rule.spec.tsx b/packages/alfa-rules/test/sia-r67/rule.spec.tsx new file mode 100644 index 0000000000..636ded9ea4 --- /dev/null +++ b/packages/alfa-rules/test/sia-r67/rule.spec.tsx @@ -0,0 +1,120 @@ +import {Node} from "@siteimprove/alfa-aria"; +import {getName} from "@siteimprove/alfa-aria/src/get-name"; +import { Device } from "@siteimprove/alfa-device"; +import { jsx } from "@siteimprove/alfa-dom/jsx"; +import { Option } from "@siteimprove/alfa-option"; +import { Predicate } from "@siteimprove/alfa-predicate"; +import { test } from "@siteimprove/alfa-test"; + +import { Document, Element } from "@siteimprove/alfa-dom"; + +import R67, { Outcomes } from "../../src/sia-r67/rule"; + +import { evaluate } from "../common/evaluate"; +import { passed, failed, inapplicable } from "../common/outcome"; + +const { and } = Predicate; +const { hasId } = Element; + +const device = Device.standard(); + +function getElementById(document: Document): (id: string) => Element { + return (id) => + document + .descendants() + .find(and(Element.isElement, hasId(id))) + .get(); +} + +test("evaluate() passes on elements marked as decorative and not exposed", async (t) => { + const document = Document.of((self) => [ + Element.fromElement( + + + + + + + + + + , + Option.of(self) + ), + ]); + const getById = getElementById(document); + const emptyAlt = getById("empty-alt"); + const roleNone = getById("role-none"); + const rolePresentation = getById("role-presentation"); + const svg = getById("svg"); + const ariaHidden = getById("aria-hidden"); + const ariaHiddenInherit = getById("aria-hidden-inherit"); + + t.deepEqual(await evaluate(R67, { device, document }), [ + passed(R67, emptyAlt, { 1: Outcomes.IsNotExposed }), + passed(R67, roleNone, { 1: Outcomes.IsNotExposed }), + passed(R67, rolePresentation, { 1: Outcomes.IsNotExposed }), + passed(R67, svg, { 1: Outcomes.IsNotExposed }), + passed(R67, ariaHidden, { 1: Outcomes.IsNotExposed }), + passed(R67, ariaHiddenInherit, { 1: Outcomes.IsNotExposed }), + ]); +}); + +test("evaluate() fails on elements marked as decorative but exposed", async (t) => { + const document = Document.of((self) => [ + Element.fromElement( + + Foo + + + , + Option.of(self) + ), + ]); + const getById = getElementById(document); + const emptyAltAriaLabel = getById("empty-alt-aria-label"); + const roleNoneAriaLabelledby = getById("role-none-aria-labelledby"); + + t.deepEqual(await evaluate(R67, { device, document }), [ + failed(R67, emptyAltAriaLabel, { 1: Outcomes.IsExposed }), + failed(R67, roleNoneAriaLabelledby, { 1: Outcomes.IsExposed }), + ]); +}); + +test("evaluate() is inapplicable on non-img/svg elements", async (t) => { + const document = Document.of((self) => [ + Element.fromElement( + + + + + , + Option.of(self) + ), + ]); + + t.deepEqual(await evaluate(R67, { device, document }), [inapplicable(R67)]); +}); + +test("evaluate() is inapplicabale on elements which are not marked as decorative", async (t) => { + const document = Document.of((self) => [ + Element.fromElement( + + foo + + + + + + , + Option.of(self) + ), + ]); + + t.deepEqual(await evaluate(R67, { device, document }), [inapplicable(R67)]); +}); diff --git a/packages/alfa-rules/tsconfig.json b/packages/alfa-rules/tsconfig.json index 3a2c163619..d23a55cab7 100644 --- a/packages/alfa-rules/tsconfig.json +++ b/packages/alfa-rules/tsconfig.json @@ -125,6 +125,7 @@ "test/sia-r57/rule.spec.tsx", "test/sia-r64/rule.spec.tsx", "test/sia-r61/rule.spec.tsx", + "test/sia-r67/rule.spec.tsx", "test/sia-r68/rule.spec.tsx", "test/sia-r69/rule.spec.tsx", "test/sia-r71/rule.spec.tsx",