Repository navigation
focus_on: check who has the focus after a key that changes nothing visible - #1579
Conversation
…sible A Tab onto an icon button, or an ArrowDown that highlights an option, gave its step no postcondition, and a strict policy refused it as vacuous. The recorder now keeps the focused element's identity around each key, and the compiler adds a focus_on postcondition when that element has a specific identity no other element shares at teach time: a data-testid, an id, or a role with a non-empty accessible name. At replay the focused element must have that identity and be the only one with it, and, for an option highlight, an option must be active (aria-activedescendant or aria-selected). Anything weaker asserts nothing, never passes, and still counts as vacuous. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AI review (coordinator)Verdict: changes requested. Not queued for merge. Three probes in headless Chromium show I reviewed head Blocking
Should fix (rule 3 as agreed)
Non-blocking
Checked and sound
|
…ope active options Review fixes. The focused element's frame chain is recorded and must match at replay, so a button with the same testid in an iframe no longer passes. Focus inside a shadow root (or on a custom element that may hide a closed one) is unreadable: no check is mined and any check fails there. A framework-generated id (React useId, Radix, Headless UI, MUI, Ember and similar) is skipped for role and name. An option is active only as a role=option inside the listbox the control names by aria-controls; aria-owns is no longer read. Each hole has a real-Chromium test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AI review (coordinator), re-review of
|
| Case | Before | Now |
|---|---|---|
| A: iframe with the same testid; focus in the iframe | held=True |
held=False (frame_path ["iframe:nth-of-type(1)"] is not ()); top button still True |
| B: duplicate ids | sound | sound (falls back to role and name; a duplicate at replay halts) |
C: :r2: id shift |
wrong element True, right element False |
mined as role_name button "Save": wrong element False, right element True |
| D: open shadow root | host id mined, True on another inner button |
nothing mined (unreadable) |
E: activedescendant to another listbox, to a non-option, aria-owns |
all True |
all False |
| F: script moves focus 400 ms later | True |
True (non-blocking; every polled check has this) |
Blocking
G. React 19.2+ useId ids (_r_N_, and _R_N_ from server rendering) aren't on the generated-id list. React 19.2 replaced «r» with _r_ as the default useId format, so most current React apps use it. This is the same case as finding 3:
- Teach page:
_r_1_Cancel,_r_2_Save. Teach (focus Save): mined asfocus_by=id, _r_2_. - Replay page, with one extra button rendered first:
- Focus on the wrong element, Cancel (now
_r_2_):held=True. - Focus on the right element, Save (now
_r_3_):held=False.
- Focus on the wrong element, Cancel (now
The id list will keep missing formats. These ids are still mined as focus_by=id today: react-aria-:r1: (the :r…: pattern only matches the whole id), mat-input-0 and cdk-overlay-3 (Angular Material and CDK; the numeric suffix is under 4 digits), and rc_select_0 (Ant Design).
Suggested fix: keep the id list, and close the whole class with one structural change. When focus_by is testid or id and the element had a role and a non-empty name at teach time, store them too, and require all of them to match at replay. A shifted id that lands on a different control then fails, because that control's role and name differ. At minimum, add _r_[0-9a-z]+_ and match :r…:, «r…» and _r_…_ anywhere in the id, not only as the whole id.
Non-blocking
-
A closed shadow root on a built-in element still identifies the host. Example:
<div id=w tabindex=-1>withattachShadow({mode:'closed'})holding "One" and "Delete".el.shadowRootis null for a closed root anddivhas no hyphen, so the check is mined asid=wand passes with focus on "Delete". This setup is rare. The structural fix above also closes it, because the host div has no role or name. -
Treating custom elements as unreadable removes checks on common design systems. It fails closed, so it doesn't block. Examples:
- Angular Material hosts that take focus themselves without any shadow DOM (
mat-step-header,mat-chip-option,mat-list-option,mat-tree-node): mymat-step-header role=tab aria-label="Step 2"probe mined nothing. - Shoelace, Web Awesome, Material Web, Ionic, Spectrum, Fluent and UI5 buttons with
delegatesFocus:sl-buttonmined nothing.
A narrower rule would keep those checks: treat the element as unreadable only when it has
el.shadowRootorcustomElements.get(el.localName)is defined. Angular components aren't registered custom elements, so they would stay readable. - Angular Material hosts that take focus themselves without any shadow DOM (
To merge: fix G (ideally the structural fix), then queue.
…er shadow rule Round 2 of the review. A testid or id check now also records the element's role and accessible name when it had them, and replay requires both, so an id that moves to another element can't pass whatever framework generated it. The generated-id patterns stay as a hint and match anywhere in the id (React's :r…:, «r…» and 19.2's _r_…_; react-aria, Angular Material and CDK, rc-select). Focus is unreadable only on an element with a shadow root or a registered custom element, so unregistered framework components keep their checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AI review (coordinator), re-review of
|
| Case | Result |
|---|---|
| A: iframe with the same testid; focus in the iframe | held=False; the top button True |
| B: duplicate ids; unique at teach time, duplicated at replay | falls back to role and name; a duplicate at replay halts |
C: :r2: shift |
role_name: the wrong element False, the right one True |
| D: open shadow root | nothing mined |
E: activedescendant to another listbox, to a non-option, aria-owns |
all False |
| F: script moves focus 400 ms later | True (non-blocking; every polled check has this) |
G: React 19.2 _r_N_ shift |
role_name: the wrong element False, the right one True |
H: mat-input-0, rc_select_0, react-aria-:r1:, cdk-overlay-3, _R_1_ |
all fall back to role_name |
J: Angular-style mat-step-header (unregistered, no shadow) |
mined as role_name tab "Step 2", so readable again |
K: Shoelace-style delegatesFocus button |
nothing mined (fails closed) |
| N: registered custom element, no shadow | nothing mined (fails closed) |
Non-blocking
-
An accessible name that changes at run time now halts. Example: a
data-testid=savebutton named "Save (3)" at teach time and "Save (4)" at replay givesheld=False. Badge counts, "Saved 2 min ago" and similar names are affected. This fails closed. If it shows up often, the name check could compare a normalized name, with digits and parenthesized counts removed, when a testid is also bound. -
An id-only check still remains when the element has no role or name. Binding role and name doesn't cover these cases, and my earlier note that it would close case I was wrong.
- I (closed shadow root on a plain
<div id=w>): mined asid=w; it passes with focus on the inner "Delete" button. - M (an unlisted generated id on an unnamed
<div tabindex=0>, shifted by one render): passes on the wrong div.
Both need an unnamed focusable element, and rule 1 allows a bare id. A follow-up could stop creating id-only checks for elements that have no role and name.
- I (closed shadow root on a plain
-
Inside an iframe, role and name aren't read. An id or testid check there has no role-and-name binding; only the frame path and the id list protect it.
The change only adds a check: is_vacuous stays strict for a FOCUS_ON without a specific identity, nothing else is relaxed, and the VLM rescue can't apply to it.
Bumps openadapt-flow to 2.4.1 (pyproject, __version__, the editable root in uv.lock) and writes the 2.4.1 changelog for everything user-visible merged since openadapt-flow-v2.4.0: #1593 (Windows config pin), #1594 (product code no longer imports the benchmark package), #1569 (div, grid and card worklist rows), #1539 (fault gate per class) and #1579/#1595 (focus_on). Regenerates llms-full.txt. Merging this publishes 2.4.1 through release-package.yml once main's Packages run for this commit passes.
Bug 22 from the second-installation work (#1548).
Problem
Medplum's form has controls with nothing readable beside them, so #1548 teaches some of its fields by keyboard. Two of those keys change nothing the compiler can see:
The compiler gave those steps no postcondition, and
clinical-writerefused the program (prohibit_vacuous_postconditions,max_unverified_steps).Change
A structural postcondition,
focus_on, alongsideurl_changedandtitle_changed. It is mined only for key steps that have no visual postcondition.keyorhotkeyevent, it keeps the focused element's identity (focus_before,focus_after).PlaywrightBackend.focus_identity()reads:data-testidand theid, each with the count of elements sharing it in the element's document;aria-activedescendantnames an element, or the listbox it controls has anaria-selectedoption.focus_ononly when the key moved the focus or made an option active, and the element has a specific identity no other element shared at teach time. That is a testid, then an id, then a role with a non-empty name. A tag alone, a role without a name, or a shared identity gets no postcondition, and the step stays vacuous.focus_active_option, an option must also be active. An unreadable focus, nothing focused, a different element, a second element with the same identity, or a postcondition with no specific identity all fail.is_vacuouscounts afocus_onas asserting something only when it names a specific element (focus_identity_is_specific). A degenerate one is still refused byprohibit_vacuous_postconditions.lint_param_leakagechecks the focus name and value for demonstrated parameter values.focus_onout of revalidation, as it doeslocal_edit: it is relative to the moment the key was pressed.Tests
tests/test_focus_on_postcondition.py(18 tests):focus_on. A tag only, a role with no name, a shared role and name, a shared testid, or a key that moved nothing gets no postcondition. An option highlighted on the same control is markedfocus_active_option.focus_onmakes a key step non-vacuous. One without a specific identity stays vacuous.Local runs:
test_record_compile_replay_emit_roundtripandtest_installer_starts_with_an_empty_allow_list_and_default_reboot_policy) fail the same way on a branch without this change: a subprocess can't import the package in this environment.Not yet run against Medplum. That comes at the next fixture lease, on #1548.
🤖 Generated with Claude Code