From c56012af03e2ab9d6e162bf6cc9a2cde44e6126c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristj=C3=A1n=20Oddsson?= Date: Thu, 20 Jun 2024 21:40:08 +0200 Subject: [PATCH 1/4] aria-require-children --- src/rules/aria-required-children.ts | 95 +++++++++++++++++++++++++++++ tests/act/act.js | 1 - tests/aria-required-children.ts | 34 +++++++++++ 3 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 src/rules/aria-required-children.ts create mode 100644 tests/aria-required-children.ts diff --git a/src/rules/aria-required-children.ts b/src/rules/aria-required-children.ts new file mode 100644 index 0000000..29646ad --- /dev/null +++ b/src/rules/aria-required-children.ts @@ -0,0 +1,95 @@ +import { AccessibilityError } from "../scanner"; +import { querySelectorAll } from "../utils"; + +// TODO: This list is incomplete! +type Role = + | "checkbox" + | "combobox" + | "heading" + | "menuitemcheckbox" + | "menuitemradio" + | "meter" + | "radio" + | "scrollbar" + | "seperator" + | "slider" + | "switch"; + +// TODO: This list is incomplete! +type AriaAttribute = + | "aria-checked" + | "aria-expanded" + | "aria-level" + | "aria-checked" + | "aria-valuenow" + | "aria-controls"; + +/** + * Required States and Properties: + * + * @see https://w3c.github.io/aria/#authorErrorDefaultValuesTable + */ +const roleToRequiredStatesAndPropertiesMaps: Record = { + feed: ["article"], + grid: ["rowgroup", "row"], + list: ["listitem"], + listbox: ["group", "option"], + menu: [ + "group", + "menuitemradio", + "menuitem", + "menuitemcheckbox", + "menu", + "separator", + ], + menubar: [ + "group", + "menuitemradio", + "menuitem", + "menuitemcheckbox", + "menu", + "separator", + ], + row: ["cell", "columnheader", "gridcell", "rowheader"], + rowgroup: ["row"], + suggestion: ["insertion", "deletion"], + table: ["rowgroup", "row"], + tablist: ["tab"], + tree: ["group", "treeitem"], + treegrid: ["rowgroup", "row"], +}; + +export const references = { + act: { + id: "ff89c9", + text: "ARIA required context role", + url: "https://act-rules.github.io/rules/ff89c9", + }, + axe: { + id: "aria-required-children", + text: "Certain ARIA roles must contain particular children", + url: `https://dequeuniversity.com/rules/axe/4.4/aria-required-children?application=RuleDescription`, + }, +}; + +export function ariaRequiredChildren(el: Element): AccessibilityError[] { + const errors = []; + + const selector = Object.entries(roleToRequiredStatesAndPropertiesMaps) + .map(([role, attributes]) => { + return `[role=${role}]:not(${attributes.map((attr) => `[${attr}]`).join("")})`; + }) + .join(","); + + const elements = querySelectorAll(selector, el); + if (el.matches(selector)) elements.push(el); + + for (const element of elements) { + errors.push({ + element, + text, + url, + }); + } + return errors; +} diff --git a/tests/act/act.js b/tests/act/act.js index 1b021d0..a9eea06 100644 --- a/tests/act/act.js +++ b/tests/act/act.js @@ -108,7 +108,6 @@ const rulesToIgnore = [ "efbfc7", "f51b46", "fd3a94", - "ff89c9", "ffbc54", "ffd0e9", "m6b1q3", diff --git a/tests/aria-required-children.ts b/tests/aria-required-children.ts new file mode 100644 index 0000000..b8a7dd2 --- /dev/null +++ b/tests/aria-required-children.ts @@ -0,0 +1,34 @@ +import { fixture, expect } from "@open-wc/testing"; +import { Scanner } from "../src/scanner"; +import { ariaRequiredChildren } from "../src/rules/aria-required-children"; + +const scanner = new Scanner([ariaRequiredChildren]); + +const passes = []; +const violations = []; + +describe("aria-required-attr", async function () { + // for (const markup of passes) { + // const el = await fixture(markup); + // it(el.outerHTML, async () => { + // const results = (await scanner.scan(el)).map(({ text, url }) => { + // return { text, url }; + // }); + // expect(results).to.be.empty; + // }); + // } + // for (const markup of violations) { + // const el = await fixture(markup); + // it(el.outerHTML, async () => { + // const results = (await scanner.scan(el)).map(({ text, url }) => { + // return { text, url }; + // }); + // expect(results).to.eql([ + // { + // text: "ARIA attributes must conform to valid names", + // url: "https://dequeuniversity.com/rules/axe/4.4/aria-valid-attr", + // }, + // ]); + // }); + // } +}); From a5804e37488e9fce7d9fb83eff5a193aec6a0e0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristj=C3=A1n=20Oddsson?= Date: Fri, 21 Jun 2024 10:05:14 +0200 Subject: [PATCH 2/4] update rule --- src/rules/aria-required-children.ts | 82 ++++++++++++++++++++--------- src/scanner.ts | 2 + 2 files changed, 60 insertions(+), 24 deletions(-) diff --git a/src/rules/aria-required-children.ts b/src/rules/aria-required-children.ts index 29646ad..4c1d598 100644 --- a/src/rules/aria-required-children.ts +++ b/src/rules/aria-required-children.ts @@ -3,33 +3,51 @@ import { querySelectorAll } from "../utils"; // TODO: This list is incomplete! type Role = + | "article" + | "cell" | "checkbox" + | "columnheader" | "combobox" + | "deletion" + | "feed" + | "grid" + | "gridcell" + | "group" | "heading" + | "insertion" + | "list" + | "listbox" + | "listitem" + | "menu" + | "menubar" + | "menuitem" | "menuitemcheckbox" | "menuitemradio" | "meter" + | "option" | "radio" + | "row" + | "rowgroup" + | "rowheader" | "scrollbar" + | "separator" | "seperator" | "slider" - | "switch"; - -// TODO: This list is incomplete! -type AriaAttribute = - | "aria-checked" - | "aria-expanded" - | "aria-level" - | "aria-checked" - | "aria-valuenow" - | "aria-controls"; + | "suggestion" + | "switch" + | "tab" + | "table" + | "tablist" + | "tree" + | "treegrid" + | "treeitem"; /** * Required States and Properties: * * @see https://w3c.github.io/aria/#authorErrorDefaultValuesTable */ -const roleToRequiredStatesAndPropertiesMaps: Record = { +const roleToRequiredChildRoleMapping: Partial> = { feed: ["article"], grid: ["rowgroup", "row"], list: ["listitem"], @@ -73,23 +91,39 @@ export const references = { }; export function ariaRequiredChildren(el: Element): AccessibilityError[] { + const root = document.createElement("div"); + root.append(el); const errors = []; - const selector = Object.entries(roleToRequiredStatesAndPropertiesMaps) - .map(([role, attributes]) => { - return `[role=${role}]:not(${attributes.map((attr) => `[${attr}]`).join("")})`; - }) - .join(","); + // Loop over all the different rules. + for (const [role, requiredChildren] of Object.entries( + roleToRequiredChildRoleMapping, + )) { + // Find all the elements with a role that we are interested in. + for (const parent of querySelectorAll(`[role=${role}]`, root)) { + let isValid = false; - const elements = querySelectorAll(selector, el); - if (el.matches(selector)) elements.push(el); + // Look for children of the parents with the correct roles. + // TODO: Probably special case `aria-owns` as that allows you to not have + // the items as descendants. + const childSelector = requiredChildren.reduce((selector, childRole) => { + if (!selector) return `[role=${childRole}]`; + return `${selector},[role=${childRole}]`; + }, ""); + for (const child of querySelectorAll(childSelector, parent)) { + if (!child) continue; + // TODO: Check if child is valid somehow + isValid = true; + } - for (const element of elements) { - errors.push({ - element, - text, - url, - }); + if (isValid) continue; + errors.push({ + element: parent, + text: references.axe.text, + url: references.axe.text, + }); + } } + return errors; } diff --git a/src/scanner.ts b/src/scanner.ts index 0f09989..586042a 100644 --- a/src/scanner.ts +++ b/src/scanner.ts @@ -11,6 +11,7 @@ import label from "./rules/label"; import linkName from "./rules/link-name"; import nestedInteractive from "./rules/nested-interactive"; import validLang from "./rules/valid-lang"; +import { ariaRequiredChildren } from "./rules/aria-required-children"; import { Logger } from "./logger"; @@ -38,6 +39,7 @@ export const allRules: Rule[] = [ linkName, nestedInteractive, validLang, + ariaRequiredChildren, ]; export async function requestIdleScan( From 6098e02fcd56faa9b35a21193b8375e0c77d1486 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristj=C3=A1n=20Oddsson?= Date: Fri, 21 Jun 2024 10:06:49 +0200 Subject: [PATCH 3/4] tests --- tests/aria-required-children.ts | 218 ++++++++++++++++++++++++++++---- 1 file changed, 193 insertions(+), 25 deletions(-) diff --git a/tests/aria-required-children.ts b/tests/aria-required-children.ts index b8a7dd2..0221af3 100644 --- a/tests/aria-required-children.ts +++ b/tests/aria-required-children.ts @@ -4,31 +4,199 @@ import { ariaRequiredChildren } from "../src/rules/aria-required-children"; const scanner = new Scanner([ariaRequiredChildren]); -const passes = []; -const violations = []; +// Just to get prettier working ;) +const html = String.raw; + +const passes = [ + html`
+
Item 1
+
`, + html`
+
+
+
`, + html`
+
+
+
+
+
`, + html`
+
+ Item 1 +
+
`, + html`
+ +
`, + html``, + html`
+ option + option +
`, + html`
+
+
option
+
+
`, + html`
`, + html`
`, + html``, + html`
`, + html`
+ + + +
Item 1
+
item 2
+
item 2
+
item 2
+
`, + html``, + html``, + html``, + html`
+
+
ignore
+ +
  • item 1
    • +
  • item 2
    • +
    +
    `, +]; + +const violations = [ + html`
    + +
    `, + html`
    `, + html`
    `, + html``, + html`
    +
    +
    List item 1
    +
    +
    `, + html`
    +
    + Item 1 +
    +
    `, + html`
    `, + html`
    +
    +
    `, + html`
    `, + html`
    +
  • Item 1
    • + Item 2 +
    `, + html`
    +
    +
    List item 1
    +
    List item 2
    +
    +
    `, + html`
    +
    +
    List item 1
    +
    List item 2
    +
    +
    `, + html`
    +
    +
    `, + html`
    +
    unallowed role
    +
    `, +]; + +const incomplete = [ + `
    `, + `
    `, + `
    `, + `
    `, + `
    `, + `
    `, + `
    `, + `
    `, + `
    +
    +
    `, + ``, + '', +]; + +const inapplicable = [ + `
    `, + `
    `, + `
    +
    Heading
    +
      +
      • +
      • +
    +
    `, +]; describe("aria-required-attr", async function () { - // for (const markup of passes) { - // const el = await fixture(markup); - // it(el.outerHTML, async () => { - // const results = (await scanner.scan(el)).map(({ text, url }) => { - // return { text, url }; - // }); - // expect(results).to.be.empty; - // }); - // } - // for (const markup of violations) { - // const el = await fixture(markup); - // it(el.outerHTML, async () => { - // const results = (await scanner.scan(el)).map(({ text, url }) => { - // return { text, url }; - // }); - // expect(results).to.eql([ - // { - // text: "ARIA attributes must conform to valid names", - // url: "https://dequeuniversity.com/rules/axe/4.4/aria-valid-attr", - // }, - // ]); - // }); - // } + for (const markup of passes) { + const el = await fixture(markup); + it(el.outerHTML, async () => { + const results = (await scanner.scan(el)).map(({ text, url }) => { + return { text, url }; + }); + expect(results).to.be.empty; + }); + } + for (const markup of violations) { + const el = await fixture(markup); + it(el.outerHTML, async () => { + const results = (await scanner.scan(el)).map(({ text, url }) => { + return { text, url }; + }); + expect(results).to.eql([ + { + text: "ARIA attributes must conform to valid names", + url: "https://dequeuniversity.com/rules/axe/4.4/aria-valid-attr", + }, + ]); + }); + } }); From 5fdfd73ce4d7a1502ccf22198d0a17274acc6bcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristj=C3=A1n=20Oddsson?= Date: Fri, 21 Jun 2024 10:46:40 +0200 Subject: [PATCH 4/4] aria-required-children --- src/rules/aria-required-children.ts | 4 +--- tests/act/act.js | 4 ++++ tests/aria-required-children.ts | 4 ++-- web-test-runner.config.mjs | 27 ++++++++++++++------------- 4 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/rules/aria-required-children.ts b/src/rules/aria-required-children.ts index 4c1d598..132c32f 100644 --- a/src/rules/aria-required-children.ts +++ b/src/rules/aria-required-children.ts @@ -91,8 +91,6 @@ export const references = { }; export function ariaRequiredChildren(el: Element): AccessibilityError[] { - const root = document.createElement("div"); - root.append(el); const errors = []; // Loop over all the different rules. @@ -100,7 +98,7 @@ export function ariaRequiredChildren(el: Element): AccessibilityError[] { roleToRequiredChildRoleMapping, )) { // Find all the elements with a role that we are interested in. - for (const parent of querySelectorAll(`[role=${role}]`, root)) { + for (const parent of querySelectorAll(`[role=${role}]`, el)) { let isValid = false; // Look for children of the parents with the correct roles. diff --git a/tests/act/act.js b/tests/act/act.js index a9eea06..a268f4e 100644 --- a/tests/act/act.js +++ b/tests/act/act.js @@ -116,6 +116,7 @@ const rulesToIgnore = [ "ucwvc8", "ye5d6e", "bf051a", + "ff89c9", ]; const ignoredExamples = [ @@ -125,6 +126,9 @@ const ignoredExamples = [ "https://act-rules.github.io/testcases/qt1vmo/0ef4f516db9ed70cb25f39c99637272808b8e60f.html", ]; +// TODO: Instead of dynamic tests which behave weird in Web Test Runner, we +// should generate the tests from the HTML testcases. It would be great if it's +// easy to regenerate so we don't accidentally change the tests. describe("ACT Rules", function () { for (const rule of applicableRules) { const { diff --git a/tests/aria-required-children.ts b/tests/aria-required-children.ts index 0221af3..16d64ae 100644 --- a/tests/aria-required-children.ts +++ b/tests/aria-required-children.ts @@ -193,8 +193,8 @@ describe("aria-required-attr", async function () { }); expect(results).to.eql([ { - text: "ARIA attributes must conform to valid names", - url: "https://dequeuniversity.com/rules/axe/4.4/aria-valid-attr", + text: "TODO", + url: "TODO", }, ]); }); diff --git a/web-test-runner.config.mjs b/web-test-runner.config.mjs index f113677..f76f83b 100644 --- a/web-test-runner.config.mjs +++ b/web-test-runner.config.mjs @@ -1,7 +1,6 @@ // eslint-disable-next-line foo import { env } from "node:process"; -import { summaryReporter } from "@web/test-runner"; import { esbuildPlugin } from "@web/dev-server-esbuild"; import { playwrightLauncher } from "@web/test-runner-playwright"; import { junitReporter } from "@web/test-runner-junit-reporter"; @@ -15,23 +14,12 @@ if (env.CI) { ); } -const reporters = [ - summaryReporter(), - env.CI - ? junitReporter({ - outputPath: "./test-results.xml", - reportLogs: true, - }) - : null, -]; - -export default { +const config = { nodeResolve: true, coverage: true, files: ["tests/**/*.ts", "tests/**/*.js"], plugins: [esbuildPlugin({ ts: true, target: "esnext" })], browsers, - reporters, filterBrowserLogs(log) { if ( typeof log.args[0] === "string" && @@ -44,3 +32,16 @@ export default { return true; }, }; + +if (env.CI) { + config.reporters = [ + env.CI + ? junitReporter({ + outputPath: "./test-results.xml", + reportLogs: true, + }) + : null, + ]; +} + +export default config;