Skip to content

Commit b98e645

Browse files
committed
fix(ios): keep the keyboard clip out of the scroll's rotation basis
`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.
1 parent 1a41440 commit b98e645

4 files changed

Lines changed: 159 additions & 44 deletions

File tree

.github/workflows/ios.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ jobs:
167167
-only-testing:AgentDeviceRunnerUITests/RunnerTests/testCoordinateTapTextInputProbeSkipsPenalizedXCTestChannel \
168168
-only-testing:AgentDeviceRunnerUITests/RunnerTests/testScrollViewportKeyboardClipMatchesGoldenParityTable \
169169
-only-testing:AgentDeviceRunnerUITests/RunnerTests/testScrollViewportPolicyUsesParityTableConstants \
170+
-only-testing:AgentDeviceRunnerUITests/RunnerTests/testScrollViewportDispatchKeepsTheUnclippedFrameAsItsCoordinateRotationBasis \
170171
-only-testing:AgentDeviceRunnerUITests/RunnerTests/testSynthesizedGesturePoliciesMatchCommandContracts \
171172
-only-testing:AgentDeviceRunnerUITests/RunnerTests/testFreshCoordinateTapContainsUnavailableTextInputProbe \
172173
-only-testing:AgentDeviceRunnerUITests/RunnerTests/testTextInputProbeIssueScopeIsThreadBound \

apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerScrollViewportPolicy.swift

Lines changed: 131 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,71 @@ enum RunnerScrollKeyboardClip: Equatable {
3030

3131
/** Where one directional scroll may place its swipe, once the keyboard has taken its share. */
3232
enum RunnerScrollViewport {
33-
/** The frame to plan inside, plus the keyboard top when the swipe was clipped for one. */
34-
case swipe(frame: CGRect, keyboardMinY: Double?)
33+
/**
34+
* The band to plan the swipe inside, the frame to rotate its coordinates against, and the keyboard
35+
* top when the band was clipped for one. The two frames are separate on purpose: a clip shortens
36+
* only the band, while `nativeSynthesizedPoint` derives a `landscapeRight` native x from the
37+
* frame's HEIGHT, so rotating inside the band moves the dispatched path sideways off the planned
38+
* one.
39+
*/
40+
case swipe(planFrame: CGRect, coordinateFrame: CGRect, keyboardMinY: Double?)
3541
/** Nothing to swipe. The caller answers `occlusionRunnerCode` and performs no gesture. */
3642
case occluded(keyboardMinY: Double, visibleHeight: Double)
3743
}
3844

45+
/// The gesture one directional scroll dispatches, built from a resolved viewport in one place so the
46+
/// band the plan was made inside and the frame its coordinates rotate against cannot be swapped.
47+
struct ScrollGestureDispatch {
48+
let plan: RunnerScrollGesturePlan
49+
let planFrame: CGRect
50+
let coordinateFrame: CGRect
51+
let keyboardMinY: Double?
52+
}
53+
54+
/** What a resolved viewport turns into for the command: a gesture, or the reason there is none. */
55+
enum ScrollGestureOutcome {
56+
case gesture(ScrollGestureDispatch)
57+
case unusableFrame
58+
case unusablePlan
59+
case occluded(keyboardMinY: Double, visibleHeight: Double)
60+
}
61+
62+
extension RunnerScrollViewport {
63+
/// Plans the swipe inside the band the keyboard left and keeps the viewport as the coordinate basis,
64+
/// so a clip shortens the travel without moving the gesture's lane.
65+
func gestureDispatch(
66+
direction: RunnerScrollDirection,
67+
amount: Double?,
68+
pixels: Double?
69+
) -> ScrollGestureOutcome {
70+
switch self {
71+
case .occluded(let keyboardMinY, let visibleHeight):
72+
return .occluded(keyboardMinY: keyboardMinY, visibleHeight: visibleHeight)
73+
case .swipe(let planFrame, let coordinateFrame, let keyboardMinY):
74+
guard planFrame.width > 0, planFrame.height > 0 else {
75+
return .unusableFrame
76+
}
77+
guard let plan = runnerScrollGesturePlan(
78+
direction: direction,
79+
amount: amount,
80+
pixels: pixels,
81+
referenceWidth: planFrame.width,
82+
referenceHeight: planFrame.height
83+
) else {
84+
return .unusablePlan
85+
}
86+
return .gesture(
87+
ScrollGestureDispatch(
88+
plan: plan,
89+
planFrame: planFrame,
90+
coordinateFrame: coordinateFrame,
91+
keyboardMinY: keyboardMinY
92+
)
93+
)
94+
}
95+
}
96+
}
97+
3998
enum ScrollViewportPolicy {
4099
/** Below this fraction of the viewport, the clipped band cannot hold a reliable swipe. */
41100
static let minVisibleFraction: Double = 0.15
@@ -82,6 +141,20 @@ enum ScrollViewportPolicy {
82141
)
83142
}
84143

144+
/// Splits a clip verdict into the two frames a dispatch needs. The gesture planner runs inside the
145+
/// clipped band; the coordinate rotation keeps the frame the viewport was resolved against, because
146+
/// the rotation basis is a property of the screen, not of what the keyboard left free.
147+
static func frames(referenceFrame: CGRect, clip: RunnerScrollKeyboardClip) -> RunnerScrollViewport {
148+
switch clip {
149+
case .unobstructed:
150+
return .swipe(planFrame: referenceFrame, coordinateFrame: referenceFrame, keyboardMinY: nil)
151+
case .avoided(let frame, let keyboardMinY):
152+
return .swipe(planFrame: frame, coordinateFrame: referenceFrame, keyboardMinY: keyboardMinY)
153+
case .occluded(let keyboardMinY, let visibleHeight):
154+
return .occluded(keyboardMinY: keyboardMinY, visibleHeight: visibleHeight)
155+
}
156+
}
157+
85158
private static func isUsable(_ rect: CGRect) -> Bool {
86159
return [rect.minX, rect.minY, rect.width, rect.height].allSatisfy(\.isFinite)
87160
&& rect.width > 0 && rect.height > 0
@@ -102,35 +175,35 @@ extension RunnerTests {
102175
// all: a policy that forbids the probe, and a probe that finds no keyboard.
103176
guard context.allowsKeyboardProbe else {
104177
logScrollViewport(decision: "probeSkipped", keyboardMinY: nil, swipeHeight: context.referenceFrame.height, context: context)
105-
return .swipe(frame: context.referenceFrame, keyboardMinY: nil)
178+
return ScrollViewportPolicy.frames(referenceFrame: context.referenceFrame, clip: .unobstructed)
106179
}
107180
guard let keyboardFrame = visibleKeyboardFrame(app: app) else {
108181
logScrollViewport(decision: "noKeyboard", keyboardMinY: nil, swipeHeight: context.referenceFrame.height, context: context)
109-
return .swipe(frame: context.referenceFrame, keyboardMinY: nil)
182+
return ScrollViewportPolicy.frames(referenceFrame: context.referenceFrame, clip: .unobstructed)
110183
}
111-
switch ScrollViewportPolicy.clip(viewport: context.referenceFrame, keyboard: keyboardFrame) {
184+
let clip = ScrollViewportPolicy.clip(viewport: context.referenceFrame, keyboard: keyboardFrame)
185+
switch clip {
112186
case .unobstructed:
113187
logScrollViewport(decision: "unobstructed", keyboardMinY: nil, swipeHeight: context.referenceFrame.height, context: context)
114-
return .swipe(frame: context.referenceFrame, keyboardMinY: nil)
115188
case .avoided(let frame, let keyboardMinY):
116189
logScrollViewport(
117190
decision: "avoided",
118191
keyboardMinY: keyboardMinY,
119192
swipeHeight: frame.height,
120193
context: context
121194
)
122-
return .swipe(frame: frame, keyboardMinY: keyboardMinY)
123195
case .occluded(let keyboardMinY, let visibleHeight):
124196
logScrollViewport(
125197
decision: "occluded",
126198
keyboardMinY: keyboardMinY,
127199
swipeHeight: visibleHeight,
128200
context: context
129201
)
130-
return .occluded(keyboardMinY: keyboardMinY, visibleHeight: visibleHeight)
131202
}
203+
return ScrollViewportPolicy.frames(referenceFrame: context.referenceFrame, clip: clip)
132204
#else
133-
return .swipe(frame: resolvedTouchReferenceFrame(app: app, appFrame: app.frame), keyboardMinY: nil)
205+
let fallbackFrame = resolvedTouchReferenceFrame(app: app, appFrame: app.frame)
206+
return ScrollViewportPolicy.frames(referenceFrame: fallbackFrame, clip: .unobstructed)
134207
#endif
135208
}
136209

@@ -237,6 +310,55 @@ extension RunnerTests {
237310
XCTAssertEqual(constants.accessoryAllowance, ScrollViewportPolicy.accessoryAllowance)
238311
}
239312

313+
/// A clipped landscape band shortens the frame, and `nativeSynthesizedPoint` derives a
314+
/// `landscapeRight` native x from the frame's HEIGHT. Rotating inside the band therefore moves the
315+
/// dispatched path sideways by exactly what the keyboard took, off the lane the plan was built for,
316+
/// so the plan band and the coordinate basis stay separate values through dispatch (#2500).
317+
func testScrollViewportDispatchKeepsTheUnclippedFrameAsItsCoordinateRotationBasis() throws {
318+
let viewport = CGRect(x: 0, y: 0, width: 1210, height: 834)
319+
let keyboard = CGRect(x: 0, y: 588, width: 1210, height: 246)
320+
let clip = ScrollViewportPolicy.clip(viewport: viewport, keyboard: keyboard)
321+
guard case .avoided(let band, let keyboardMinY) = clip else {
322+
return XCTFail("expected a landscape keyboard to be avoided, got \(clip)")
323+
}
324+
XCTAssertEqual(band.height, 576)
325+
326+
guard case .gesture(let gesture) = ScrollViewportPolicy.frames(
327+
referenceFrame: viewport,
328+
clip: clip
329+
).gestureDispatch(direction: .up, amount: nil, pixels: nil) else {
330+
return XCTFail("expected a gesture inside the clipped band")
331+
}
332+
XCTAssertEqual(gesture.planFrame, band)
333+
XCTAssertEqual(gesture.keyboardMinY, keyboardMinY)
334+
XCTAssertEqual(gesture.coordinateFrame, viewport, "the rotation basis must survive the clip")
335+
XCTAssertLessThanOrEqual(
336+
max(gesture.plan.y1, gesture.plan.y2),
337+
keyboard.minY - ScrollViewportPolicy.accessoryAllowance,
338+
"a landscape swipe must stay clear of the keys"
339+
)
340+
341+
let orientedStartY = gesture.planFrame.minY + gesture.plan.y1
342+
let dispatched = nativeSynthesizedPoint(
343+
orientedX: gesture.planFrame.minX + gesture.plan.x1,
344+
orientedY: orientedStartY,
345+
in: gesture.coordinateFrame,
346+
interfaceOrientation: RunnerInterfaceOrientation.landscapeRight
347+
)
348+
let clippedBasis = nativeSynthesizedPoint(
349+
orientedX: gesture.planFrame.minX + gesture.plan.x1,
350+
orientedY: orientedStartY,
351+
in: gesture.planFrame,
352+
interfaceOrientation: RunnerInterfaceOrientation.landscapeRight
353+
)
354+
XCTAssertEqual(
355+
dispatched.x - clippedBasis.x,
356+
viewport.height - band.height,
357+
accuracy: 0.001,
358+
"rotating inside the clipped band would shift native x by what the keyboard took"
359+
)
360+
}
361+
240362
private func loadScrollViewportPolicyFixture() throws -> ScrollViewportPolicyFixture {
241363
let fixtureURL = URL(fileURLWithPath: #filePath)
242364
.deletingLastPathComponent() // AgentDeviceRunnerUITests

apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift

Lines changed: 26 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1954,58 +1954,50 @@ extension RunnerTests {
19541954
)
19551955
}
19561956
let viewport = resolvedScrollViewport(app: activeApp, context: scrollContext)
1957-
let frame: CGRect
1958-
let keyboardMinY: Double?
1959-
switch viewport {
1957+
let defaults = runnerDragCommandDefaults(command)
1958+
switch viewport.gestureDispatch(
1959+
direction: direction,
1960+
amount: defaults.scrollAmount,
1961+
pixels: command.pixels
1962+
) {
19601963
case .occluded(let occlusionKeyboardMinY, let visibleHeight):
19611964
return scrollKeyboardOccludedResponse(
19621965
direction: direction.rawValue,
19631966
keyboardMinY: occlusionKeyboardMinY,
19641967
visibleHeight: visibleHeight
19651968
)
1966-
case .swipe(let swipeFrame, let clippedAboveKeyboardMinY):
1967-
frame = swipeFrame
1968-
keyboardMinY = clippedAboveKeyboardMinY
1969-
}
1970-
guard frame.width > 0, frame.height > 0 else {
1969+
case .unusableFrame:
19711970
return Response(
19721971
ok: false,
19731972
error: ErrorPayload(message: "scroll could not resolve a usable interaction frame")
19741973
)
1975-
}
1976-
let defaults = runnerDragCommandDefaults(command)
1977-
guard let plan = runnerScrollGesturePlan(
1978-
direction: direction,
1979-
amount: defaults.scrollAmount,
1980-
pixels: command.pixels,
1981-
referenceWidth: frame.width,
1982-
referenceHeight: frame.height
1983-
) else {
1974+
case .unusablePlan:
19841975
return Response(
19851976
ok: false,
19861977
error: ErrorPayload(
19871978
code: "INVALID_ARGS",
19881979
message: "scroll could not compute a gesture plan"
19891980
)
19901981
)
1982+
case .gesture(let gesture):
1983+
guard scrollDurationIsValid(command.durationMs) else {
1984+
return invalidScrollDurationResponse(commandName: "scroll")
1985+
}
1986+
return attachingScrollViewportEvidence(
1987+
executeScrollDragGesture(
1988+
activeApp: activeApp,
1989+
x: gesture.planFrame.minX + gesture.plan.x1,
1990+
y: gesture.planFrame.minY + gesture.plan.y1,
1991+
x2: gesture.planFrame.minX + gesture.plan.x2,
1992+
y2: gesture.planFrame.minY + gesture.plan.y2,
1993+
durationMs: defaults.durationMs,
1994+
message: "scrolled",
1995+
context: scrollContext.withReferenceFrame(gesture.coordinateFrame),
1996+
releaseBehavior: command.scrollReleaseBehavior
1997+
),
1998+
keyboardMinY: gesture.keyboardMinY
1999+
)
19912000
}
1992-
guard scrollDurationIsValid(command.durationMs) else {
1993-
return invalidScrollDurationResponse(commandName: "scroll")
1994-
}
1995-
return attachingScrollViewportEvidence(
1996-
executeScrollDragGesture(
1997-
activeApp: activeApp,
1998-
x: frame.minX + plan.x1,
1999-
y: frame.minY + plan.y1,
2000-
x2: frame.minX + plan.x2,
2001-
y2: frame.minY + plan.y2,
2002-
durationMs: defaults.durationMs,
2003-
message: "scrolled",
2004-
context: scrollContext.withReferenceFrame(frame),
2005-
releaseBehavior: command.scrollReleaseBehavior
2006-
),
2007-
keyboardMinY: keyboardMinY
2008-
)
20092001
case .desktopScroll:
20102002
guard let rawDirection = command.direction,
20112003
let direction = RunnerScrollDirection(rawValue: rawDirection)

apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Interaction.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ private struct RunnerUnsupportedOperationError: LocalizedError {
1111
var errorDescription: String? { message }
1212
}
1313

14-
private enum RunnerInterfaceOrientation {
14+
enum RunnerInterfaceOrientation {
1515
#if AGENT_DEVICE_RUNNER_UNIT_TESTS
1616
static let unknown = 0
1717
#endif

0 commit comments

Comments
 (0)