From 95ba7e8239fbe6ac98cb6c697264279d2bd7caba Mon Sep 17 00:00:00 2001 From: Adam Rowe Date: Fri, 16 Oct 2026 14:56:31 -0400 Subject: [PATCH 1/2] fix(scroll): keep Android directional swipe endpoints inside the resolved scrollable A directional scroll centred its swipe on the window, so on an inset scrollable the drag could start on a header above the list and the command failed with scroll_no_progress. The daemon now resolves the scroll container from the current stored tree before the gesture and hands its rect to the owner. The shared planner keeps both endpoints inside the container's intersection with the padded frame: a span that fits is unchanged, one that leaves the band moves into it, and one longer than the band shrinks to it and reports `requestedPixels` beside the dispatched `pixels`. Android applies it after the keyboard clip, so the two compose. iOS plans in the runner and follows separately. Refs #2714 --- packages/contracts/src/scroll-command.ts | 22 ++++- .../src/scroll-gesture-container.test.ts | 94 +++++++++++++++++++ packages/contracts/src/scroll-gesture.ts | 78 +++++++++++++-- .../__tests__/scroll-runtime.test.ts | 67 +++++++++++++ .../daemon/src/execution/scroll-movement.ts | 39 ++++++-- .../daemon/src/execution/scroll-runtime.ts | 16 +++- .../src/__tests__/input-actions.test.ts | 50 ++++++++++ .../platform-android/src/input-actions.ts | 12 ++- 8 files changed, 353 insertions(+), 25 deletions(-) create mode 100644 packages/contracts/src/scroll-gesture-container.test.ts diff --git a/packages/contracts/src/scroll-command.ts b/packages/contracts/src/scroll-command.ts index 39de1e80cf..52e6f89fcb 100644 --- a/packages/contracts/src/scroll-command.ts +++ b/packages/contracts/src/scroll-command.ts @@ -1,6 +1,7 @@ import { AppError } from '@agent-device/kernel/errors'; import type { SettleObservation } from './interaction.ts'; import type { ScrollDirection } from './scroll-gesture.ts'; +import type { Rect } from '@agent-device/kernel/snapshot'; export const SCROLL_DURATION_MAX_MS = 10_000; export const DEFAULT_MOBILE_SCROLL_DURATION_MS = 300; @@ -38,13 +39,24 @@ export type ScrollTimingOptions = { export type ScrollCommandOptions = ScrollDistanceOptions & ScrollTimingOptions; -export type ScrollExecutionOptions = ScrollCommandOptions & { - releaseBehavior?: ScrollReleaseBehavior; +/** + * The scroll container the caller resolved from the current tree, in the snapshot's absolute space. + * An owner that plans its own swipe keeps both endpoints inside it (#2714); an owner without that + * planner ignores it. + */ +export type ScrollContainerTarget = { + containerRect?: Rect; }; -export type ResolvedScrollExecutionOptions = ScrollCommandOptions & { - releaseBehavior: ScrollReleaseBehavior; -}; +export type ScrollExecutionOptions = ScrollCommandOptions & + ScrollContainerTarget & { + releaseBehavior?: ScrollReleaseBehavior; + }; + +export type ResolvedScrollExecutionOptions = ScrollCommandOptions & + ScrollContainerTarget & { + releaseBehavior: ScrollReleaseBehavior; + }; export function resolveScrollExecutionOptions( options: ScrollCommandOptions, diff --git a/packages/contracts/src/scroll-gesture-container.test.ts b/packages/contracts/src/scroll-gesture-container.test.ts new file mode 100644 index 0000000000..e969e22b8c --- /dev/null +++ b/packages/contracts/src/scroll-gesture-container.test.ts @@ -0,0 +1,94 @@ +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import type { Rect } from '@agent-device/kernel/snapshot'; +import { buildScrollGesturePlan, type ScrollDirection } from './scroll-gesture.ts'; + +// The iPhone geometry #2714 was reported on: a 402x874 frame, the default 10% edge padding leaving +// y 87..787 for a swipe. +const FRAME = { referenceWidth: 402, referenceHeight: 874 }; +const PADDED_TOP = 87; +const PADDED_BOTTOM = 787; +const INSET_CONTAINER: Rect = { x: 18, y: 178, width: 366, height: 662 }; +const FITTING_CONTAINER: Rect = { x: 18, y: 62, width: 366, height: 729 }; + +function endpointsInside( + plan: { x1: number; y1: number; x2: number; y2: number }, + rect: Rect, +): boolean { + const inside = (x: number, y: number) => + x >= rect.x && x <= rect.x + rect.width && y >= rect.y && y <= rect.y + rect.height; + return inside(plan.x1, plan.y1) && inside(plan.x2, plan.y2); +} + +test.each(['up', 'down'])( + 'a %s swipe longer than an inset container shrinks to the container and reports both spans', + (direction) => { + const unclamped = buildScrollGesturePlan({ direction, amount: 0.75, ...FRAME }); + const plan = buildScrollGesturePlan({ + direction, + amount: 0.75, + ...FRAME, + container: INSET_CONTAINER, + }); + + assert.ok(endpointsInside(plan, INSET_CONTAINER), JSON.stringify(plan)); + assert.deepEqual( + [Math.min(plan.y1, plan.y2), Math.max(plan.y1, plan.y2)], + [INSET_CONTAINER.y, PADDED_BOTTOM], + ); + assert.equal(plan.pixels, PADDED_BOTTOM - INSET_CONTAINER.y); + assert.equal(plan.requestedPixels, unclamped.pixels); + assert.equal(Math.sign(plan.y2 - plan.y1), Math.sign(unclamped.y2 - unclamped.y1)); + }, +); + +test.each(['up', 'down', 'left', 'right'])( + 'a %s swipe already inside its container keeps today’s endpoints and distance', + (direction) => { + const unclamped = buildScrollGesturePlan({ direction, amount: 0.75, ...FRAME }); + const plan = buildScrollGesturePlan({ + direction, + amount: 0.75, + ...FRAME, + container: FITTING_CONTAINER, + }); + + assert.deepEqual(plan, unclamped); + assert.equal('requestedPixels' in plan, false); + }, +); + +test('a swipe that fits an offset container moves into it without shortening', () => { + const lowerContainer: Rect = { x: 0, y: 300, width: 402, height: 574 }; + const unclamped = buildScrollGesturePlan({ direction: 'down', amount: 0.5, ...FRAME }); + const plan = buildScrollGesturePlan({ + direction: 'down', + amount: 0.5, + ...FRAME, + container: lowerContainer, + }); + + assert.ok(endpointsInside(plan, lowerContainer), JSON.stringify(plan)); + assert.equal(Math.min(plan.y1, plan.y2), lowerContainer.y); + assert.equal(Math.abs(plan.y1 - plan.y2), Math.abs(unclamped.y1 - unclamped.y2)); + assert.equal(plan.pixels, unclamped.pixels); + assert.equal('requestedPixels' in plan, false); +}); + +test('a vertical swipe beside a narrow container runs down its centre line', () => { + const rightColumn: Rect = { x: 250, y: 0, width: 152, height: 874 }; + const unclamped = buildScrollGesturePlan({ direction: 'up', ...FRAME }); + const plan = buildScrollGesturePlan({ direction: 'up', ...FRAME, container: rightColumn }); + + assert.equal(plan.x1, rightColumn.x + rightColumn.width / 2); + assert.equal(plan.x2, plan.x1); + assert.deepEqual([plan.y1, plan.y2, plan.pixels], [unclamped.y1, unclamped.y2, unclamped.pixels]); +}); + +test('a container lying wholly in the edge padding leaves the padded swipe as it was', () => { + const statusBand: Rect = { x: 0, y: 0, width: 402, height: PADDED_TOP - 1 }; + const unclamped = buildScrollGesturePlan({ direction: 'up', ...FRAME }); + const plan = buildScrollGesturePlan({ direction: 'up', ...FRAME, container: statusBand }); + + assert.deepEqual(plan, unclamped); +}); diff --git a/packages/contracts/src/scroll-gesture.ts b/packages/contracts/src/scroll-gesture.ts index 5b8be2fe8f..bf450867d8 100644 --- a/packages/contracts/src/scroll-gesture.ts +++ b/packages/contracts/src/scroll-gesture.ts @@ -49,6 +49,12 @@ export type ScrollGestureOptions = { pixels?: number; referenceWidth: number; referenceHeight: number; + /** + * The scroll container the swipe is meant for, in the reference frame's own zero-origin space. + * Both endpoints stay inside its intersection with the padded frame (#2714): a span that already + * fits is unchanged, one that leaves it is moved in, and one longer than it is shortened. + */ + container?: Rect; }; export type ScrollGesturePlan = { @@ -60,7 +66,10 @@ export type ScrollGesturePlan = { referenceWidth: number; referenceHeight: number; amount?: number; + /** The span dispatched, which is less than `requestedPixels` when the container shortened it. */ pixels: number; + /** Present only when the container shortened the swipe: the span the padded frame would carry. */ + requestedPixels?: number; }; export type SwipePresetGesturePlan = { @@ -125,16 +134,65 @@ export function buildScrollGesturePlan(options: ScrollGestureOptions): ScrollGes pixels: travelPixels, }); - switch (direction) { - case 'up': - return buildPlan(centerX, centerY - halfTravel, centerX, centerY + halfTravel); - case 'down': - return buildPlan(centerX, centerY + halfTravel, centerX, centerY - halfTravel); - case 'left': - return buildPlan(centerX - halfTravel, centerY, centerX + halfTravel, centerY); - case 'right': - return buildPlan(centerX + halfTravel, centerY, centerX - halfTravel, centerY); - } + const vertical = direction === 'up' || direction === 'down'; + const unclamped = { + axisStart: (vertical ? centerY : centerX) - halfTravel, + axisEnd: (vertical ? centerY : centerX) + halfTravel, + cross: vertical ? centerX : centerY, + }; + const lane = options.container + ? keepSwipeInContainer(unclamped, options.container, vertical, { + min: edgePadding, + max: axisLength - edgePadding, + }) + : unclamped; + const shortened = lane.axisEnd - lane.axisStart < unclamped.axisEnd - unclamped.axisStart; + const plan = buildPlan( + vertical ? lane.cross : lane.axisStart, + vertical ? lane.axisStart : lane.cross, + vertical ? lane.cross : lane.axisEnd, + vertical ? lane.axisEnd : lane.cross, + ); + const placed = shortened + ? { ...plan, pixels: lane.axisEnd - lane.axisStart, requestedPixels: travelPixels } + : plan; + // `up`/`left` drag from the low end toward the high end; `down`/`right` drag the other way. + return direction === 'up' || direction === 'left' + ? placed + : { ...placed, x1: placed.x2, y1: placed.y2, x2: placed.x1, y2: placed.y1 }; +} + +type SwipeLane = { axisStart: number; axisEnd: number; cross: number }; + +/** + * Moves a centred swipe lane inside the container's intersection with the padded axis, shortening + * it only when the intersection is shorter than the lane. A container wholly inside the padding + * leaves nothing to swipe in, so the lane stays as the padded frame planned it. + */ +function keepSwipeInContainer( + lane: SwipeLane, + container: Rect, + vertical: boolean, + padded: { min: number; max: number }, +): SwipeLane { + const containerMin = vertical ? container.y : container.x; + const containerSize = vertical ? container.height : container.width; + const bandMin = Math.max(padded.min, Math.round(containerMin)); + const bandMax = Math.min(padded.max, Math.round(containerMin + containerSize)); + if (bandMax <= bandMin) return lane; + + const crossMin = vertical ? container.x : container.y; + const crossSize = vertical ? container.width : container.height; + const cross = + lane.cross >= crossMin && lane.cross <= crossMin + crossSize + ? lane.cross + : Math.round(crossMin + crossSize / 2); + + const length = lane.axisEnd - lane.axisStart; + if (length >= bandMax - bandMin) return { axisStart: bandMin, axisEnd: bandMax, cross }; + const shift = + lane.axisStart < bandMin ? bandMin - lane.axisStart : Math.min(0, bandMax - lane.axisEnd); + return { axisStart: lane.axisStart + shift, axisEnd: lane.axisEnd + shift, cross }; } /** diff --git a/packages/daemon/src/execution/__tests__/scroll-runtime.test.ts b/packages/daemon/src/execution/__tests__/scroll-runtime.test.ts index 0dd7118984..4219f10ba7 100644 --- a/packages/daemon/src/execution/__tests__/scroll-runtime.test.ts +++ b/packages/daemon/src/execution/__tests__/scroll-runtime.test.ts @@ -786,3 +786,70 @@ test('bound scroll will not bill an unread earlier gesture to the scroll it is d assert.equal(result.movement, 'unobserved'); assert.equal(captures.calls(), 0); }); + +/** + * The container a directional swipe is planned inside (#2714): read off the same current tree the + * movement claim compares against, before the gesture, and never off a tree that may no longer be + * the screen. + */ +async function containerHandedToOwner( + positionals: string[], + session: SessionState, + flags?: CommandFlags, +): Promise { + const calls: ScrollCall[] = []; + await runScroll( + positionals, + {}, + { + scroll: async (direction, options) => { + calls.push({ direction, options }); + return { ...OBSERVED_SWIPE }; + }, + }, + { session, ...(flags === undefined ? {} : { flags }) }, + ); + const [call] = calls; + assert.ok(call && calls.length === 1, `expected one scroll, got ${calls.length}`); + return (call.options as { containerRect?: unknown }).containerRect; +} + +test.each<[string, CommandFlags | undefined]>([ + ['an observed scroll', undefined], + ['a --settle scroll', { settle: true }], +])('bound scroll hands %s the container resolved from the current tree', async (_, flags) => { + const containerRect = await containerHandedToOwner( + ['down', '0.75'], + sessionWithStoredScreen(automationScreen(0)), + flags, + ); + + assert.deepEqual(containerRect, SCREEN_CONTAINER); +}); + +test('bound scroll hands no container from a tree that predates the last mutation', async () => { + const session = sessionWithStoredScreen(automationScreen(0)); + expireRefFrame(session); + + assert.equal(await containerHandedToOwner(['down', '0.75'], session), undefined); +}); + +test('bound scroll hands no container for a horizontal swipe the edge analyzer cannot place', async () => { + const containerRect = await containerHandedToOwner( + ['left'], + sessionWithStoredScreen(automationScreen(0)), + ); + + assert.equal(containerRect, undefined); +}); + +test('bound scroll hands no container to a desktop owner that never synthesizes a swipe', async () => { + const session = makeMacOsSession('scroll-desktop'); + session.snapshot = buildSnapshotState( + { nodes: automationScreen(0), backend: 'xctest', producer: 'apple-runner' }, + undefined, + ); + activateCompleteRefFrame(session); + + assert.equal(await containerHandedToOwner(['down', '0.75'], session), undefined); +}); diff --git a/packages/daemon/src/execution/scroll-movement.ts b/packages/daemon/src/execution/scroll-movement.ts index 250c25fb07..0fc1f0454e 100644 --- a/packages/daemon/src/execution/scroll-movement.ts +++ b/packages/daemon/src/execution/scroll-movement.ts @@ -164,17 +164,40 @@ export function readScrollSurfaceBaseline( function freezeScrollSurfaceBaseline(session: SessionState): ScrollMovementPlan { const baseline = readScrollSurfaceBaseline(session.snapshot); if (!baseline) return { kind: 'unobservable', reason: 'stored-surface-unusable' }; - if (refFrameState(session) !== 'active') { - return { kind: 'unobservable', reason: 'stored-surface-not-current' }; - } - if (isPostGestureStabilizationPending(session)) { - // A gesture nobody has read yet may still be moving this tree, which would bill its motion to - // the scroll being dispatched now. - return { kind: 'unobservable', reason: 'prior-gesture-unsettled' }; - } + const staleness = storedSurfaceStaleness(session); + if (staleness) return { kind: 'unobservable', reason: staleness }; return { kind: 'observe', baseline }; } +function storedSurfaceStaleness( + session: SessionState, +): Exclude | undefined { + if (refFrameState(session) !== 'active') return 'stored-surface-not-current'; + // A gesture nobody has read yet may still be moving this tree, which would bill its motion to + // the scroll being dispatched now. + if (isPostGestureStabilizationPending(session)) return 'prior-gesture-unsettled'; + return undefined; +} + +/** + * The scroll container a directional swipe should stay inside (#2714), read before the gesture off + * the same current tree a movement claim would compare against, through the edge verdict's own + * selection. A tree that may no longer be the screen names no container, and neither does a + * horizontal scroll, which the vertical-only edge analyzer cannot place, nor a platform that moves + * content without a synthesized swipe. + */ +export async function readScrollTargetContainer( + device: DeviceInfo, + session: SessionState, + direction: ScrollDirection, +): Promise { + if (!isMobilePlatform(device)) return undefined; + const edge = verticalEdgeFor(direction); + const nodes = session.snapshot?.nodes; + if (!edge || !nodes || nodes.length === 0 || storedSurfaceStaleness(session)) return undefined; + return (await readScrollEdgeState(nodes, edge)).containerRect; +} + /** * Observes one directional scroll and returns the movement its response may claim. * diff --git a/packages/daemon/src/execution/scroll-runtime.ts b/packages/daemon/src/execution/scroll-runtime.ts index 0716eadc0f..e58671749b 100644 --- a/packages/daemon/src/execution/scroll-runtime.ts +++ b/packages/daemon/src/execution/scroll-runtime.ts @@ -47,6 +47,7 @@ import { import { observeScrollMovement, planScrollMovement, + readScrollTargetContainer, reportScrollMovementNotApplicable, reportScrollMovementUnobserved, type ScrollMovementPlan, @@ -145,10 +146,23 @@ export async function resolveBoundScrollRuntime( flags: params.flags, session: params.session, }); + const containerRect = await readScrollTargetContainer( + params.device, + params.session, + target.direction, + ); + const directionOptions = + containerRect === undefined ? options : { ...options, containerRect }; return await resolveBoundGenericRuntime( { ...admission, use: plan.use }, async (runtime, dispatchContext) => - await executeDirectionScroll(runtime, target, options, dispatchContext, movementPlan), + await executeDirectionScroll( + runtime, + target, + directionOptions, + dispatchContext, + movementPlan, + ), ); } case 'edge': { diff --git a/packages/platform-android/src/__tests__/input-actions.test.ts b/packages/platform-android/src/__tests__/input-actions.test.ts index 35261477c9..29906d8337 100644 --- a/packages/platform-android/src/__tests__/input-actions.test.ts +++ b/packages/platform-android/src/__tests__/input-actions.test.ts @@ -134,6 +134,56 @@ test('scrollAndroid swipes the whole window when no IME window is on screen', as }); }); +function swipeSpan(result: Record): [number, number] { + const ys = [Number(result.y1), Number(result.y2)]; + return [Math.min(...ys), Math.max(...ys)]; +} + +test('scrollAndroid keeps both endpoints inside an inset container and reports the shortened span', async () => { + // A window below the status bar, holding a list that starts under a tall header: the unclamped + // 0.8 swipe would start in the header, so the gesture is shortened to the list (#2714). + const offsetWindow = { x: 0, y: 136, width: 1080, height: 2264 }; + const list = { x: 0, y: 436, width: 1080, height: 1700 }; + await withFakeAdb(helperRouteAdb(offsetWindow), async ({ device }) => { + const unclamped = await scrollAndroid(device, 'down', { amount: 0.8 }); + const result = await scrollAndroid(device, 'down', { amount: 0.8, containerRect: list }); + + assert.ok(swipeSpan(unclamped)[0] < list.y, 'the fixture must start outside the list'); + assert.deepEqual(swipeSpan(result), [list.y, list.y + list.height]); + assert.ok(Number(result.y1) > Number(result.y2), 'a down scroll still drags upward'); + assert.equal(result.pixels, list.height); + assert.equal(result.requestedPixels, unclamped.pixels); + }); +}); + +test('scrollAndroid keeps the swipe inside a container that runs under the keyboard', async () => { + // The keyboard clip alone keeps the frame's top, so a long swipe would start on the header above + // the list; the container and the clip together leave only the list above the keys. + const list = { x: 0, y: 400, width: 1080, height: 1880 }; + await withFakeAdb( + helperRouteAdb({ ...PORTRAIT_WINDOW, keyboard: LOWER_HALF_KEYBOARD }), + async ({ device }) => { + const result = await scrollAndroid(device, 'up', { amount: 0.8, containerRect: list }); + const [top, bottom] = swipeSpan(result); + + assert.equal(result.keyboardAvoided, true); + assert.equal(top, list.y); + assert.ok(bottom <= LOWER_HALF_KEYBOARD.y, `swipe endpoint ${bottom} landed on the keyboard`); + assert.equal(result.pixels, bottom - top); + }, + ); +}); + +test('scrollAndroid leaves a swipe that already fits its container unchanged', async () => { + const list = { x: 0, y: 100, width: 1080, height: 2100 }; + await withFakeAdb(helperRouteAdb({ ...PORTRAIT_WINDOW }), async ({ device }) => { + const unclamped = await scrollAndroid(device, 'down', { pixels: 600 }); + const result = await scrollAndroid(device, 'down', { pixels: 600, containerRect: list }); + + assert.deepEqual(result, unclamped); + }); +}); + test('scrollAndroid refuses rather than flinging into a keyboard that owns the window', async () => { // A landscape IME leaves 40px of a 900px window: a swipe there reads as a stuck surface, so the // command refuses with its own typed reason instead of the generic no-progress stop. diff --git a/packages/platform-android/src/input-actions.ts b/packages/platform-android/src/input-actions.ts index 49fbc0fbbc..342294b8f5 100644 --- a/packages/platform-android/src/input-actions.ts +++ b/packages/platform-android/src/input-actions.ts @@ -10,6 +10,7 @@ import { import { buildGesturePlan } from '@agent-device/contracts/gesture-plan'; import { GESTURE_DURATION_MIN_MS } from '@agent-device/contracts/gesture-plan-types'; import { + type ScrollContainerTarget, type ScrollReleaseBehavior, DEFAULT_MOBILE_SCROLL_DURATION_MS, } from '@agent-device/contracts/scroll-command'; @@ -220,7 +221,8 @@ export async function scrollAndroid( pixels?: number; durationMs?: number; releaseBehavior?: ScrollReleaseBehavior; - } & AndroidHelperSessionOptions, + } & ScrollContainerTarget & + AndroidHelperSessionOptions, ): Promise> { // The viewport read and the gesture are two helper calls one command apart: giving the read the // command's session scope keeps both on the same instrumentation. @@ -234,6 +236,9 @@ export async function scrollAndroid( pixels: options?.pixels, referenceWidth: swipeSurface.viewport.width, referenceHeight: swipeSurface.viewport.height, + ...(options?.containerRect === undefined + ? {} + : { container: relativeToViewport(options.containerRect, swipeSurface.viewport) }), }); const scrollPlan = { ...relativePlan, @@ -275,6 +280,11 @@ export async function scrollAndroid( }; } +/** The planner works in the swipe surface's zero-origin frame; snapshot rects are absolute. */ +function relativeToViewport(rect: Rect, viewport: Rect): Rect { + return { ...rect, x: rect.x - viewport.x, y: rect.y - viewport.y }; +} + /** * The band one Android scroll may swipe, refusing when the keyboard owns the window (#2500). * From 86214adf3da05b91c72cd5111085143abbd5bc06 Mon Sep 17 00:00:00 2001 From: Adam Rowe Date: Fri, 16 Oct 2026 20:24:00 -0400 Subject: [PATCH 2/2] fix(scroll): keep clamped swipe endpoints strictly inside the container The lane clamp treated the container's bottom edge as reachable, but Android bounds are half-open, so a shortened `scroll down` above a footer could start one pixel below the list. The usable band is now the container's half-open range inset by 8 px on both sides, on both axes. Along the scrolled axis it is intersected with the padded frame, which still wins when it leaves nothing of the container. Across it, the padded range is preferred, but a rail or chip row lying wholly in that padding is swiped along its own inset range rather than missed. A lane that only loses the rounding pixel of an odd span is no longer reported as shortened. Refs #2714 --- CHANGELOG.md | 1 + .../src/scroll-gesture-container.test.ts | 137 +++++++++++++---- packages/contracts/src/scroll-gesture.ts | 141 +++++++++++------- .../src/__tests__/input-actions.test.ts | 45 +++--- 4 files changed, 226 insertions(+), 98 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 89500c54b4..7d0c0d22e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- On adb-backed Android, a directional `scroll` keeps both swipe endpoints inside the scroll container the current snapshot resolves, inset from its edges and within the 10% edge padding, so a list under a header or above a footer no longer receives a drag that starts outside it and ends in `scroll_no_progress`. A swipe that already fits is unchanged; one longer than the container is shortened, and the response reports the dispatched `pixels` beside `requestedPixels`. Horizontal scrolls, scrolls after a mutation that has not been re-snapshotted, iOS, and WebDriver (Appium) sessions keep the unclamped swipe for now (#2714). - Add `uninstall ` and `client.apps.uninstall({ app })` to remove an installed app on adb-backed Android targets, iOS simulators, and CoreDevice iOS devices. Both report `wasInstalled`. A package name or bundle id that is not installed succeeds with `wasInstalled: false`. Other targets, Appium and Limrun sessions included, refuse it before anything runs. - Add `settings mobile-data on|off` for adb-backed Android targets. It switches mobile data through `svc data` and reports the `mobileData` state stored in the global `mobile_data` row; multi-SIM read-back is not covered yet. Every other target refuses it before anything runs, and `--app` is refused because the switch is device-wide. - On iOS, a `fill` that runs under a degraded XCTest snapshot channel now aims at a point, including ref, selector and replay fills. When the runner identifies no text input under that point, it refuses with `NO_TEXT_INPUT_AT_POINT` (`error.details.reason: no_text_input_at_point`, `dispatched: no`) before dispatching its focus tap, and no field changes; when the input lookup itself cannot answer, it refuses with `TEXT_INPUT_PROBE_UNAVAILABLE` (`error.details.reason: text_input_probe_unavailable`). Previously such a fill cleared and typed through the app-level first responder, so it could rewrite a field the caller never named, or report a clear failure after emptying it. A ref or selector fill whose rect centre misses the real field (an outer container, a non-input node) now needs a point inside the input's own rect. diff --git a/packages/contracts/src/scroll-gesture-container.test.ts b/packages/contracts/src/scroll-gesture-container.test.ts index e969e22b8c..591bf4df06 100644 --- a/packages/contracts/src/scroll-gesture-container.test.ts +++ b/packages/contracts/src/scroll-gesture-container.test.ts @@ -4,44 +4,81 @@ import type { Rect } from '@agent-device/kernel/snapshot'; import { buildScrollGesturePlan, type ScrollDirection } from './scroll-gesture.ts'; // The iPhone geometry #2714 was reported on: a 402x874 frame, the default 10% edge padding leaving -// y 87..787 for a swipe. +// x 40..362 and y 87..787 for a swipe. const FRAME = { referenceWidth: 402, referenceHeight: 874 }; +const PADDED_LEFT = 40; const PADDED_TOP = 87; const PADDED_BOTTOM = 787; +// Inset under a navigation bar and running to the bottom of the frame. const INSET_CONTAINER: Rect = { x: 18, y: 178, width: 366, height: 662 }; +// Inset at both ends: a footer sits below it, well above the padded band's bottom. +const FOOTERED_CONTAINER: Rect = { x: 18, y: 178, width: 366, height: 500 }; const FITTING_CONTAINER: Rect = { x: 18, y: 62, width: 366, height: 729 }; -function endpointsInside( - plan: { x1: number; y1: number; x2: number; y2: number }, - rect: Rect, -): boolean { +type Endpoints = { x1: number; y1: number; x2: number; y2: number }; + +/** + * A container rect is half-open, as Android bounds and View hit-testing are: `y + height` is the + * first row outside it. Strictly inside on both sides also keeps the drag off the edge row. + */ +function strictlyInside(plan: Endpoints, rect: Rect): boolean { const inside = (x: number, y: number) => - x >= rect.x && x <= rect.x + rect.width && y >= rect.y && y <= rect.y + rect.height; + x > rect.x && x < rect.x + rect.width && y > rect.y && y < rect.y + rect.height; return inside(plan.x1, plan.y1) && inside(plan.x2, plan.y2); } -test.each(['up', 'down'])( - 'a %s swipe longer than an inset container shrinks to the container and reports both spans', - (direction) => { +test.each<[ScrollDirection, string, Rect]>([ + ['up', 'inset', INSET_CONTAINER], + ['down', 'inset', INSET_CONTAINER], + ['up', 'footered', FOOTERED_CONTAINER], + ['down', 'footered', FOOTERED_CONTAINER], +])( + 'a %s swipe longer than an %s container shrinks inside it and reports both spans', + (direction, _, container) => { const unclamped = buildScrollGesturePlan({ direction, amount: 0.75, ...FRAME }); - const plan = buildScrollGesturePlan({ - direction, - amount: 0.75, - ...FRAME, - container: INSET_CONTAINER, - }); + const plan = buildScrollGesturePlan({ direction, amount: 0.75, ...FRAME, container }); - assert.ok(endpointsInside(plan, INSET_CONTAINER), JSON.stringify(plan)); - assert.deepEqual( - [Math.min(plan.y1, plan.y2), Math.max(plan.y1, plan.y2)], - [INSET_CONTAINER.y, PADDED_BOTTOM], - ); - assert.equal(plan.pixels, PADDED_BOTTOM - INSET_CONTAINER.y); + assert.ok(strictlyInside(plan, container), JSON.stringify(plan)); + assert.ok(Math.max(plan.y1, plan.y2) <= PADDED_BOTTOM, JSON.stringify(plan)); + assert.equal(plan.pixels, Math.abs(plan.y2 - plan.y1)); + assert.ok(plan.pixels < unclamped.pixels); assert.equal(plan.requestedPixels, unclamped.pixels); assert.equal(Math.sign(plan.y2 - plan.y1), Math.sign(unclamped.y2 - unclamped.y1)); }, ); +test.each(['left', 'right'])( + 'a %s swipe longer than a narrow container shrinks inside it along x', + (direction) => { + const column: Rect = { x: 100, y: 0, width: 200, height: 874 }; + const unclamped = buildScrollGesturePlan({ direction, ...FRAME }); + const plan = buildScrollGesturePlan({ direction, ...FRAME, container: column }); + + assert.ok(strictlyInside(plan, column), JSON.stringify(plan)); + assert.deepEqual([plan.y1, plan.y2], [unclamped.y1, unclamped.y2]); + assert.equal(plan.pixels, Math.abs(plan.x2 - plan.x1)); + assert.ok(plan.pixels < unclamped.pixels); + assert.equal(plan.requestedPixels, unclamped.pixels); + assert.equal(Math.sign(plan.x2 - plan.x1), Math.sign(unclamped.x2 - unclamped.x1)); + }, +); + +test('a horizontal swipe that fits an offset container moves into it without shortening', () => { + const rightPane: Rect = { x: 180, y: 0, width: 222, height: 874 }; + const unclamped = buildScrollGesturePlan({ direction: 'left', pixels: 100, ...FRAME }); + const plan = buildScrollGesturePlan({ + direction: 'left', + pixels: 100, + ...FRAME, + container: rightPane, + }); + + assert.ok(strictlyInside(plan, rightPane), JSON.stringify(plan)); + assert.equal(Math.abs(plan.x2 - plan.x1), Math.abs(unclamped.x2 - unclamped.x1)); + assert.equal(plan.pixels, unclamped.pixels); + assert.equal('requestedPixels' in plan, false); +}); + test.each(['up', 'down', 'left', 'right'])( 'a %s swipe already inside its container keeps today’s endpoints and distance', (direction) => { @@ -68,24 +105,72 @@ test('a swipe that fits an offset container moves into it without shortening', ( container: lowerContainer, }); - assert.ok(endpointsInside(plan, lowerContainer), JSON.stringify(plan)); - assert.equal(Math.min(plan.y1, plan.y2), lowerContainer.y); + assert.ok(strictlyInside(plan, lowerContainer), JSON.stringify(plan)); assert.equal(Math.abs(plan.y1 - plan.y2), Math.abs(unclamped.y1 - unclamped.y2)); assert.equal(plan.pixels, unclamped.pixels); assert.equal('requestedPixels' in plan, false); }); -test('a vertical swipe beside a narrow container runs down its centre line', () => { +test('an odd span that only loses its rounding pixel is not reported as shortened', () => { + // On an 875-point axis the padded band is 88..787 (699 points), and a 699-point request centred + // on 438 rounds its halves to a 700-point lane. Fitting it into the band drops that rounding + // pixel, not any of the travel asked for. + const frame = { referenceWidth: 402, referenceHeight: 875 }; + const wholeFrame: Rect = { x: 0, y: 0, width: 402, height: 875 }; + const plan = buildScrollGesturePlan({ + direction: 'up', + pixels: 699, + ...frame, + container: wholeFrame, + }); + + assert.equal(plan.pixels, 699); + assert.equal(Math.abs(plan.y2 - plan.y1), 699); + assert.equal('requestedPixels' in plan, false); +}); + +test('a vertical swipe beside a narrow container moves across into it', () => { const rightColumn: Rect = { x: 250, y: 0, width: 152, height: 874 }; const unclamped = buildScrollGesturePlan({ direction: 'up', ...FRAME }); const plan = buildScrollGesturePlan({ direction: 'up', ...FRAME, container: rightColumn }); - assert.equal(plan.x1, rightColumn.x + rightColumn.width / 2); + assert.ok(strictlyInside(plan, rightColumn), JSON.stringify(plan)); + assert.ok(plan.x1 <= FRAME.referenceWidth - PADDED_LEFT, `swipe x ${plan.x1} is in the padding`); assert.equal(plan.x2, plan.x1); assert.deepEqual([plan.y1, plan.y2, plan.pixels], [unclamped.y1, unclamped.y2, unclamped.pixels]); }); -test('a container lying wholly in the edge padding leaves the padded swipe as it was', () => { +test('a container reaching into the side padding is swiped in the part the padding leaves', () => { + const edgeColumn: Rect = { x: 0, y: 0, width: 60, height: 874 }; + const plan = buildScrollGesturePlan({ direction: 'up', ...FRAME, container: edgeColumn }); + + assert.ok(strictlyInside(plan, edgeColumn), JSON.stringify(plan)); + assert.ok(plan.x1 >= PADDED_LEFT, `swipe x ${plan.x1} is inside the side padding`); +}); + +test.each<[ScrollDirection, string, Rect]>([ + [ + 'up', + 'a navigation rail along the left edge', + { x: 0, y: 0, width: PADDED_LEFT - 1, height: 874 }, + ], + [ + 'left', + 'a chip row just above the padded band', + { x: 0, y: 60, width: 402, height: PADDED_TOP - 61 }, + ], +])( + 'a %s swipe aimed at %s runs inside it when the cross-axis padding leaves none of it', + (direction, _, container) => { + const unclamped = buildScrollGesturePlan({ direction, ...FRAME }); + const plan = buildScrollGesturePlan({ direction, ...FRAME, container }); + + assert.ok(strictlyInside(plan, container), JSON.stringify(plan)); + assert.equal(plan.pixels, unclamped.pixels); + }, +); + +test('a container lying wholly in the padding of the scrolled axis leaves the padded swipe as it was', () => { const statusBand: Rect = { x: 0, y: 0, width: 402, height: PADDED_TOP - 1 }; const unclamped = buildScrollGesturePlan({ direction: 'up', ...FRAME }); const plan = buildScrollGesturePlan({ direction: 'up', ...FRAME, container: statusBand }); diff --git a/packages/contracts/src/scroll-gesture.ts b/packages/contracts/src/scroll-gesture.ts index bf450867d8..9149863ccc 100644 --- a/packages/contracts/src/scroll-gesture.ts +++ b/packages/contracts/src/scroll-gesture.ts @@ -51,8 +51,10 @@ export type ScrollGestureOptions = { referenceHeight: number; /** * The scroll container the swipe is meant for, in the reference frame's own zero-origin space. - * Both endpoints stay inside its intersection with the padded frame (#2714): a span that already - * fits is unchanged, one that leaves it is moved in, and one longer than it is shortened. + * Both endpoints stay strictly inside its intersection with the padded frame (#2714): a span + * that already fits is unchanged, one that leaves it is moved in, and one longer than it is + * shortened. A container the padding leaves nothing of along the scrolled axis gets the padded + * swipe unchanged, since the safe bounds outrank the container. */ container?: Rect; }; @@ -106,95 +108,124 @@ export const DEFAULT_SCROLL_AMOUNT = 0.6; const DEFAULT_EDGE_PADDING_FRACTION = 0.1; // Edge presets stay close to the system gesture boundary without emitting edge coordinates. const SWIPE_PRESET_EDGE_MARGIN_PX = 8; +// A swipe kept inside a container stays this far off its edges, so the drag never starts on the +// container's boundary row. +const SCROLL_CONTAINER_EDGE_INSET = 8; export function buildScrollGesturePlan(options: ScrollGestureOptions): ScrollGesturePlan { const direction = options.direction; - const axisLength = - direction === 'up' || direction === 'down' ? options.referenceHeight : options.referenceWidth; + const vertical = direction === 'up' || direction === 'down'; + const axisLength = vertical ? options.referenceHeight : options.referenceWidth; + const crossLength = vertical ? options.referenceWidth : options.referenceHeight; const requestedAmount = resolveRequestedAmount(options.amount); const requestedPixels = options.pixels !== undefined ? normalizeRequestedPixels(options.pixels) : Math.round(axisLength * requestedAmount); - const edgePadding = Math.max(1, Math.round(axisLength * DEFAULT_EDGE_PADDING_FRACTION)); + const edgePadding = scrollEdgePadding(axisLength); const maxTravelPixels = Math.max(1, axisLength - edgePadding * 2); const travelPixels = Math.max(1, Math.min(requestedPixels, maxTravelPixels)); const halfTravel = Math.round(travelPixels / 2); - const centerX = Math.round(options.referenceWidth / 2); - const centerY = Math.round(options.referenceHeight / 2); - const buildPlan = (x1: number, y1: number, x2: number, y2: number): ScrollGesturePlan => ({ - direction, - x1, - y1, - x2, - y2, - referenceWidth: options.referenceWidth, - referenceHeight: options.referenceHeight, - amount: options.amount, - pixels: travelPixels, - }); - - const vertical = direction === 'up' || direction === 'down'; + const axisCenter = Math.round(axisLength / 2); const unclamped = { - axisStart: (vertical ? centerY : centerX) - halfTravel, - axisEnd: (vertical ? centerY : centerX) + halfTravel, - cross: vertical ? centerX : centerY, + axisStart: axisCenter - halfTravel, + axisEnd: axisCenter + halfTravel, + cross: Math.round(crossLength / 2), }; + const crossPadding = scrollEdgePadding(crossLength); const lane = options.container ? keepSwipeInContainer(unclamped, options.container, vertical, { - min: edgePadding, - max: axisLength - edgePadding, + axis: { min: edgePadding, max: axisLength - edgePadding }, + cross: { min: crossPadding, max: crossLength - crossPadding }, + crossFrame: { min: 0, max: crossLength }, }) : unclamped; - const shortened = lane.axisEnd - lane.axisStart < unclamped.axisEnd - unclamped.axisStart; - const plan = buildPlan( - vertical ? lane.cross : lane.axisStart, - vertical ? lane.axisStart : lane.cross, - vertical ? lane.cross : lane.axisEnd, - vertical ? lane.axisEnd : lane.cross, - ); - const placed = shortened - ? { ...plan, pixels: lane.axisEnd - lane.axisStart, requestedPixels: travelPixels } - : plan; - // `up`/`left` drag from the low end toward the high end; `down`/`right` drag the other way. - return direction === 'up' || direction === 'left' - ? placed - : { ...placed, x1: placed.x2, y1: placed.y2, x2: placed.x1, y2: placed.y1 }; + const laneLength = lane.axisEnd - lane.axisStart; + // Centring rounds an odd span up by one, so only a lane shorter than the travel itself is short. + const shortened = laneLength < travelPixels; + return { + direction, + ...laneEndpoints(direction, lane), + referenceWidth: options.referenceWidth, + referenceHeight: options.referenceHeight, + amount: options.amount, + pixels: shortened ? laneLength : travelPixels, + ...(shortened ? { requestedPixels: travelPixels } : {}), + }; +} + +/** `up`/`left` drag from the lane's low end toward its high end; `down`/`right` drag the other way. */ +function laneEndpoints( + direction: ScrollDirection, + lane: SwipeLane, +): Pick { + const vertical = direction === 'up' || direction === 'down'; + const low = vertical + ? { x: lane.cross, y: lane.axisStart } + : { x: lane.axisStart, y: lane.cross }; + const high = vertical ? { x: lane.cross, y: lane.axisEnd } : { x: lane.axisEnd, y: lane.cross }; + const [start, end] = direction === 'up' || direction === 'left' ? [low, high] : [high, low]; + return { x1: start.x, y1: start.y, x2: end.x, y2: end.y }; } type SwipeLane = { axisStart: number; axisEnd: number; cross: number }; +type CoordinateRange = { min: number; max: number }; + +function scrollEdgePadding(length: number): number { + return Math.max(1, Math.round(length * DEFAULT_EDGE_PADDING_FRACTION)); +} + /** - * Moves a centred swipe lane inside the container's intersection with the padded axis, shortening - * it only when the intersection is shorter than the lane. A container wholly inside the padding - * leaves nothing to swipe in, so the lane stays as the padded frame planned it. + * Moves a centred swipe lane inside the container's intersection with the padded frame, + * shortening it only when that intersection is shorter than the lane. The container is half-open, + * as Android bounds and View hit-testing are, and inset from its edges, so every endpoint lands on + * a row the container owns. A container the padding leaves nothing of on the scrolled axis keeps + * the lane the padded frame planned: the safe bounds outrank the container, and the movement read + * reports the miss as `container-outside-swipe` when nothing else moved. */ function keepSwipeInContainer( lane: SwipeLane, container: Rect, vertical: boolean, - padded: { min: number; max: number }, + padded: { axis: CoordinateRange; cross: CoordinateRange; crossFrame: CoordinateRange }, ): SwipeLane { - const containerMin = vertical ? container.y : container.x; - const containerSize = vertical ? container.height : container.width; - const bandMin = Math.max(padded.min, Math.round(containerMin)); - const bandMax = Math.min(padded.max, Math.round(containerMin + containerSize)); - if (bandMax <= bandMin) return lane; - - const crossMin = vertical ? container.x : container.y; + const band = insetContainerRange( + vertical ? container.y : container.x, + vertical ? container.height : container.width, + padded.axis, + ); + const crossStart = vertical ? container.x : container.y; const crossSize = vertical ? container.width : container.height; + // The padding guards the scrolled axis against system bars. Across it, a rail or chip row that + // sits wholly in the padding is still the surface asked for, so the swipe runs along it there. + const crossBand = + insetContainerRange(crossStart, crossSize, padded.cross) ?? + insetContainerRange(crossStart, crossSize, padded.crossFrame); + if (!band || !crossBand) return lane; + const cross = - lane.cross >= crossMin && lane.cross <= crossMin + crossSize + lane.cross >= crossBand.min && lane.cross <= crossBand.max ? lane.cross - : Math.round(crossMin + crossSize / 2); - + : Math.round((crossBand.min + crossBand.max) / 2); const length = lane.axisEnd - lane.axisStart; - if (length >= bandMax - bandMin) return { axisStart: bandMin, axisEnd: bandMax, cross }; + if (length >= band.max - band.min) return { axisStart: band.min, axisEnd: band.max, cross }; const shift = - lane.axisStart < bandMin ? bandMin - lane.axisStart : Math.min(0, bandMax - lane.axisEnd); + lane.axisStart < band.min ? band.min - lane.axisStart : Math.min(0, band.max - lane.axisEnd); return { axisStart: lane.axisStart + shift, axisEnd: lane.axisEnd + shift, cross }; } +/** The coordinates of `[start, start + size)` an inset swipe may use inside `padded`, if any. */ +function insetContainerRange( + start: number, + size: number, + padded: CoordinateRange, +): CoordinateRange | undefined { + const min = Math.max(padded.min, Math.ceil(start) + SCROLL_CONTAINER_EDGE_INSET); + const max = Math.min(padded.max, Math.floor(start + size) - 1 - SCROLL_CONTAINER_EDGE_INSET); + return max > min ? { min, max } : undefined; +} + /** * Validates pre-frame scroll inputs (amount/pixels) the same way buildScrollGesturePlan would, * so the daemon throws INVALID_ARGS for bad inputs BEFORE sending the fused runner `scroll` diff --git a/packages/platform-android/src/__tests__/input-actions.test.ts b/packages/platform-android/src/__tests__/input-actions.test.ts index 29906d8337..0af4e6b3ea 100644 --- a/packages/platform-android/src/__tests__/input-actions.test.ts +++ b/packages/platform-android/src/__tests__/input-actions.test.ts @@ -139,22 +139,33 @@ function swipeSpan(result: Record): [number, number] { return [Math.min(...ys), Math.max(...ys)]; } -test('scrollAndroid keeps both endpoints inside an inset container and reports the shortened span', async () => { - // A window below the status bar, holding a list that starts under a tall header: the unclamped - // 0.8 swipe would start in the header, so the gesture is shortened to the list (#2714). - const offsetWindow = { x: 0, y: 136, width: 1080, height: 2264 }; - const list = { x: 0, y: 436, width: 1080, height: 1700 }; - await withFakeAdb(helperRouteAdb(offsetWindow), async ({ device }) => { - const unclamped = await scrollAndroid(device, 'down', { amount: 0.8 }); - const result = await scrollAndroid(device, 'down', { amount: 0.8, containerRect: list }); - - assert.ok(swipeSpan(unclamped)[0] < list.y, 'the fixture must start outside the list'); - assert.deepEqual(swipeSpan(result), [list.y, list.y + list.height]); - assert.ok(Number(result.y1) > Number(result.y2), 'a down scroll still drags upward'); - assert.equal(result.pixels, list.height); - assert.equal(result.requestedPixels, unclamped.pixels); - }); -}); +function strictlyInsideRows(span: [number, number], rect: { y: number; height: number }): boolean { + // Android bounds are half-open: `y + height` is the first row the container does not own. + return span[0] > rect.y && span[1] < rect.y + rect.height; +} + +test.each(['down', 'up'] as const)( + 'scrollAndroid keeps a %s swipe inside a list with a footer below it and reports the shortened span', + async (direction) => { + // A window below the status bar, holding a list between a tall header and a footer: the + // unclamped 0.8 swipe would start on the header or the footer, so the gesture is shortened to + // rows the list owns (#2714). + const offsetWindow = { x: 0, y: 136, width: 1080, height: 2264 }; + const list = { x: 0, y: 436, width: 1080, height: 1500 }; + await withFakeAdb(helperRouteAdb(offsetWindow), async ({ device }) => { + const unclamped = await scrollAndroid(device, direction, { amount: 0.8 }); + const result = await scrollAndroid(device, direction, { amount: 0.8, containerRect: list }); + const span = swipeSpan(result); + + assert.ok(swipeSpan(unclamped)[0] < list.y, 'the fixture must start outside the list'); + assert.ok(swipeSpan(unclamped)[1] >= list.y + list.height, 'and end below it'); + assert.ok(strictlyInsideRows(span, list), `swipe ${span} leaves the list`); + assert.equal(Number(result.y1) > Number(result.y2), direction === 'down'); + assert.equal(result.pixels, span[1] - span[0]); + assert.equal(result.requestedPixels, unclamped.pixels); + }); + }, +); test('scrollAndroid keeps the swipe inside a container that runs under the keyboard', async () => { // The keyboard clip alone keeps the frame's top, so a long swipe would start on the header above @@ -167,7 +178,7 @@ test('scrollAndroid keeps the swipe inside a container that runs under the keybo const [top, bottom] = swipeSpan(result); assert.equal(result.keyboardAvoided, true); - assert.equal(top, list.y); + assert.ok(top > list.y, `swipe start ${top} is on the list's top edge or above it`); assert.ok(bottom <= LOWER_HALF_KEYBOARD.y, `swipe endpoint ${bottom} landed on the keyboard`); assert.equal(result.pixels, bottom - top); },