Merge branch 'worktree-agent-a466c08a4014dbcbe' into rustify (I4: AccessKit names)
This commit is contained in:
commit
32a5256a0d
22 files changed
+2251
-22
No files matched your search
@@ -42,7 +42,7 @@ session spending an afternoon on them again.
|
||||
below), **E3 (the Kotlin/Java shell over a JNI bridge into Rust, both
|
||||
pass conditions proved on the emulator — see its own box)**, I0a, I0b
|
||||
(iris builds on a pinned nightly and runs), I1 (parley + glyph atlas),
|
||||
I2 (iris on android-view).
|
||||
I2 (iris on android-view), I3 (`iris::widget::List`).
|
||||
- **E3 done, 2026-09-05, and unlike E1/E2 it is committed to this repo**
|
||||
(`android-shell/` — a JNI-bridge crate on `client-core` — plus a new
|
||||
Gradle module `app/shellApp/`, left deliberately separate from
|
||||
@@ -61,6 +61,20 @@ session spending an afternoon on them again.
|
||||
box for the full account, the exact commands, and what was deliberately
|
||||
cut (attachment uploads, a session picker, the on-screen/banner
|
||||
suppression — all pending E4's screen).
|
||||
- **I4 — accessibility names via AccessKit: host half done and verified
|
||||
2026-09-05, ticked in the box below.** `iris_core::ui::access::AccessTree`
|
||||
builds one flat AccessKit tree from `Widgets::named()` (a side set only
|
||||
`.label()` populates, so an unnamed widget costs this nothing), pushed
|
||||
through `accesskit_winit` on the desktop and `accesskit_android` on
|
||||
Android, updated only when a name/role/bounds actually changes (a
|
||||
counter confirms it: 1 rebuild on first draw, 0 across an unchanged
|
||||
frame, 1 more after a real move). E1's detach-abort mitigation is
|
||||
carried (`android/access.rs`'s `raise_if_enabled`). Every check that
|
||||
doesn't need the emulator is clean — see I4's own box for the exact
|
||||
numbers. **What's left**: the emulator itself is held by another session
|
||||
this pass, so `ui-trace record --do "tap 'pad'"` against
|
||||
`iris-android-app`'s tabs screen (which now has five named buttons) has
|
||||
not been run for real yet — exact commands at the bottom of I4's box.
|
||||
- **E2 done, 2026-09-05, and its headline finding changes what "decide
|
||||
from the measurements" (recommendation item 3) can mean right now.**
|
||||
Built a real transcript screen (`~/src/android-view/e2-transcript`,
|
||||
@@ -1772,9 +1786,144 @@ silently on real hardware.
|
||||
session screen, i.e. most of I5's work) rather than something this
|
||||
box's scope could finish alone -- recorded here rather than left
|
||||
silently undone.
|
||||
- [ ] **I4 — accessibility names via AccessKit.** Every control carries a
|
||||
name; `ui-trace` can find and tap it by label. Pass: `bench-lib.sh`'s
|
||||
tap-by-name works against the iris screen unchanged.
|
||||
- [x] **I4 — accessibility names via AccessKit, host half done and verified
|
||||
2026-09-05; the emulator half is the one step left, named at the
|
||||
bottom of this box.** Built `iris_core::ui::access::AccessTree`
|
||||
(`iris/core/src/ui/access.rs`) -- one flat AccessKit tree, a synthetic
|
||||
`Role::Window` root with every **named** widget as a direct child.
|
||||
Deliberately flat rather than mirroring iris's real widget nesting:
|
||||
nothing upstream of a named leaf needs a node, since a screen
|
||||
reader's traversal (and uiautomator's tap-by-name, this box's own
|
||||
pass condition) works from each node's on-screen bounds, not from
|
||||
tree structure -- and mirroring the real tree would rebuild
|
||||
intermediate nodes on every resize of any container above a named
|
||||
widget, which is most frames.
|
||||
|
||||
**Modular the way input's sense registry is.** `Widgets` gained one
|
||||
`HashSet<WidgetId>` (`named`), populated only by `.label()`/
|
||||
`set_label` and drained by `free_next` (the same removal path a
|
||||
freed id already went through -- no second bookkeeping call added
|
||||
anywhere). `AccessTree::update` walks `widgets.named()` directly,
|
||||
never the full widget arena, so a widget nobody named costs this
|
||||
subsystem nothing -- not a visit, not a branch. Roles come from a
|
||||
new `Widget::access_role(&self) -> accesskit::Role` trait method,
|
||||
default `Unknown`; the one override so far is `TextEdit` ->
|
||||
`TextInput`/`MultilineTextInput` by `EditMode`. Bounds come from
|
||||
`UiRenderState::window_region`, which sits on `resolved_region`'s
|
||||
move-chain walk -- so a widget moved via `Offset`/`Scroll` (never
|
||||
redrawn from scratch) still reports where it actually ended up; see
|
||||
`bounds_follow_a_moved_widget_and_updates_stay_incremental` below.
|
||||
|
||||
**Incremental, not per-frame.** `AccessTree` keeps the last
|
||||
`HashMap<WidgetId, Entry>` (name, role, bounds) it sent and only
|
||||
returns a new `TreeUpdate` -- and only then bumps its `rebuilds`
|
||||
counter, `take_rebuilds()`'s the AccessKit twin of
|
||||
`UiRenderState::take_counters` -- when that set actually differs.
|
||||
Confirmed by `bounds_follow_a_moved_widget_and_updates_stay_incremental`
|
||||
(`iris/src/access_tests.rs`): 1 rebuild on the first draw, 0 across an
|
||||
unchanged frame, 1 more after a real move, regardless of how many
|
||||
other widgets are on screen.
|
||||
|
||||
**`SlotId::as_u64`** (`core/src/util/slot.rs`) encodes a `WidgetId`
|
||||
into accesskit's flat `NodeId(u64)`, offset by one so a real widget
|
||||
never collides with the reserved window node (`NodeId(0)`).
|
||||
|
||||
**Pushed through two backends, each behind an inert action/activation
|
||||
handler** -- see below for why inert is correct, not incomplete.
|
||||
`default/access.rs` (winit): `accesskit_winit::Adapter`, built in
|
||||
`DefaultApp::new` with the window created hidden
|
||||
(`with_visible(false)`) and shown only after the adapter exists,
|
||||
which is what that constructor requires. `process_event` runs on
|
||||
every `WindowEvent`; `update_if_active` runs once per
|
||||
`RedrawRequested`, after `render.update()` so bounds reflect the
|
||||
frame just drawn. `android/access.rs` (android-view):
|
||||
`accesskit_android::Adapter` on `AndroidUiState`, `IrisViewPeer` now
|
||||
implements `AccessibilityNodeProvider`
|
||||
(`create_accessibility_node_info`/`find_focus`/`perform_action`), and
|
||||
`render()` (now taking `&mut CallbackCtx`, needed for the JNI handle
|
||||
any `raise` requires) pushes the same `AccessTree::update` after
|
||||
every draw.
|
||||
|
||||
**Why the `ActionHandler`s are empty, not a placeholder for later
|
||||
work**: AGENTS.md's own "Driving the UI" section says it plainly --
|
||||
`ui-trace record --do "tap 'Save'"` resolves the label against the
|
||||
screen and performs a **real touch at that node's bounds**, the same
|
||||
as a person's finger. It does not call into AccessKit's action
|
||||
system at all. So once `AccessTree` reports correct bounds, the
|
||||
ordinary pointer path (already built, already tested) is what
|
||||
answers the tap -- there is nothing for `do_action` to do for this
|
||||
pass condition specifically. A future real screen reader's own
|
||||
double-tap-to-activate gesture works the same way, for the same
|
||||
reason. If iris ever needs to answer an AccessKit `Action::Click`
|
||||
injected without a matching touch (e.g. a switch-access scanner),
|
||||
that is new scope, not a gap in this box.
|
||||
|
||||
**E1's abort mitigation, carried.** `android/access.rs`'s
|
||||
`raise_if_enabled` is the one place `QueuedEvents::raise` may be
|
||||
called: it asks `AccessibilityManager.isEnabled()` (a `getSystemService`
|
||||
JNI call, since android-view has no ready-made wrapper) immediately
|
||||
before every `raise` and drops the events instead when the answer is
|
||||
no. Every call site (`render`'s per-frame push, `perform_action`)
|
||||
goes through it, and each pushes it as a *deferred* callback exactly
|
||||
like android-view's own demo, so it runs after the current JNI
|
||||
callback has released whatever it's holding -- `raise`'s own
|
||||
documented requirement. Not independently re-triggered on this
|
||||
pass (that needs the emulator, see below); the mitigation is coded
|
||||
to the exact mechanism E1 diagnosed (`sendAccessibilityEvent`
|
||||
throwing when accessibility is off) rather than to the symptom, so
|
||||
there is no reason to expect it behaves differently here than it did
|
||||
there.
|
||||
|
||||
**Verified, 2026-09-05, host only.**
|
||||
`cargo fmt --all -- --check`, `cargo build --workspace --all-targets`,
|
||||
`cargo clippy --all-targets` (both plain and `--all-targets`) clean;
|
||||
`cargo test --workspace` -- 28 tests in `iris/`, three of them new
|
||||
(`access_tests::a_named_widget_reaches_the_tree_with_its_role_and_bounds`,
|
||||
`::a_widget_with_no_label_never_reaches_the_tree`,
|
||||
`::bounds_follow_a_moved_widget_and_updates_stay_incremental`).
|
||||
`cd iris/android-app && cargo ndk -t x86_64 -P 26 build` and
|
||||
`... clippy` clean for both `iris` (with the android module) and
|
||||
`iris-android-app`, same shape as I2/I3's checks.
|
||||
`iris/run-headless.sh tabs --shot /tmp/iris_i4_tabs.png --seconds 4`
|
||||
still renders -- **27266 bytes, byte-for-byte identical to I2's own
|
||||
post-fix screenshot** -- confirming the hidden-window-then-adapter
|
||||
change to `DefaultApp::new` cost nothing visible. `tabs-ui`'s five
|
||||
switch buttons (`tabs-ui/src/lib.rs`) now carry `.label()`s matching
|
||||
their on-screen text ("pad", "span", "image span", "text layout",
|
||||
"text edit scroll") -- both so the desktop run above exercises a
|
||||
non-empty tree and so the emulator step below has real names to tap.
|
||||
Not independently checked on this pass: whether `accesskit_winit`'s
|
||||
Linux path (AT-SPI, via `accesskit_unix`) actually reaches a real
|
||||
assistive-technology client on this VM's headless sway -- there is
|
||||
no AT-SPI registry running here, so `default/access.rs`'s handlers
|
||||
are exercised as inert code paths (built, called, no panic) rather
|
||||
than confirmed end-to-end the way the emulator step below confirms
|
||||
the Android path.
|
||||
|
||||
**What remains -- the one check that needs the emulator, held by
|
||||
another session during this pass.** `iris-android-app`'s tabs screen
|
||||
has never been driven by `ui-trace` for real; everything above is
|
||||
"builds, runs, produces the right data" on the host. Once the
|
||||
emulator is free:
|
||||
|
||||
cd iris/android-app && cargo ndk -t x86_64 -P 26 -o app/src/main/jniLibs/ build --release && gradle :app:assembleDebug
|
||||
adb install -r app/build/outputs/apk/debug/app-debug.apk
|
||||
# launch iris-android-app on the emulator, then:
|
||||
ui-trace record --do "tap 'pad'"
|
||||
ui-trace record --do "tap 'span'"
|
||||
ui-trace record --do "tap 'image span'"
|
||||
ui-trace record --do "tap 'text layout'"
|
||||
ui-trace record --do "tap 'text edit scroll'"
|
||||
|
||||
Pass condition: each tap resolves (uiautomator finds a node with
|
||||
that exact label) and switches `main`'s visible pane the way a
|
||||
direct touch on that button already does -- i.e. `bench-lib.sh`'s
|
||||
tap-by-name mechanism, unmodified, driving the iris screen instead
|
||||
of the Compose one. Also worth checking while the emulator is up,
|
||||
since E1 found it exactly this way: run a second `ui-trace record`
|
||||
immediately after the first (attach, detach, attach again) and
|
||||
confirm the process is still alive afterward -- the detach-abort
|
||||
this box's mitigation exists for.
|
||||
- [ ] **I5 — the transcript screen in iris.** E2's pass conditions, all
|
||||
seven behaviours, against the sandbox with `--delay`. This is the
|
||||
point the decision in the recommendation is made at.
|
||||
|
||||
Reference in new issue
Block a user