diff --git a/CHANGELOG.md b/CHANGELOG.md
index 15a033f..e659c41 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,32 @@
All notable changes to `@ngbracket/a11y-devtools` are documented here.
This project adheres to [Semantic Versioning](https://semver.org/).
+## 0.15.4
+
+### Fixed
+
+- **`ngbr/unreachable-control` checks that arrow keys are handled before
+ skipping a roving-tabindex item.** 0.15.2 skipped an item with
+ `tabindex="-1"` whenever Tab could get into its composite widget, so a half-built
+ tab list (the first tab at `tabindex="0"`, the rest at `-1`, no arrow-key
+ handling) passed silently. The item is now skipped only when something handles
+ keys: the widget, anything inside it, an element that controls it, or an
+ element around it up to and including the nearest component's host (so an app
+ shell's shortcut listener doesn't count). When nothing does, it's reported as
+ moderate, to verify by hand. Without Angular's dev-mode debug API (a
+ production build), listeners can't be read, so these items are still skipped.
+- **Key listeners bound with modifiers now count as key handling.** Angular
+ reports `(keydown.enter)` as `keydown.enter`, which wasn't recognised, so
+ `ngbr/click-without-key` flagged a control with `(click)`, `(keydown.enter)`
+ and `(keydown.space)`: the fix its own docs recommend. The same applies to
+ `(keydown.arrowRight)` and friends in the checks above.
+- **The mouse-shortcut skip no longer hides `ngbr/click-without-key`.** 0.15.3's
+ skip for click-only parts of a keyboard-operated widget ran before the
+ Tab-reachability check, so a click-only element Tab *does* reach, inside such a
+ widget, got no finding at all. The skip now applies only to elements Tab can't
+ reach.
+- `--from` with `--serve` no longer also names `--base` in its error message.
+
## 0.15.3
### Fixed
diff --git a/bin/ngbr-a11y-report.mjs b/bin/ngbr-a11y-report.mjs
index 3e51fc7..d825f73 100755
--- a/bin/ngbr-a11y-report.mjs
+++ b/bin/ngbr-a11y-report.mjs
@@ -111,7 +111,6 @@ function parseArgs(argv) {
}
const opts = parseArgs(process.argv.slice(2));
-if (opts.serve && !opts.base) opts.base = 'http://localhost:4200';
// Options that only mean something when scanning — refused with --from, rather
// than silently ignored.
@@ -131,6 +130,9 @@ if (opts.from && !opts.help) {
}
}
+// After the --from check, so --serve alone isn't reported as --base too.
+if (opts.serve && !opts.base) opts.base = 'http://localhost:4200';
+
if (opts.help || (!opts.from && (!opts.base || opts.routes.length === 0))) {
process.stderr.write(USAGE);
process.exit(opts.help ? 0 : 1);
diff --git a/e2e/angular-app/src/app.ts b/e2e/angular-app/src/app.ts
index 072c359..4ce651c 100644
--- a/e2e/angular-app/src/app.ts
+++ b/e2e/angular-app/src/app.ts
@@ -43,6 +43,17 @@ export class FancyDirective {}
Unreachable
No key handler
+
+ Keys
+
+
One
+
Two
+
+
+
+
One
+
Two
+
`,
})
export class HomePageComponent {
diff --git a/package-lock.json b/package-lock.json
index c25ace2..6163dfe 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,12 +1,12 @@
{
"name": "@ngbracket/a11y-devtools",
- "version": "0.15.3",
+ "version": "0.15.4",
"lockfileVersion": 3,
"requires": true,
"packages": {
"": {
"name": "@ngbracket/a11y-devtools",
- "version": "0.15.3",
+ "version": "0.15.4",
"license": "MIT",
"dependencies": {
"axe-core": "^4.13.0"
diff --git a/package.json b/package.json
index 5c081a8..b4efe9d 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
{
"name": "@ngbracket/a11y-devtools",
- "version": "0.15.3",
+ "version": "0.15.4",
"description": "Dev-only in-app accessibility auditing for Angular that maps each axe violation back to the component that rendered it — the attribution React overlay tools can't do.",
"license": "MIT",
"author": "Duncan Faulkner",
diff --git a/src/keyboard/keyboard-scan.ts b/src/keyboard/keyboard-scan.ts
index 57698ac..1694fba 100644
--- a/src/keyboard/keyboard-scan.ts
+++ b/src/keyboard/keyboard-scan.ts
@@ -17,6 +17,7 @@ import {
resolveComponentPath,
resolveDirectiveNames,
resolveListenerEvents,
+ ngDebug,
} from '../attribution.js';
import type { A11yFinding } from '../scan.js';
import {
@@ -62,6 +63,15 @@ const INTERACTIVE_ROLES = new Set([
const KEY_EVENTS = ['keydown', 'keyup', 'keypress'];
+/**
+ * True when `events` includes a key event. Angular reports a listener by the name
+ * in the template, key modifiers and all (`(keydown.enter)` → `keydown.enter`),
+ * so match on the event before the first dot.
+ */
+function hasKeyEvent(events: readonly string[]): boolean {
+ return events.some((e) => KEY_EVENTS.includes(e.split('.')[0]));
+}
+
/**
* Composite widgets: Tab reaches the widget once, and arrow keys move between
* its items (WAI-ARIA APG "Keyboard navigation inside components").
@@ -71,17 +81,26 @@ const COMPOSITE_SELECTOR = ['tablist', 'toolbar', 'menu', 'menubar', 'radiogroup
.join(',');
/**
- * True when `element` is an item that its composite widget reaches with arrow
- * keys, so Tab skipping it is correct:
- * - **aria-activedescendant**: focus stays on the widget (or on a combobox that
- * controls it) and points at the item; or
- * - **roving tabindex**: the item has a tabindex (usually -1), so script can
- * focus it, and Tab can get into the widget: one of its items is tabbable,
- * the widget itself is, or another element controls it (a menu or listbox
- * popup, which gets focus when it opens).
- * An item with neither, or a widget Tab can't enter at all, is still reported.
+ * How a composite widget's item is reached, when Tab skipping it may be correct:
+ * - `'reached'`: the keyboard reaches it within the widget —
+ * **aria-activedescendant** (focus stays on the widget, or on a combobox that
+ * controls it, and points at the item), or **roving tabindex** (the item has a
+ * tabindex, usually -1, so script can focus it; Tab can get into the widget —
+ * one of its items is tabbable, the widget itself is, or another element
+ * controls it, like a menu button — and something handles keys: see
+ * {@link handlesKeys}).
+ * - `'no-keys'`: set up for roving tabindex, but no key handling was found, so
+ * arrow keys may not move to it — the classic half-built tab list.
+ * - `false`: not a composite item, or one Tab can't get into / script can't focus.
+ *
+ * Key listeners are read through Angular's dev-mode debug API. Without it (a
+ * production build) nothing can be seen, so roving tabindex counts as reached.
*/
-function reachedWithinWidget(element: Element, isVisible?: (el: Element) => boolean): boolean {
+function reachedWithinWidget(
+ element: Element,
+ isVisible?: (el: Element) => boolean,
+ keyCache: Map = new Map(),
+): 'reached' | 'no-keys' | false {
const widget = element.parentElement?.closest(COMPOSITE_SELECTOR);
if (!widget) return false;
const controllers = widget.id
@@ -91,11 +110,52 @@ function reachedWithinWidget(element: Element, isVisible?: (el: Element) => bool
.includes(widget.id),
)
: [];
- if ([widget, ...controllers].some((el) => el.hasAttribute('aria-activedescendant'))) return true;
+ if ([widget, ...controllers].some((el) => el.hasAttribute('aria-activedescendant'))) return 'reached';
const tabindex = element.getAttribute('tabindex');
if (tabindex === null || Number.isNaN(Number.parseInt(tabindex, 10))) return false;
- return isTabbable(widget, isVisible) || controllers.length > 0 || hasTabbableDescendant(widget, isVisible);
+ const enterable =
+ isTabbable(widget, isVisible) || controllers.length > 0 || hasTabbableDescendant(widget, isVisible);
+ if (!enterable) return false;
+ if (!listenersVisible()) return 'reached';
+ // Same widget, same answer: work it out once per scan, not once per item.
+ let keys = keyCache.get(widget);
+ if (keys === undefined) keyCache.set(widget, (keys = handlesKeys(widget, controllers)));
+ return keys ? 'reached' : 'no-keys';
+}
+
+/** True when Angular's debug API can tell us which listeners an element has. */
+function listenersVisible(): boolean {
+ return typeof ngDebug()?.getListeners === 'function';
+}
+
+/**
+ * True when something that could move focus between `widget`'s items listens
+ * for keys: the widget, an element that controls it, an ancestor up to and
+ * including the nearest component host (Angular Material, for one, puts a tab
+ * list's keydown on a wrapper around the `role="tablist"`), or anything inside
+ * the widget. The walk stops at that host so an app shell's shortcut listener
+ * doesn't vouch for every widget below it.
+ */
+function handlesKeys(widget: Element, controllers: readonly Element[]): boolean {
+ const ng = ngDebug();
+ const body = widget.ownerDocument?.body;
+ const ancestors: Element[] = [];
+ for (let el = widget.parentElement; el && el !== body; el = el.parentElement) {
+ ancestors.push(el);
+ if (isComponentHost(el, ng)) break;
+ }
+ const keyed = (el: Element) => hasKeyEvent(resolveListenerEvents(el));
+ // Cheapest first; the widget's contents (every cell of a grid) last.
+ return [widget, ...controllers, ...ancestors].some(keyed) || [...widget.querySelectorAll('*')].some(keyed);
+}
+
+function isComponentHost(el: Element, ng: ReturnType): boolean {
+ try {
+ return ng?.getComponent(el) != null;
+ } catch {
+ return false;
+ }
}
const ITEM_SELECTOR = [...INTERACTIVE_ROLES].map((role) => `[role="${role}"]`).join(',');
@@ -110,14 +170,19 @@ const ITEM_SELECTOR = [...INTERACTIVE_ROLES].map((role) => `[role="${role}"]`).j
* keys. `` is never tabbable, so a page-wide shortcut listener doesn't
* count.
*/
-function partOfKeyboardOperatedWidget(element: Element, isVisible?: (el: Element) => boolean): boolean {
+function partOfKeyboardOperatedWidget(
+ element: Element,
+ isVisible?: (el: Element) => boolean,
+ keyCache?: Map,
+): boolean {
const item = element.parentElement?.closest(ITEM_SELECTOR);
if (item?.parentElement?.closest(COMPOSITE_SELECTOR)) {
- if (isTabbable(item, isVisible) || reachedWithinWidget(item, isVisible)) return true;
+ // 'no-keys' too: the item itself is reported, so its parts needn't be.
+ if (isTabbable(item, isVisible) || reachedWithinWidget(item, isVisible, keyCache)) return true;
}
for (let el = element.parentElement; el; el = el.parentElement) {
if (!isTabbable(el, isVisible)) continue;
- return resolveListenerEvents(el).some((e) => KEY_EVENTS.includes(e));
+ return hasKeyEvent(resolveListenerEvents(el));
}
return false;
}
@@ -159,6 +224,7 @@ export function scanKeyboard(
const prefixes = options.frameworkPrefixes ?? DEFAULT_FRAMEWORK_PREFIXES;
const isVisible = options.isVisible;
const findings: A11yFinding[] = [];
+ const keyCache = new Map();
const make = (
element: Element,
@@ -204,12 +270,30 @@ export function scanKeyboard(
// Hidden (a closed , [hidden], display:none): not reachable because
// it isn't shown. Check it when it is.
if (isHidden(element, isVisible)) continue;
- if (!interactiveByRole && partOfKeyboardOperatedWidget(element, isVisible)) continue;
const focusable = isTabbable(element, isVisible);
if (!focusable) {
- if (interactiveByRole && reachedWithinWidget(element, isVisible)) continue;
+ // A click-only part of something the keyboard operates is a mouse shortcut.
+ // Only for unreachable elements: one Tab reaches is still checked below.
+ if (!interactiveByRole && partOfKeyboardOperatedWidget(element, isVisible, keyCache)) continue;
+ const within = interactiveByRole ? reachedWithinWidget(element, isVisible, keyCache) : false;
+ if (within === 'reached') continue;
+ if (within === 'no-keys') {
+ findings.push(
+ make(
+ element,
+ 'ngbr/unreachable-control',
+ 'moderate',
+ `Keyboard users may not be able to reach this item: it has role="${role}" and is ` +
+ `out of the tab order, like a roving-tabindex item, but nothing in or around its ` +
+ `widget handles keys, so arrow keys may not move to it. Handle the arrow keys, or ` +
+ `use aria-activedescendant. Heuristic — verify manually.`,
+ `${RULE_DOCS}/unreachable-control`,
+ ),
+ );
+ continue;
+ }
const reason = interactiveByRole ? `has role="${role}"` : 'has a click handler';
findings.push(
make(
@@ -227,7 +311,7 @@ export function scanKeyboard(
// Focusable, but a bare (click) never fires on Enter/Space the way a native
// button does — so a keyboard user can reach it and still not activate it.
- const hasKey = events.some((e) => KEY_EVENTS.includes(e));
+ const hasKey = hasKeyEvent(events);
if (hasClick && !hasKey) {
findings.push(
make(
diff --git a/src/testing/cli-from.spec.ts b/src/testing/cli-from.spec.ts
index b624074..b1a7447 100644
--- a/src/testing/cli-from.spec.ts
+++ b/src/testing/cli-from.spec.ts
@@ -101,6 +101,12 @@ describe.skipIf(!built)('CLI: --from re-renders a saved report', () => {
expect(stderr).toContain("--route, --keyboard can't be used with it");
});
+ it('names only --serve, not a defaulted --base, when --serve is used with it', async () => {
+ const { code, stderr } = await runCli(['--from', saved, '--serve', 'npx ng serve']);
+ expect(code).toBe(2);
+ expect(stderr).toContain("so --serve can't be used with it");
+ });
+
it('refuses a file that is not a JSON report', async () => {
const md = join(dir, 'report.md');
writeFileSync(md, toMarkdown(report));
diff --git a/src/testing/e2e-angular.spec.ts b/src/testing/e2e-angular.spec.ts
index 0a0402e..7ae34af 100644
--- a/src/testing/e2e-angular.spec.ts
+++ b/src/testing/e2e-angular.spec.ts
@@ -143,6 +143,18 @@ describe.skipIf(!ready)('attribution in a real Angular app (E2E)', () => {
);
});
+ it('counts listeners with key modifiers, like (keydown.enter), as key handling', () => {
+ const on = (marker: string) => page('/').findings.filter((f) => f.html.includes(marker)).map((f) => f.id);
+ expect(on('key-modifiers')).toEqual([]);
+ expect(on('roving-tab')).toEqual([]);
+ });
+
+ it('flags a roving-tabindex tab when nothing handles the arrow keys, as moderate', () => {
+ const tab = find(page('/').findings, 'ngbr/unreachable-control', 'half-built-tab');
+ expect(tab.impact).toBe('moderate');
+ expect(tab.component).toBe('HomePageComponent');
+ });
+
it('attributes findings on a second route to that route’s page component', () => {
expect(find(page('/settings').findings, 'color-contrast', 'faint').component).toBe('SettingsPageComponent');
});
diff --git a/src/testing/keyboard-scan.spec.ts b/src/testing/keyboard-scan.spec.ts
index 5e9ad46..be7f8cd 100644
--- a/src/testing/keyboard-scan.spec.ts
+++ b/src/testing/keyboard-scan.spec.ts
@@ -11,10 +11,11 @@ function withNg(
listeners: Map,
owners: Map,
fn: () => void,
+ hosts: ReadonlySet = new Set(),
): void {
const original = (globalThis as { ng?: unknown }).ng;
(globalThis as { ng?: unknown }).ng = {
- getComponent: () => null,
+ getComponent: (el: Element) => (hosts.has(el) ? {} : null),
getOwningComponent: (el: Element) =>
owners.has(el) ? { constructor: { name: owners.get(el) } } : null,
getDirectives: () => [],
@@ -86,7 +87,93 @@ describe('scanKeyboard', () => {
Two
Three
`);
- withNg(new Map(), new Map(), () => expect(unreachable(host)).toEqual([]));
+ const keys = new Map([[host.querySelector(`[role="${widget}"]`)!, ['keydown']]]);
+ withNg(keys, new Map(), () => expect(unreachable(host)).toEqual([]));
+ });
+
+ describe('roving tabindex needs something to handle the arrow keys', () => {
+ const tabs = () =>
+ fixture(`
+
+
+
One
+
Two
+
Three
+
+
`);
+
+ it('flags the out-of-order items as a moderate, verify-by-hand finding when nothing handles keys', () => {
+ const host = tabs();
+ withNg(new Map(), new Map(), () => {
+ const found = scanKeyboard(host).filter((f) => f.id === 'ngbr/unreachable-control');
+ expect(found.map((f) => f.target)).toEqual(['div#t2', 'div#t3']);
+ expect(found.every((f) => f.impact === 'moderate')).toBe(true);
+ expect(found[0].help).toContain('nothing in or around its widget handles keys');
+ expect(found[0].help).toContain('Heuristic — verify manually');
+ });
+ });
+
+ it.each([
+ ['the widget', '#tl'],
+ ['an item', '#t1'],
+ ['a wrapper around the widget (Angular Material tabs)', '#wrap'],
+ ])('counts key handling on %s', (_where, selector) => {
+ const host = tabs();
+ withNg(new Map([[host.querySelector(selector)!, ['keydown']]]), new Map(), () =>
+ expect(unreachable(host)).toEqual([]),
+ );
+ });
+
+ it('counts key listeners with modifiers, like (keydown.arrowRight)', () => {
+ const host = tabs();
+ withNg(new Map([[host.querySelector('#tl')!, ['keydown.arrowright', 'keydown.arrowleft']]]), new Map(), () =>
+ expect(unreachable(host)).toEqual([]),
+ );
+ });
+
+ it('counts key handling on an element that controls the widget', () => {
+ const host = fixture(`
+
+
Edit
Delete
`);
+ withNg(new Map([[host.querySelector('button')!, ['keydown']]]), new Map(), () =>
+ expect(unreachable(host)).toEqual([]),
+ );
+ });
+
+ it('looks no further up than the nearest component host', () => {
+ // → →
: the shell's
+ // shortcut listener mustn't vouch for the tab list inside another component.
+ const host = fixture(`
+
+
A
B
+
`);
+ const shell = host.querySelector('#shell')!;
+ const tabsHost = host.querySelector('#tabs-host')!;
+ const hosts = new Set([shell, tabsHost]);
+ withNg(new Map([[shell, ['keydown']]]), new Map(), () => expect(unreachable(host)).toEqual(['div#b']), hosts);
+ withNg(new Map([[tabsHost, ['keydown']]]), new Map(), () => expect(unreachable(host)).toEqual([]), hosts);
+ });
+
+ it('does not count a page-wide key listener on ', () => {
+ const host = tabs();
+ withNg(new Map([[document.body, ['keydown']]]), new Map(), () =>
+ expect(unreachable(host)).toEqual(['div#t2', 'div#t3']),
+ );
+ });
+
+ it.each([
+ ['no window.ng', undefined],
+ ['a window.ng without getListeners', {}],
+ ])('gives the benefit of the doubt when listeners cannot be read (%s)', (_case, ng) => {
+ const host = tabs();
+ const original = (globalThis as { ng?: unknown }).ng;
+ (globalThis as { ng?: unknown }).ng = ng; // production builds
+ try {
+ expect(unreachable(host)).toEqual([]);
+ } finally {
+ (globalThis as { ng?: unknown }).ng = original;
+ }
+ });
});
it('aria-activedescendant on the widget: items need no tabindex', () => {
@@ -108,7 +195,7 @@ describe('scanKeyboard', () => {
const host = fixture(`
Edit
Delete
`);
- withNg(new Map(), new Map(), () => expect(unreachable(host)).toEqual([]));
+ withNg(new Map([[host.querySelector('#m')!, ['keydown']]]), new Map(), () => expect(unreachable(host)).toEqual([]));
});
it('still flags a widget Tab can never enter (every item tabindex="-1")', () => {
@@ -132,7 +219,11 @@ describe('scanKeyboard', () => {
Readme
`);
- const clickOn = (host: HTMLElement) => new Map([[host.querySelector('#twisty')!, ['click']]]);
+ const clickOn = (host: HTMLElement) =>
+ new Map([
+ [host.querySelector('#twisty')!, ['click']],
+ [host.querySelector('[role="tree"]')!, ['keydown']],
+ ]);
it.each([
['the item is tabbable', '0', '-1'],
@@ -183,6 +274,21 @@ describe('scanKeyboard', () => {
withNg(listeners, new Map(), () => expect(unreachable(host)).toEqual(['span#bar-1']));
});
+ it('still checks a click-only mark Tab reaches (click-without-key), not only unreachable ones', () => {
+ const host = fixture(`
+
+ Q1
+
`);
+ const listeners = new Map([
+ [host.querySelector('#plot')!, ['keydown']],
+ [host.querySelector('#bar-1')!, ['click']],
+ ]);
+ withNg(listeners, new Map(), () => {
+ const ids = scanKeyboard(host).map((f) => `${f.id} ${f.target}`);
+ expect(ids).toEqual(['ngbr/click-without-key span#bar-1']);
+ });
+ });
+
it('does not count a page-wide key listener (body is never tabbable)', () => {
const host = fixture(`
x
`);
const listeners = new Map([
@@ -278,6 +384,15 @@ describe('scanKeyboard', () => {
});
});
+ it('counts key handling bound with modifiers: (keydown.enter) and (keydown.space)', () => {
+ // Angular reports these by their template names, not as plain "keydown".
+ const host = fixture(`
Go
`);
+ const el = host.querySelector('#btn')!;
+ withNg(new Map([[el, ['click', 'keydown.enter', 'keydown.space']]]), new Map(), () => {
+ expect(scanKeyboard(host)).toEqual([]);
+ });
+ });
+
it('never flags a native button, even with only a click handler', () => {
const host = fixture(``);
const el = host.querySelector('#real')!;