fix(ios): a text field under the software keyboard reports focused in the snapshot - #2925
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
One or more issues must be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (2)
What changed in this PR
Updates iOS snapshot focus detection to include software-keyboard focus.
Changes:
- Combines
hasFocusandhasKeyboardFocusin XCTest snapshots. - Requests and merges keyboard focus in the private AX bridge.
- Extends the AX fixture shape.
| File | Description |
|---|---|
| apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+AXRecoveryConformanceTests.swift | Updated as part of this pull request. |
| apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotAcquisition.swift | Updated as part of this pull request. |
| apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerAXSnapshotBridge.m | Updated as part of this pull request. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Reviewed at 8a3ae74. The fix itself is a plausible read of The live validation in the PR description ( Smoke Tests builds and selectively runs the iOS runner XCTest regressions against a booted simulator, which is the mechanism that would exercise this test — it's currently queued/in progress, not failing. Please wait for that run to go green, confirming I didn't have simulator access in this pass, so I couldn't run that test myself or confirm from the code alone whether the mapper drops or maps the new keypath. I also couldn't confirm from the diff whether the affected-XCTest-selection step actually includes this test for this PR's file set, only that the changed files match what that job builds and targets. |
|
[claude-fable-5-1] responding on behalf of Oskar You were right to ask. The Smoke run showed the mapper's answer for the bridge's keypaths: 7565615 changes the test to what holds: the request names the native focus, and a snapshot that arrives with keyboard focus set reads as I did not find a CLI flag that forces |
7565615 to
a49fee9
Compare
|
Reviewed at a49fee9. Findings from the earlier pass (8a3ae74, #2925 (comment)) are unresolved.
All 13 checks are green. CI job 107797328001 (Smoke Tests) confirms Nothing else is blocking once the fixed code path has its own test. |
|
[claude-fable-5-1] responding on behalf of Oskar Agreed on both counts, done in 7e6ca6a.
Gate on the head: |
… the snapshot Both snapshot producers read hasFocus, the focus engine's attribute (tvOS, keyboard navigation), so the field a software keyboard was typing into never reported focused. Keyboard focus is hasKeyboardFocus, which the text-entry readiness check already consults; the XCTest producer and the AX bridge now report either as focused.
…d focus too The query-sweep producer and the collapsed-tab nodes read focus through elementHasFocus, which still asked for hasFocus alone; it now reads the same pair as the snapshot producers. The AX conformance fixture can hold keyboard focus on one node, and a test proves the bridge asks for the attribute and reports that node focused.
…sts, and the keyboard-focus test asserts it asked for keyboard focus
XCTAutomationSupport declares HasNativeFocus and FocusedApplications and no keyboard-focus attribute, so the mapper drops the hasKeyboardFocus keypath and the private-AX fallback reports the focus engine's focus alone; the XCTest producers carry keyboard focus. The test now asserts the request names the native focus and that a snapshot arriving with keyboard focus reads as focused, and the bridge comment says why the keypath stays.
…re the fix is The private-AX bridge cannot see keyboard focus (the AX server declares no such attribute), so its OR and the extra keypath were code for a state that cannot occur; they are gone, and the bridge is what it was on main. The two reads that carry the fix, snapshotHasFocus and elementHasFocus, now share one focusBool reader (hasKeyboardFocus, else hasFocus, through KVC), and a host-lane test drives that reader from fixtures carrying only keyboard focus, only native focus, neither, and no key at all.
7e6ca6a to
2ff4dcf
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…ocus engine's key
|
This looks ready. The fix addresses the focus report correctly and the code checks out at 279ccef. Not blocking: the focusBool doc comment says one reader serves every producer, but the AX bridge still reads Checks are still queued or in progress with nothing failing so far; the changed files are Swift-runner-only, which overlaps the route Smoke Tests exercises, so a failure there would need to be checked against this diff, but there is nothing to attribute yet. Once Smoke Tests and Coverage finish, this is ready for a human review. |
thymikee
added
the
ready-for-human
…ves and the bridge it does not
|
[claude-fable-5-1] responding on behalf of Oskar Took the wording nit in c7dece5: the |
Summary
A text field the software keyboard was typing into never reported
focusedin an iOS snapshot.Both focus reads asked for
hasFocus, the focus engine's attribute (tvOS, keyboard navigation); the field under a software keyboard holdshasKeyboardFocusinstead, which the text-entry readiness check already consults.snapshotHasFocus(RunnerTests+SnapshotAcquisition.swift, the recursive-tree producer) andelementHasFocus(RunnerTests+TvRemote.swift, the query-sweep and collapsed-tab element read) now share one reader,focusBool, which answershasKeyboardFocus || hasFocusthrough KVC and reads a missing key as false. The wire shape is unchanged:focusedis stilltrueor absent.The private-AX bridge is untouched: the AX server declares
HasNativeFocusandFocusedApplicationsand no keyboard-focus attribute, so that fallback cannot see keyboard focus and gets no speculative code for it.3 files, runner only. Android already reports focus.
Validation
Tested at the pushed head, on main.
pnpm check:xctest-selection,pnpm check:packaged-runner-swift, andpnpm check:affected --run --base c31cc49f6passed.testFocusBoolReadsKeyboardFocusBesideTheFocusEnginesFocus(host lane) drives the shared reader from KVC fixtures: both keys in every combination, each key alone with the other absent, and no key at all. It passes on the booted iPhone 17 Pro simulator viaxcodebuild test-without-building.Live on that simulator, React Native app: after tapping
name-input,snapshot --jsonreports the keyboardvisibleand exactly one focused node:Before the change the same snapshot reported no focused node.
Risk: a field with keyboard focus gains
focused, so anunchangedcomparison across a focus change now reports it.