fix(ios): target the active window for foldable interactions - #2724
Conversation
Size Report
Startup median (7 runs, lower is better):
|
|
Superseded by later commits on this PR — the host-defect hypothesis in this comment was wrong. The open-pose no-ops were runner-side: synthesized records were pinned to the main display (default init) and the app-frame-derived viewport did not match the resolved window. With the display-aware initializer and the window-resolved displayID (resolve the window frame first, then read its screen), open-pose injection delivers — verified by measured UITouch coordinates (see the Duo row in |
|
Superseded — the "display 3 is a black hole" conclusion in this comment was wrong, and with it the matrix interpretation. Every displayID=3 cell here used the app-frame-derived viewport, which does not match the resolved inner window; those taps would miss regardless of display routing, so the matrix could not distinguish "sink dead" from "coordinates wrong". The closed-pose control row only proves displayID routing is honored (which it is). The shipped commits fix both halves: window-resolved displayID + window-based viewport, with measured UITouch regression evidence. Comment retained as process record only. |
|
Reviewed at f552c5e. A later edit replaced my earlier review comment here with the open-pose notes, so I am posting it again. The code change looks right, but the live evidence does not cover the widest path it changes yet. The change swaps the rotation frame for every synthesized gesture on every iOS device, and the closed-pose Duo run is portrait only. The open-pose notes explain why the Duo open pose cannot prove delivery, but they do not cover a regular iPhone in landscape. Please add a run on a regular iPhone simulator in landscape (landscapeRight, and landscapeLeft if you can): take a snapshot, click an element by ref near a screen edge, then scroll once. The tap must land on its target (a UI change or a screenshot), and the Can Smoke Tests has passed since my first post, and all checks are green now. Not blocking: the new orientation test round-trips over a hand-written frame, so it passes on the old code too and mostly repeats the existing |
|
|
Reviewed a2f0e20, as a follow-up to the review on f552c5e. The delta moves On iOS, The non-zero-origin question from the last review is still open. Coverage fails because of this PR: Not blocking: the display ID, |
|
Responding to both review threads (landscape evidence + the origin question + Coverage). Coverage — fixed in 469cd9e: the package-source test's Live landscape run (iPhone 17 Pro simulator, iOS 26.2, interface 874×402) — I made the dispatch geometry durable instead of a one-off patch: 97b837a logs
Non-zero origin — we concluded the add-back would be invented, and measured evidence agrees: an
|
|
Reviewed f3d5387, the delta since a2f0e20. The Coverage fix works, and the landscapeRight and landscapeLeft runner logs you posted cover the landscape run I asked for. One point is still open. The measured rows and the landscape logs all use a window at origin (0,0), and all come from an iPhone. Does that also hold for iPad Split View, Slide Over or Stage Manager windows? Coverage and the Android, Linux and macOS Smoke Tests pass. iOS Smoke Tests is still running, and it runs the Next step: add the one-resolution-per-capture helper on this branch, or show iOS Smoke green past the first |
|
Smoke rerun data for the branch head, since the earlier green signals were pre-a2f0e2068:
So the unification (resolve the window once per capture and derive screen/viewport/orientation from that same value) is the merge gate for this branch — Smoke is the proof, not a formality. The Coverage gate itself is green on f3d5387 (run 35625371572) after 469cd9e + f3d5387 (the formatter had joined |
f3d5387 to
073b568
Compare
|
Rebased onto current On the one-resolution-per-capture ask: that is exactly the next layer, #2741 — resolve the window first, then read the display off that same instance, shared by the viewport, the display lookup, and the observation paths, inside the capture budget with typed fail-closed reasons. It is being finalized there right now (the app-window-first/system-surface-second formulation), so the helper lands stacked on this branch rather than being re-implemented underneath its own PR. On the flake-vs-stall question: iOS Smoke at f3d5387 already went green past the first |
|
Answering the Split View / Slide Over / Stage Manager question on its own terms: Nothing on this branch measures iPad multi-window modes, and I'd resist "just add the origin back" as a cheap hedge. In the portrait case re-adding What is true today, and where the exposure is: capture and dispatch read the same resolved window instance, so a pane at non-zero origin in its display's native space would diverge the two by exactly that origin — the invariant |
|
Rebase onto func resolveRunnerWindow(app: XCUIApplication) -> (element: XCUIElement, frame: CGRect)One walk of
The helper deliberately resolves the window only: no system-surface fallback and no typed reasons. That is #2741's Alert activation now also logs its landing so a tap that lands beside its button is visible in The iOS Smoke deep-link step is still red on this branch; the artifact shows the accept dismissing the SpringBoard sheet without routing, and the trace shows the activation tapping the button's frame center |
|
Reviewed 8001513. The new Take the case the helper exists for: window 0 exists with an empty frame and window N has the lit frame. The reference frame is booked against window N, but the display-ID read on window 0 either fails with "no resolved application window", or, if window 0 has a frame on the other panel, sends the gesture to that panel's display with coordinates in window N's space. So synthesized taps, scrolls and drags on a foldable can fail or land on the wrong panel. Could the bridge take the resolved window instead ( Not blocking: CI: iOS Smoke fails at "wait for Automation lab" with Next step: route the synthesized display ID through |
|
Reviewed-against fix pushed as Display ID now routes through the resolved window. The bridge gained The remaining Tests: Duo runner.log, open pose ( The record routes by Failed-step evidence for the deep-link red (from the On the not-blocking notes: took the frame re-read and the unit tests; the viewport-from-root-snapshot read is deliberately untouched — the viewport is needed before the tree capture and filters it, so swapping it to the snapshot's root changes capture ordering rather than just queries. |
|
Reviewed b9748e2. The display ID now comes from the same resolved window as the reference frame, so the finding from 8001513 is fixed. The Duo display ID evidence you posted is enough for that part. There is one new problem, and it likely explains the iOS Smoke failure. On main, iOS Not blocking: when CI: iOS Smoke fails at "wait for Automation lab" on both 8001513 and b9748e2. The failure comes right after Next step: restore the app-origin anchor. Then show a green iOS Smoke run that passes "wait for Automation lab", or a simulator runner.log where the |
`interactionCoordinate` resolved the interaction root through `resolveRunnerWindow`, so an iOS coordinate tap/double-tap/long-press/drag anchored on the first qualifying window and subtracted its frame origin. On SpringBoard that first window is not necessarily the alert's — the wallpaper or status-bar window can qualify first — so `alert accept` (reached via activateElement, tapAt, performCoordinateTap) missed the Open button, which is why iOS Smoke failed at "wait for Automation lab" after the deep-link confirmation. Restore the main-branch iOS app-origin anchor (app.coordinate(0,0) + the snapshot-space point). Reference frames and the synthesized display ID still resolve through `resolveRunnerWindow`, so a foldable tap keeps its panel and routes on the resolved window's display. macOS keeps the window-relative anchor. Refs #2724
b9748e2 to
8cbac15
Compare
|
Non-blocking item addressed in |
|
Reviewed 4116405. 8cbac15 restores the app-origin anchor from main for iOS coordinate tap, double tap, long press and drag (RunnerTests+Interaction.swift#L593), as the earlier review asked. This still needs evidence for that anchor on this head. The PR body says Not blocking: when no window has a non-empty frame, CI: iOS Smoke is still queued on 4116405. It runs the changed route ( |
|
Rebased onto Anchor restored (the Smoke cause). Rebase. One conflict in Evidence — green iOS Smoke. iOS workflow run Not-blocking nil fallback: left as-is — the |
|
Thanks. On 4116405, iOS Smoke is green, so the SpringBoard One item is still open. Coordinate taps now anchor at the app origin again, and the PR body says CI: all checks pass on 4116405. No conflicts. |
The synthesized lane derived its rotation frame from XCUIScreen.main.screenshot() image size while capture normalizes node rects into the acquisition viewport (app.frame). On an iPhone Duo in the open pose the runner's main screen is the dark outer panel, so dispatch was not the inverse of capture and taps drifted by a constant offset (hit the Home tab by luck, missed Settings). Use app.frame as the reference frame for every synthesized lane so dispatch is capture's inverse on any panel and orientation, and assert that inverse as a round-trip unit test including the Duo inner panel (669x951, rot90). Drops the per-gesture main-screen screenshot.
captureSnapshotRootBounded moved to RunnerTests+SnapshotAcquisition.swift with the capture split; the source-shape guard still read RunnerTests+Snapshot.swift, failing the Coverage job.
The dispatch point after rotation, the reference window it was computed against, and the display a record is routed to are the three facts that decide whether a synthesized event lands. runner.log now carries them (SYNTHESIZED_DISPATCH, SYNTHESIZED_RECORD) so delivery regressions are readable without instrumenting a build. Documents the origin invariant CoordinateSpaceRotation relies on.
…sizedCoordinateContext The formatter had joined it to the opening brace, which the selection scanner's line-start directive match misses, so the else branch parsed as unmatched.
The viewport, the interaction root, and the synthesized reference frame each walked app.windows with their own exists/frame rules, so a sheet or a fold could move one consumer without the others. resolveRunnerWindow is now the single walk: the first window with a non-empty frame, the application otherwise. Alert activation logs the chosen button's label and frame center so a tap that lands beside its button is visible in runner.log.
…indow The gesture reference frame is booked against the window resolveRunnerWindow picks, but record creation re-walked windows.firstMatch for the display ID. With an empty window 0 beside a lit window N the reference lands on N while the gesture routes to window 0's display, or fails outright. The bridge now takes the resolved window: the gesture entry points accept it, record creation reads the display from it, and every remaining firstMatch geometry read (keyboard avoidance, touch reference frame, system edge gestures) goes through the shared resolver. The interaction anchor and its frame come from one resolution pass, keeping the coordinate offset on the window the tap was measured against. Unit tests cover empty-window-0, absent windows, and the pure window-index rule.
`interactionCoordinate` resolved the interaction root through `resolveRunnerWindow`, so an iOS coordinate tap/double-tap/long-press/drag anchored on the first qualifying window and subtracted its frame origin. On SpringBoard that first window is not necessarily the alert's — the wallpaper or status-bar window can qualify first — so `alert accept` (reached via activateElement, tapAt, performCoordinateTap) missed the Open button, which is why iOS Smoke failed at "wait for Automation lab" after the deep-link confirmation. Restore the main-branch iOS app-origin anchor (app.coordinate(0,0) + the snapshot-space point). Reference frames and the synthesized display ID still resolve through `resolveRunnerWindow`, so a foldable tap keeps its panel and routes on the resolved window's display. macOS keeps the window-relative anchor. Refs #2724
The gesture callers resolve the window in Swift before the call, so a nil resolved window already means no window qualified. Walking windows.firstMatch a second time in record creation could only book a display that every Swift caller had refused to book geometry against; the missing window is now the error.
4116405 to
b8fcbdb
Compare
resolveRunnerWindow reports "no window" honestly: it returns nil alongside app.frame when no window has a non-empty frame. synthesizedCoordinateContext still built a context from that frame, so a synthesized drag or pinch carried a nil window into record creation and failed with "no resolved application window" on a case main dispatched. The context now refuses to exist without its window, which sends the gesture down the XCUITest fallback that already covers it and lets SynthesizedCoordinateContext.resolvedWindow be non-optional. Refs #2724
Every synthesized gesture routes its display through the window it already resolved for geometry, so the wrapper that re-walked windows.firstMatch had no production caller left. Its assertions move to RunnerResolveWindowDisplayID, so the resolve-before-read ordering, the primary-window identity, and both refusal branches stay under test. Refs #2724
|
Rebased onto iOS Smoke is green on this head. Run Live Duo run on this head (iPhone Duo, iOS 27.1, runner rebuilt from
Open-pose coordinate long press does not reach the control, and I could not make it pass. Holds of 800, 900, 1200 and 1800ms at the control's own center produce no event at all — not even So of your two unproven items: Both not-blocking items landed:
Body corrected: the Summary no longer says XCTest coordinate actions anchor to the window — it now says the resolved window drives capture geometry and the synthesized display ID, not the tap or hold anchor. Validation names |
|
Reviewed 24a6bbc. The new commits hold up. A gesture with no resolved window now returns Your Duo run on this head covers the open-pose coordinate press through Not blocking: the All 21 checks pass on 24a6bbc, and there are no conflicts. This is ready for human review. |
Summary
Fix iPhone Duo interactions when unfolded. Synthesized gestures now target the resolved app window's display, and capture/gesture transforms use that window's viewport. iOS XCTest coordinate actions keep main's app-origin anchor; the resolved window drives capture geometry and the synthesized display ID, not the tap or hold anchor. Coordinate presses, scrolling, and synthesized gestures keep reaching their controls across closed, half-open, and open pose changes; open-pose coordinate holds are called out under Validation.
The previous diagnosis in this PR was incorrect: XCTest has a display-aware event initializer, and the inner display receives events. The failures combined outer-display targeting with an incorrect
app.frameviewport. Resolving the window frame before reading its screen is essential.Touches 16 files within Apple interaction/capture support, regression fixtures, help, and docs. Gross churn exceeds the usual budget because two existing files exceeded 1,000 lines; gesture and snapshot acquisition helpers and their tests were extracted to satisfy the repository's required split-before-behavior rule. No new runtime layer was introduced.
Validation
Tested head:
24a6bbca06975ab8683ff5e979a29eb2b1c32faf.pnpm check:affected --run: all runnable checks passed.automation-press(476,439)fires the canary,Last input: none→Last input: press. Route ispresswithout--synthesized→tapAt→performCoordinateTap→interactionCoordinate, so it exercises the restored anchor.automation-longpress(233,476), 800ms, increments the durable counter,Long presses: 0→Long presses: 1.onPress, while a plain tap at that same point firesonPress.longPressAt→performCoordinateLongPress→interactionCoordinateis unchanged frommainon iOS (theos(iOS)branch is byte-identical, and the hold path has zero changed lines), so this is pre-existing open-pose behavior rather than a regression from this PR, and it is not fixed here.(226,475)reaches window(476,226)in the 951×669 viewport.CI on this head: Repo Guards, Lint & Format, Typecheck & Package, Integration Tests and three Smoke lanes are green, and the five display-routing/window-selection unit tests pass locally. iOS Smoke covers
alert accept→tapAt→performCoordinateTap→interactionCoordinate, so its result counts for the restored anchor. Multipointer gestures share the corrected synthesis path but were not separately live-tested. Verification sessions and retained runners cleaned up.