refactor: derive INTERACTIVE_ROLES from aria-query taxonomy (+ widen menu-pattern exception)#27
refactor: derive INTERACTIVE_ROLES from aria-query taxonomy (+ widen menu-pattern exception)#27
Conversation
🏎️ Benchmark Comparison
Full mitata output |
Import INTERACTIVE_ROLES and COMPOSITE_WIDGET_CHILDREN from the shared lib/utils/interactive-roles.js util (introduced in #27 — byte-identical copy here so either PR can land first without conflict). Drop the hardcoded 19-role set previously duplicated inline in each rule. Behavior changes: - ARIA widget role set expands from 19 to 35 roles — picks up menubar, menu, listbox, tree, tablist, grid, treegrid, radiogroup, alertdialog, progressbar, and other widget-descended roles in aria-query's taxonomy that the hardcoded list missed. - tooltip is no longer treated as interactive. Per WAI-ARIA 1.2 §5.3.3, tooltip is a document-structure role, not a widget. #27's util reflects this (tooltip explicitly excluded). Old <div role="tooltip" onclick> test moves from valid to invalid — spec-correct. - Composite-widget nesting exception expanded via COMPOSITE_WIDGET_CHILDREN. Canonical APG patterns (<ul role="menubar"><li role="menuitem">..., <ul role="listbox"><li role="option">..., grid/row/gridcell, treegrid, radiogroup/radio) no longer flag as nested-interactive. Previously the rule only handled menuitem-in-menuitem explicitly. Pairs with #37's HTML-content-model authority split to complete the two-authority architecture: HTML §3.2.5.2.7 via html-interactive-content, ARIA widget taxonomy via interactive-roles. Each util cites one authority honestly; rules compose both.
There was a problem hiding this comment.
Pull request overview
Refactors accessibility role handling by centralizing INTERACTIVE_ROLES derivation from aria-query and expanding template-no-nested-interactive to allow canonical composite-widget role hierarchies (per requiredOwnedElements, plus an APG submenu exception).
Changes:
- Introduce
lib/utils/interactive-roles.jsto deriveINTERACTIVE_ROLESfromaria-query(+ manualtoolbar, excludetooltip) and to computeCOMPOSITE_WIDGET_CHILDREN. - Update
template-no-invalid-interactiveandtemplate-no-nested-interactiveto consume the shared role utilities. - Add/adjust tests to validate role derivation and composite-widget nesting exceptions (including
tooltipnow being treated as non-interactive).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/utils/interactive-roles.js | New shared derivation for interactive roles and composite-widget child/descendant allowances. |
| lib/rules/template-no-nested-interactive.js | Switch to derived role sets and broaden nesting exception to composite-widget patterns (+ submenu rule). |
| lib/rules/template-no-invalid-interactive.js | Switch to derived interactive role set; tooltip no longer treated as interactive. |
| tests/lib/utils/interactive-roles-test.js | New tests for INTERACTIVE_ROLES and COMPOSITE_WIDGET_CHILDREN derivation and drift detection. |
| tests/lib/rules/template-no-nested-interactive.js | Add valid cases for canonical composite-widget hierarchies. |
| tests/lib/rules/template-no-invalid-interactive.js | Remove tooltip from valid widget-role cases; add invalid case asserting tooltip is non-interactive. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… pin gridcell/row/columnheader/rowheader)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… pin gridcell/row/columnheader/rowheader)
dff1d91 to
9c49f26
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
11787d4 to
b7cfff0
Compare
Import INTERACTIVE_ROLES and COMPOSITE_WIDGET_CHILDREN from the shared lib/utils/interactive-roles.js util (introduced in #27 — byte-identical copy here so either PR can land first without conflict). Drop the hardcoded 19-role set previously duplicated inline in each rule. Behavior changes: - ARIA widget role set expands from 19 to 35 roles — picks up menubar, menu, listbox, tree, tablist, grid, treegrid, radiogroup, alertdialog, progressbar, and other widget-descended roles in aria-query's taxonomy that the hardcoded list missed. - tooltip is no longer treated as interactive. Per WAI-ARIA 1.2 §5.3.3, tooltip is a document-structure role, not a widget. #27's util reflects this (tooltip explicitly excluded). Old <div role="tooltip" onclick> test moves from valid to invalid — spec-correct. - Composite-widget nesting exception expanded via COMPOSITE_WIDGET_CHILDREN. Canonical APG patterns (<ul role="menubar"><li role="menuitem">..., <ul role="listbox"><li role="option">..., grid/row/gridcell, treegrid, radiogroup/radio) no longer flag as nested-interactive. Previously the rule only handled menuitem-in-menuitem explicitly. Pairs with #37's HTML-content-model authority split to complete the two-authority architecture: HTML §3.2.5.2.7 via html-interactive-content, ARIA widget taxonomy via interactive-roles. Each util cites one authority honestly; rules compose both.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Consolidates two hand-maintained INTERACTIVE_ROLES sets (in template-no-invalid-interactive and template-no-nested-interactive) into a single derivation sourced from aria-query's role taxonomy. lib/utils/interactive-roles.js filters concrete roles whose superClass chain includes widget / command / composite / input / range, then adds tooltip and toolbar explicitly (same rationale as jsx-a11y: ARIA 1.2 categorizes tooltip as a widget but aria-query's superClass chain doesn't reflect that; toolbar supports aria-activedescendant and is widget-like in practice). Behavioral change: Adds 18 roles to the interactive set: columnheader, doc-backlink, doc-biblioref, doc-glossref, doc-noteref, grid, listbox, menu, menubar, meter, progressbar, radiogroup, row, rowheader, tablist, toolbar, tree, treegrid. These were silently treated as non-interactive — a composite widget (role="tree") could contain nested interactive elements without being flagged; an <a href> inside a role="menu" was accepted but nested-interactive per ARIA. No removals — tooltip was in the hand list and is preserved explicitly. Follow-up: template-no-nested-interactive's menu-pattern exception broadened from "menuitem inside menuitem" to "menu patterns per WAI-ARIA" — menuitem/menuitemcheckbox/menuitemradio inside menu/menubar/menuitem, and menu inside menuitem (for submenus), are the standard menu hierarchy.
Our earlier derivation was broader (widget|command|composite|input|range
ancestors). jsx-a11y and lit-a11y both use the narrower 'widget ancestor'
check. The only role our broader check added was 'meter' — spec-readonly
and not interactive in peer-plugin terms.
Narrow to match peer behavior exactly:
- derivation: def.superClass.some(chain => chain.includes('widget'))
- + 'toolbar' (doesn't descend from widget but supports activedescendant)
- + 'tooltip' (ARIA 1.2 categorization is ambiguous; our existing tests
treat it as interactive)
No test changes — the new set is a subset of the previous except for
meter (not covered by any existing test).
Per WAI-ARIA 1.2 §5.3.3 — Document Structure Roles: https://www.w3.org/TR/wai-aria-1.2/#tooltip Tooltip is explicitly listed under document-structure roles (alongside img, note, paragraph, etc.), not widget roles. The spec notes: 'Document structures are not usually interactive.' jsx-a11y and lit-a11y agree — neither adds tooltip to their interactive- role set (both add only toolbar). The one test case expecting tooltip to be treated as interactive moves accordingly. aria-query's superClass for tooltip is structure/section, which the narrow widget-ancestor filter correctly excludes.
…erns
Deriving INTERACTIVE_ROLES from aria-query's widget taxonomy added roles
(`listbox`, `tablist`, `tree`, `treegrid`, `grid`, `radiogroup`, `row`,
and their children) to `template-no-nested-interactive`'s interactive
set. Without a corresponding exception, canonical WAI-ARIA APG composite
widgets would trip the rule:
<div role="listbox"><div role="option">A</div></div>
<div role="tablist"><div role="tab">…</div></div>
<div role="tree"><div role="treeitem">…</div></div>
<div role="grid"><div role="row"><div role="gridcell">…</div></div></div>
<div role="radiogroup"><div role="radio">…</div></div>
Widen the existing menu-pattern exception into a general composite-widget
exception driven by aria-query's `requiredOwnedElements` data. For each
parent role, inherit `requiredOwnedElements` from ancestor roles in
`superClass` (so `treegrid` picks up both `row` via `grid` and `treeitem`
via `tree`) and close transitively over intermediate composite roles (so
`grid → row → gridcell` is allowed at arbitrary depth in the hierarchy).
The submenu pattern `menuitem → menu`, which aria-query does not express
via `requiredOwnedElements`, is kept as an explicit special case.
Also add an invalid-interactive test for `role="tooltip"` — now that
tooltip is no longer in INTERACTIVE_ROLES (per WAI-ARIA 1.2 §5.3.3, it
is a Document Structure role), a click handler on a tooltip is an
invalid interactive handler on a non-interactive element.
Covers the full util surface: - Canonical widget roles (button, link, combobox, etc.) — in set - Composite-widget containers (listbox, grid, tablist, tree, treegrid) — in set - toolbar manual override — in set (documented exception) - tooltip exclusion — NOT in set (WAI-ARIA 1.2 §5.3.3) - progressbar included (documented divergence from lit-a11y) - Set-size pin (35 roles) — surfaces aria-query taxonomy drift as test failure - COMPOSITE_WIDGET_CHILDREN sanity (listbox/option, grid/gridcell transitivity, treegrid/row+treeitem superClass inheritance, radiogroup/radio, submenu) Also extend the util JSDoc to document progressbar inclusion vs lit-a11y's readonly-value-based exclusion — previously only noted in the PR body.
…ILDREN expansion (Copilot review)
…tails> body panel During the rebase that introduced isSummaryFirstChildOfDetails, the broader isAllowedDetailsChild check was lost. The disclosed panel of <details> is flow content — interactive children after <summary> are valid and must not be flagged. Restores isAllowedDetailsChild as a wrapper that delegates the <summary>-position check to isSummaryFirstChildOfDetails.
…HtmlInteractiveContent)
59b93ec to
ebdc2c8
Compare
|
Closing in favour of test/composite-widget-nesting. The INTERACTIVE_ROLES derivation, COMPOSITE_WIDGET_CHILDREN map, and the util test are already on master (landed via piecemeal commits). The only missing piece — composite-widget hierarchy valid cases in the rule test — will be added via a clean branch from current master. The remaining diff on this PR reverts ember-cli#2748 (deletes html-interactive-content.js) and drops 'use strict', both of which would be regressions. |
Note
This is part of a series where Claude has audited
eslint-plugin-emberagainst jsx-a11y, vuejs-accessibility, angular-eslint, lit-a11y and html-validate,ember-template-lint, and the HTML and WCAG specs.Consolidates two hand-maintained
INTERACTIVE_ROLESsets (intemplate-no-invalid-interactiveandtemplate-no-nested-interactive) into a single derivation sourced fromaria-query's role taxonomy, and widens the composite-widget nesting exception so canonical WAI-ARIA APG patterns are not flagged.Derivation
New
lib/utils/interactive-roles.js— one shared set used by both rules:widgetin anysuperClasschain per aria-query's taxonomy.toolbar— it supportsaria-activedescendantand is widget-like in practice but does not descend fromwidgetin aria-query's data. Matches jsx-a11y's same manual addition.tooltipis not included. Per WAI-ARIA 1.2 §5.3.3 "Document Structure Roles",tooltipis a document-structure role, not a widget; the spec says "The following roles describe structures that organize content in a page. Document structures are not usually interactive." aria-query confirms:tooltip.superClassis[['roletype', 'structure', 'section']]— nowidget.Matches jsx-a11y's
isInteractiveRolederivation verbatim. (Note: lit-a11y uses the same derivation but explicitly excludesprogressbarbecause its value is effectively readonly.)Final set: 35 roles.
Composite-widget nesting exception
Previously
template-no-nested-interactivehard-coded a single exception:menuitem-inside-menuitem. This PR widens it using a derivedCOMPOSITE_WIDGET_CHILDRENmap driven by aria-query'srequiredOwnedElements(a documented ARIA concept — WAI-ARIA 1.2 §5.2.6 Required Owned Elements), withsuperClassinheritance applied so treegrid inherits from both grid and tree.Note: the
superClass-based inheritance step is a composition of two spec facts (the superClass relation + requiredOwnedElements on ancestors), not a verbatim spec rule — it's a reasonable inference that matches APG patterns, but strictly speaking derived rather than directly cited.Plus an explicit
MENUITEM_ROLES → menurule for the submenu direction (documented in APG's Menu pattern; aria-query does not encode this viarequiredOwnedElements).Relation to surrounding branches
lib/utils/is-native-element.js(+83 lines) has no consumer on this branch. It's included as a byte-identical copy of the canonical version owned by refactor: extract isNativeElement util (fix component-vs-HTML-tag misclassification) #50 (fix/is-component-invocation-util) so that whichever of refactor: derive INTERACTIVE_ROLES from aria-query taxonomy (+ widen menu-pattern exception) #27 / refactor: extract isNativeElement util (fix component-vs-HTML-tag misclassification) #50 / sibling branches merges first, the rest don't pick up a conflict on this file. Happy to drop it from this PR and defer to refactor: extract isNativeElement util (fix component-vs-HTML-tag misclassification) #50 if review prefers.<details>handling comes from master via rebase. The rule now usesisAllowedDetailsChild(introduced by BUGFIX: false positive: interactive flow content inside <details> ember-cli/eslint-plugin-ember#2713 on master — summary-first-child carve-out plus flow content in the disclosed panel). No change to that helper in this PR; called out because a reviewer looking at the pre-rebase diff will see the olderisSummaryFirstChildOfDetailsdisappear.