fix(scroll): keep the swipe above the keyboard, refuse when it cannot - #2503
fix(scroll): keep the swipe above the keyboard, refuse when it cannot#2503thymikee wants to merge 4 commits into
Conversation
|
Size Report
Startup median (7 runs, lower is better):
|
|
The clipping rule still allows swipes into a floating keyboard at 0fb3f39. Both Required live evidence is also incomplete: please exercise Android with an adjustPan activity and the iOS The change has 1,561 gross lines and adds 8.2 kB unpacked. Please split it below the repository's 1,000-line PR budget and explain why the smaller owning change is insufficient, or obtain an explicit budget exception. The iOS smoke failed while waiting for the Automation lab screen, before exercising keyboard clipping; that appears unrelated. The separately reported mutation-model failure was reproduced at the base and also appears unrelated. |
0fb3f39 to
c7f4bd2
Compare
|
Addressed at Centre-line gate. The width fraction is gone from Android adjustPan, live. Pixel 7 emulator, Contacts editor ( Still open: the live Size. 1,443 gross (817 production) after the trims listed in the body, so still over 1,000. The clean split is this PR minus Android (~260 lines: helper window read, |
c7f4bd2 to
faa75e7
Compare
|
Split per the size ask. This branch is now the contract plus the Apple runner path: 17 files, 980 gross lines, under the 1,000 budget. The Android half (helper IME read,
One thing the rebuild surfaced: the previous head also deleted a nine-line comment in Still open and unchanged from my last note: the live |
|
The floating-keyboard width check and size split are resolved at faa75e7, but the clipped frame now also changes the native coordinate-rotation basis. RunnerTests+CommandExecution.swift:2004 passes the shortened frame into scrollContext.withReferenceFrame; synthesizedDragAt uses its height for landscapeRight rotation. Clipping an 834-point frame to 588 points shifts native x by 246 points, so the gesture misses its planned path. Preserve the original coordinate frame for rotation and use the clipped frame only for swipe planning; cover a landscape keyboard case through dispatch. Current-head checks pass and there are no conflicts. The reported live iOS refusal/no-dispatch validation is still missing. |
|
Fixed at The resolved viewport now names both frames —
Proved the test bites rather than asserting my own shape: handing the band to One verification correction worth recording, because it wasted my earlier confidence:
|
|
Both halves of your comment are in: the rotation-basis fix is Size needs your call, and I would rather ask than degrade the fix. The split head you cleared was 980 gross. The rotation-basis fix lands 1,125 (+145): ~55 lines of the landscape dispatch test you asked for, and the rest the Three ways through, priced:
Say 1 and I leave it as is; say 2 and I will push the reduced variant in one commit. |
|
The iOS Smoke lane failed at Not this diff:
Reran the failed job for the record. |
|
The rotation basis is fixed at b6ea4e6, but the response now reports the coordinate frame instead of the clipped planning frame. executeSynthesizedDragGesture builds its DragVisualizationFrame from plan.referenceFrame (RunnerTests+CommandExecution.swift:2492-2513), while attachingScrollViewportEvidence only adds keyboard fields. A swipe planned in a 571-point band therefore reports the full 874-point referenceHeight. Keep the full frame for synthesis and report the clipped plan dimensions; assert the returned scroll response in the regression. Current checks pass and there are no conflicts. Live iOS refusal/no-dispatch evidence remains missing, and the diff is now 1,125 gross lines, so the size budget needs a split or explicit exception again. |
…nnot The runner owns the live keyboard frame, so it does the clip and reports what it left: a scroll answers with `keyboardAvoided` and `keyboardMinY` beside its plan, and refuses with `SCROLL_KEYBOARD_OCCLUDES_SURFACE` when the keys leave too little band to swipe in instead of flinging into them. It never dismisses the keyboard, which would drop focus and mutate state that session-action provenance does not record. Scroll's keyboard policy moves to `requiredWhenAvailable`. The probe costs a live AX fetch, but gating it on a healthy tree left the first scroll of a session swiping under the keys, which is the failure this is for. Every scroll logs its decision, including the two ways it avoids reading the keyboard at all. Scroll no longer shares `frameAvoidingKeyboard`, whose 25% fail-open was a tap-reference-frame rule; that path is unchanged for its remaining callers.
The parity table only detects drift if both halves run in CI. Two of these three were reachable by no lane, so the Swift half of the table was a local assertion.
`resolvedScrollViewport` handed the command one frame for both jobs, and the coordinate rotation reads a frame's HEIGHT to map a `landscapeRight` native x. Clipping an 834pt landscape viewport to 576pt therefore moved the dispatched gesture 258pt sideways off the lane the plan had just been built for: the clip fixed the keyboard and broke the gesture. The resolved viewport now names both frames, and the gesture comes from one dispatch decision, so the band the plan is planned inside and the frame its coordinates rotate against cannot be swapped. The landscape case asserts through that decision and fails on the swap.
b2d153e to
8a67c37
Compare
|
All three points addressed. Reported frame — fixed in The regression now asserts the assembled response, not just the plan: Live iPhone 17 Pro, Form screen, field focused, Size — resolved by splitting rather than by asking you for an exception I could have avoided. My earlier claim that a contract-only PR fails
The split is tree-exact: the two branches reassemble to the same tree the 1,151-line branch had ( Live occlusion refusal — still not reproduced, and I want to be explicit that this one remains open rather than implied. The band above the keys cannot fall under 15% on an iPhone (583 of 874 = 65%), and iPadOS 26 keeps an app window clear of the keyboard (window bottom 913, keyboard top 925), so reaching it needs an iPad app window dragged down to roughly the keyboard's own height from the host desktop; the macOS desktop target timed out at the daemon budget here before the drag landed. The command that would close it is |
|
The reported-frame correction is clean on 8a67c37: successful responses now report the planned clipped frame while rotation retains the full coordinate frame. The split also brings this layer to 536 changed lines. The reported live iPhone keyboard-clipping run supports the successful path. Readiness still needs live iOS keyboard-occlusion refusal evidence showing no gesture dispatch; contract tests do not establish that device path. This also depends on #2537. |
1 participant
Summary
iOS half of #2500, stacked on #2537, where the shared rule, the golden table, the typed refusal and the response keys are stated once for every platform.
A directional
scrollplanned a viewport-center-symmetric swipe, so a focused field put the swipe's lower endpoint under the keyboard: the gesture hit keys, the surface never moved, and the edge loop reported a stuck container. The runner now reads its own live keyboard frame, clips the swipe band against the shared rule, reportskeyboardAvoided/keyboardMinY, refuses withscroll_keyboard_occludes_surfacewhen the keys leave under 15% of the band, and never dismisses the keyboard. The superseded scroll keyboard path was deleted, not guarded.Closes #2500 once #2537 and #2514 land. Android goes on top of this in #2514. main now carries #2499, so both halves of the issue are in: the clip stops the wasted gesture, and #2499's no-progress stop ends the loop.
Review pass
The width-fraction gate is gone: the clip asks whether the keyboard reaches the line the swipe runs along, not what share of the viewport's width it covers, because a vertical swipe travels the centre line and a floating keyboard straddling that line blocks it however narrow it is. The row that proves it (
centred floating keyboard under half the viewport width sits in the swipe's path) is in the shared table landed by #2537, and the Swift lane asserts that same JSON row here.Two things found while checking for reuse, both left alone deliberately:
frameAvoidingKeyboard(RunnerTests+Interaction.swift:789) still gates onintersection.width / frame.width >= 0.5, i.e. the same defect for tap/drag reference frames. Fixing it changes the tap family, so it belongs in its own change; flagged rather than bundled.keyboardAvoidingDragPoints(same file,:702) keeps a localpadding: Double = 12that names the same fact asaccessoryAllowance, and fails open where scroll refuses. Sharing the constant means editing the drag rule, and the two failure policies differ on purpose: a drag that cannot fit still wants to happen, a scroll entirely under the keys does not.Reused
visibleKeyboardFrameand the iOS lane's existing-only-testinglist.A second pass found the clip leaking into the coordinate rotation.
resolvedScrollViewportreturned one frame and the command used it for two jobs: planning the swipe, and re-basing the touch context.nativeSynthesizedPointderives alandscapeRightnative x from that frame's HEIGHT, so clipping an 834pt landscape viewport to 576pt moved the dispatched gesture 258pt sideways off the lane the plan had just been built for. The viewport now carriesplanFrameandcoordinateFrameseparately and the gesture comes from onegestureDispatchdecision, so the two cannot be swapped;testScrollViewportDispatchKeepsTheUnclippedFrameAsItsCoordinateRotationBasisruns the landscape case through that decision and fails on the swap.A third pass found that fix had moved the reported frame too. Splitting the frames left the response naming the synthesis frame:
executeSynthesizedDragGesturebuilds itsDragVisualizationFramefrom the frame the coordinates rotate in, so a swipe planned inside a 571pt band answered withreferenceHeight: 874, andpixels— which is a fraction of the band — was measured against an axis the caller was never told about. The keyboard evidence now travels with the dispatch instead of being bolted on afterwards:ScrollGestureDispatch.attachingEvidence(to:)namesplanFrameinreferenceWidth/referenceHeightand keepscoordinateFramefor synthesis, and the oldattachingScrollViewportEvidencehelper is gone. The regression above now asserts the assembled response as well, so reporting the synthesis frame is a red test rather than a reviewer's job.Validation
pnpm checkgreen at8a67c377cf(exit 0).Reproducing the runner tests locally needs
AGENT_DEVICE_XCUITEST_INCLUDE_UNIT_TESTS=1on the build:xcodebuild test-without-building -only-testing:executes 0 tests and still reports the suite as passed without it, because that env var is what definesAGENT_DEVICE_RUNNER_UNIT_TESTS. The iOS lane always sets it; my own earlier local runs did not and were therefore empty. WithAGENT_DEVICE_XCUITEST_INCLUDE_UNIT_TESTS=1 pnpm build:xcuitest:iosthe scroll and policy tests genuinely execute (Executed 4 tests, with 0 failures), and each guard bites when neutered: handing the clipped band to the rotation basis fails on a0.0vs258.0shift, and dropping the two evidence lines fails with("Optional(834.0)") is not equal to ("Optional(576.0)")— the symptom reported in review, now pinned.iOS, iPhone 17 Pro, Form screen, field focused:
scroll down --json->referenceWidth: 402,referenceHeight: 571,keyboardAvoided: true,keyboardMinY: 583, swipey1: 472 -> y2: 100,pixels: 371, and the runner loggeddecision=avoided keyboardMinY=583.0 swipeHeight=571.0. Same command with no keyboard up reportedreferenceHeight: 874,y1: 721 -> y2: 153,pixels: 568, so the two answers differ exactly by what the keys took.scroll top --jsonon the notes field earlier in the same build returnedreferenceHeight: 571,keyboardMinY: 583,y2=472.axHealth=unknown keyboardPolicy=requiredWhenAvailable decision=avoided; the oldwhenAccessibilityHealthygate skipped the probe there.referenceHeight: 874,decision=noKeyboard.iOS, iPadOS 26 windowed Safari (address bar focused):
referenceWidth: 834,referenceHeight: 913,keyboardAvoided: true,keyboardMinY: 925. The window is 913pt tall inside a 1219pt display, so this is the clip reading the app window rather than the display. That simulator was created for the attempt and deleted afterwards, and the host'sConnectHardwareKeyboardwas returned to its original value after the soft-keyboard run.Unreproduced live: the
occludedrefusal. The band above the keys cannot fall under 15% on an iPhone (583 of 874 = 65%), and iPadOS 26 keeps an app window clear of the keyboard (window bottom 913, keyboard top 925), so reaching it needs an iPad app window resized to roughly the keyboard's own height, dragged from the host desktop; the macOS desktop target timed out at the daemon budget here before the drag landed. The command that would prove it isagent-device scroll down --jsonon such a window, on a host window server session that answers desktop snapshots. The risk left open is narrow: the refusal branch is reached only from the same clip decision that the live runs above exercised, it returns before any gesture is dispatched, and the parity table rowadjustPan window with only 88px of 900px left above the IME is refusedplus the platform tests assert no swipe is planned when the keyboard owns the window.Size
6 files, 536 gross lines, one command family, 4 commits: the runner clip, its lane wiring, and the response evidence. The shared rule and golden table are #2537 (615 gross) and the Android helper read and clip are #2514 (612 gross), which is what the budget needed and also where those facts belong.