From f5818e46541ca5dfa94a9ed3b5f4cb1264e1f2bc Mon Sep 17 00:00:00 2001 From: Duncan Faulkner Date: Tue, 29 Sep 2026 08:16:08 +0100 Subject: [PATCH 1/3] fix(keyboard): accept one roving tab stop shared by several same-role widgets (0.15.5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A board with a listbox per column and one tab stop for the whole board had every card in the other columns reported unreachable. An item is also reachable when a tabbable item of the same role, in a widget of the same role, is inside the same component (bounded at the nearest component host; key handling still required). Specs incl. three negatives; sabotage-checked. Docs dogfood (154 routes): board 13 → 0, nothing new. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 14 +++++++++++ package-lock.json | 4 +-- package.json | 2 +- src/keyboard/keyboard-scan.ts | 36 ++++++++++++++++++++++++++- src/testing/keyboard-scan.spec.ts | 41 +++++++++++++++++++++++++++++++ 5 files changed, 93 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e659c41..754954b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,20 @@ All notable changes to `@ngbracket/a11y-devtools` are documented here. This project adheres to [Semantic Versioning](https://semver.org/). +## 0.15.5 + +### Fixed + +- **`ngbr/unreachable-control` accepts one roving tab stop shared by several + widgets.** A kanban board with a `role="listbox"` per column and a single tab + stop for the whole board had every card in the other columns reported as + serious, although Tab reaches the board and the arrow keys cross columns. An + item is now also reachable when a tabbable item of the same role, in a widget + of the same role, sits inside the same component. The search stops at the + nearest component host, so an unrelated widget elsewhere on the page can't + vouch for a broken one, and key handling is still required. Found on our own + docs: 13 false positives → 0. + ## 0.15.4 ### Fixed diff --git a/package-lock.json b/package-lock.json index 6163dfe..1ebb239 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@ngbracket/a11y-devtools", - "version": "0.15.4", + "version": "0.15.5", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@ngbracket/a11y-devtools", - "version": "0.15.4", + "version": "0.15.5", "license": "MIT", "dependencies": { "axe-core": "^4.13.0" diff --git a/package.json b/package.json index b4efe9d..f102c54 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@ngbracket/a11y-devtools", - "version": "0.15.4", + "version": "0.15.5", "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 1694fba..c89638c 100644 --- a/src/keyboard/keyboard-scan.ts +++ b/src/keyboard/keyboard-scan.ts @@ -115,7 +115,10 @@ function reachedWithinWidget( const tabindex = element.getAttribute('tabindex'); if (tabindex === null || Number.isNaN(Number.parseInt(tabindex, 10))) return false; const enterable = - isTabbable(widget, isVisible) || controllers.length > 0 || hasTabbableDescendant(widget, isVisible); + isTabbable(widget, isVisible) || + controllers.length > 0 || + hasTabbableDescendant(widget, isVisible) || + sharesRovingStop(widget, element, isVisible); if (!enterable) return false; if (!listenersVisible()) return 'reached'; // Same widget, same answer: work it out once per scan, not once per item. @@ -150,6 +153,37 @@ function handlesKeys(widget: Element, controllers: readonly Element[]): boolean return [widget, ...controllers, ...ancestors].some(keyed) || [...widget.querySelectorAll('*')].some(keyed); } +/** + * True when `widget` is one of several same-role widgets that share a single + * roving tab stop inside one component: a kanban board with a `role="listbox"` + * per column and one tab stop for the whole board, where arrow keys cross + * columns. A column without the current card has no tabbable item of its own, + * but Tab still reaches the board. Looks for a tabbable item of the same role, + * in a widget of the same role, within the nearest component host around the + * widget, so an unrelated widget elsewhere on the page can't vouch for it. + * Needs Angular's debug API to find that host; without it, it never applies. + */ +function sharesRovingStop(widget: Element, item: Element, isVisible?: (el: Element) => boolean): boolean { + const ng = ngDebug(); + const widgetRole = widget.getAttribute('role'); + const itemRole = item.getAttribute('role'); + if (!ng || !widgetRole || !itemRole) return false; + let host: Element | null = null; + for (let el = widget.parentElement; el; el = el.parentElement) { + if (isComponentHost(el, ng)) { + host = el; + break; + } + } + if (!host) return false; + return [...host.querySelectorAll(`[role="${widgetRole}"] [role="${itemRole}"]`)].some( + (other) => + other !== item && + other.parentElement?.closest(COMPOSITE_SELECTOR)?.getAttribute('role') === widgetRole && + isTabbable(other, isVisible), + ); +} + function isComponentHost(el: Element, ng: ReturnType): boolean { try { return ng?.getComponent(el) != null; diff --git a/src/testing/keyboard-scan.spec.ts b/src/testing/keyboard-scan.spec.ts index be7f8cd..0364618 100644 --- a/src/testing/keyboard-scan.spec.ts +++ b/src/testing/keyboard-scan.spec.ts @@ -198,6 +198,47 @@ describe('scanKeyboard', () => { withNg(new Map([[host.querySelector('#m')!, ['keydown']]]), new Map(), () => expect(unreachable(host)).toEqual([])); }); + describe('one roving tab stop shared by several widgets (a board: a listbox per column)', () => { + const board = (outside = '') => + fixture(` +
+
+
A
B
+
+
+
C
+
+
${outside}`); + const keysOn = (host: HTMLElement) => + new Map([...host.querySelectorAll('[role="option"]')].map((el) => [el, ['keydown']] as [Element, string[]])); + + it('skips items in a column without the tab stop when the board component shares one', () => { + const host = board(); + withNg(keysOn(host), new Map(), () => expect(unreachable(host)).toEqual([]), new Set([host.querySelector('#board')!])); + }); + + it('still flags them without a component host to bound the search', () => { + const host = board(); + withNg(keysOn(host), new Map(), () => expect(unreachable(host)).toEqual(['div#c'])); + }); + + it('is not vouched for by a tabbable item of another role', () => { + const host = fixture(` +
+
T
+
C
+
`); + withNg(keysOn(host), new Map(), () => expect(unreachable(host)).toEqual(['div#c']), new Set([host.querySelector('#board')!])); + }); + + it('is not vouched for by a listbox outside the component', () => { + const host = fixture(` +
C
+
X
`); + withNg(keysOn(host), new Map(), () => expect(unreachable(host)).toEqual(['div#c']), new Set([host.querySelector('#board')!])); + }); + }); + it('still flags a widget Tab can never enter (every item tabindex="-1")', () => { const host = fixture(`
`); From f6d87f564f855f4fe6ed417375368afbd5cc286b Mon Sep 17 00:00:00 2001 From: Duncan Faulkner Date: Tue, 29 Sep 2026 08:25:57 +0100 Subject: [PATCH 2/3] fix(keyboard): shared tab stop only between repeated sibling widgets; cache per widget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up: a working listbox could vouch for an unrelated broken one in the same component. Now the two widgets must be repeated siblings (their branches are the same element and share a class, as a @for renders — modifier classes may differ). Result cached per widget (was O(n^2)). CHANGELOG: dev builds only. Docs dogfood unchanged: 61 → 39 (board 13 → 0), nothing new. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 12 +++-- src/keyboard/keyboard-scan.ts | 80 ++++++++++++++++++++++++------- src/testing/keyboard-scan.spec.ts | 15 +++++- 3 files changed, 82 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 754954b..413bce2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,11 +11,13 @@ This project adheres to [Semantic Versioning](https://semver.org/). widgets.** A kanban board with a `role="listbox"` per column and a single tab stop for the whole board had every card in the other columns reported as serious, although Tab reaches the board and the arrow keys cross columns. An - item is now also reachable when a tabbable item of the same role, in a widget - of the same role, sits inside the same component. The search stops at the - nearest component host, so an unrelated widget elsewhere on the page can't - vouch for a broken one, and key handling is still required. Found on our own - docs: 13 false positives → 0. + item is now also reachable when another widget of the same role, inside the + same component and repeated alongside it (same element, a shared class: what + a `@for` over columns renders), has a tabbable item of the same role. So a + working listbox can't vouch for an unrelated broken one elsewhere in the + template, and key handling is still required. Development builds only: it + needs Angular's debug API to find the component, so a production-build scan + still reports these items. Found on our own docs: 13 false positives → 0. ## 0.15.4 diff --git a/src/keyboard/keyboard-scan.ts b/src/keyboard/keyboard-scan.ts index c89638c..afb5657 100644 --- a/src/keyboard/keyboard-scan.ts +++ b/src/keyboard/keyboard-scan.ts @@ -100,6 +100,7 @@ function reachedWithinWidget( element: Element, isVisible?: (el: Element) => boolean, keyCache: Map = new Map(), + sharedCache: Map = new Map(), ): 'reached' | 'no-keys' | false { const widget = element.parentElement?.closest(COMPOSITE_SELECTOR); if (!widget) return false; @@ -118,7 +119,7 @@ function reachedWithinWidget( isTabbable(widget, isVisible) || controllers.length > 0 || hasTabbableDescendant(widget, isVisible) || - sharesRovingStop(widget, element, isVisible); + sharesRovingStop(widget, element, isVisible, sharedCache); if (!enterable) return false; if (!listenersVisible()) return 'reached'; // Same widget, same answer: work it out once per scan, not once per item. @@ -155,15 +156,37 @@ function handlesKeys(widget: Element, controllers: readonly Element[]): boolean /** * True when `widget` is one of several same-role widgets that share a single - * roving tab stop inside one component: a kanban board with a `role="listbox"` - * per column and one tab stop for the whole board, where arrow keys cross - * columns. A column without the current card has no tabbable item of its own, - * but Tab still reaches the board. Looks for a tabbable item of the same role, - * in a widget of the same role, within the nearest component host around the - * widget, so an unrelated widget elsewhere on the page can't vouch for it. - * Needs Angular's debug API to find that host; without it, it never applies. + * roving tab stop: a kanban board with a `role="listbox"` per column and one tab + * stop for the whole board, where arrow keys cross columns. A column without the + * current card has no tabbable item of its own, but Tab still reaches the board. + * + * Accepted when another widget of the same role holds a tabbable item of the + * same role, both inside the nearest component host, AND the two are repeated + * siblings: where their branches meet, each branch is the same kind of element + * sharing a class (what a `@for` over columns renders; state modifiers such as + * a full column's extra class may differ). So a working + * listbox can't vouch for an unrelated broken one in another part of the same + * template. Needs Angular's debug API to find the host; without it (a + * production build) it never applies. */ -function sharesRovingStop(widget: Element, item: Element, isVisible?: (el: Element) => boolean): boolean { +function sharesRovingStop( + widget: Element, + item: Element, + isVisible: ((el: Element) => boolean) | undefined, + cache: Map, +): boolean { + const cached = cache.get(widget); + if (cached !== undefined) return cached; + const result = findsRepeatedSiblingStop(widget, item, isVisible); + cache.set(widget, result); + return result; +} + +function findsRepeatedSiblingStop( + widget: Element, + item: Element, + isVisible?: (el: Element) => boolean, +): boolean { const ng = ngDebug(); const widgetRole = widget.getAttribute('role'); const itemRole = item.getAttribute('role'); @@ -176,12 +199,31 @@ function sharesRovingStop(widget: Element, item: Element, isVisible?: (el: Eleme } } if (!host) return false; - return [...host.querySelectorAll(`[role="${widgetRole}"] [role="${itemRole}"]`)].some( - (other) => - other !== item && - other.parentElement?.closest(COMPOSITE_SELECTOR)?.getAttribute('role') === widgetRole && - isTabbable(other, isVisible), - ); + for (const other of host.querySelectorAll(`[role="${widgetRole}"] [role="${itemRole}"]`)) { + const otherWidget = other.parentElement?.closest(COMPOSITE_SELECTOR); + if (!otherWidget || otherWidget === widget || otherWidget.getAttribute('role') !== widgetRole) continue; + if (!isTabbable(other, isVisible)) continue; + if (repeatedSiblings(widget, otherWidget, host)) return true; + } + return false; +} + +/** + * True when `a` and `b` sit in sibling branches of the same kind: below their + * lowest common ancestor (inside `bound`), the branch holding each is the same + * tag and they share at least one class. + */ +function repeatedSiblings(a: Element, b: Element, bound: Element): boolean { + for (let branchA: Element | null = a; branchA && branchA !== bound; branchA = branchA.parentElement) { + const parent = branchA.parentElement; + if (!parent || !parent.contains(b)) continue; + let branchB: Element | null = b; + while (branchB && branchB.parentElement !== parent) branchB = branchB.parentElement; + if (!branchB || branchB === branchA || branchB.tagName !== branchA.tagName) return false; + const classesB = branchB.classList; + return [...branchA.classList].some((c) => classesB.contains(c)); + } + return false; } function isComponentHost(el: Element, ng: ReturnType): boolean { @@ -208,11 +250,12 @@ function partOfKeyboardOperatedWidget( element: Element, isVisible?: (el: Element) => boolean, keyCache?: Map, + sharedCache?: Map, ): boolean { const item = element.parentElement?.closest(ITEM_SELECTOR); if (item?.parentElement?.closest(COMPOSITE_SELECTOR)) { // 'no-keys' too: the item itself is reported, so its parts needn't be. - if (isTabbable(item, isVisible) || reachedWithinWidget(item, isVisible, keyCache)) return true; + if (isTabbable(item, isVisible) || reachedWithinWidget(item, isVisible, keyCache, sharedCache)) return true; } for (let el = element.parentElement; el; el = el.parentElement) { if (!isTabbable(el, isVisible)) continue; @@ -259,6 +302,7 @@ export function scanKeyboard( const isVisible = options.isVisible; const findings: A11yFinding[] = []; const keyCache = new Map(); + const sharedCache = new Map(); const make = ( element: Element, @@ -310,8 +354,8 @@ export function scanKeyboard( if (!focusable) { // 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 (!interactiveByRole && partOfKeyboardOperatedWidget(element, isVisible, keyCache, sharedCache)) continue; + const within = interactiveByRole ? reachedWithinWidget(element, isVisible, keyCache, sharedCache) : false; if (within === 'reached') continue; if (within === 'no-keys') { findings.push( diff --git a/src/testing/keyboard-scan.spec.ts b/src/testing/keyboard-scan.spec.ts index 0364618..e06aebd 100644 --- a/src/testing/keyboard-scan.spec.ts +++ b/src/testing/keyboard-scan.spec.ts @@ -202,10 +202,10 @@ describe('scanKeyboard', () => { const board = (outside = '') => fixture(`
-
+
A
B
-
+
C
${outside}`); @@ -231,6 +231,17 @@ describe('scanKeyboard', () => { withNg(keysOn(host), new Map(), () => expect(unreachable(host)).toEqual(['div#c']), new Set([host.querySelector('#board')!])); }); + it('is not vouched for by an unrelated working listbox elsewhere in the same component', () => { + // Not repeated siblings: the working one is in a
, the + // broken one in an