Skip to content

Commit f167fb5

Browse files
thymikeethymikee
committed
fix(interaction): keep a cross-surface look from certifying rest (#3471)
The re-observations that certify a resting position agreed on the rect alone, while the screening that decides whether to recheck at all already compares surfaces. The same line has to hold in both: an iOS system sheet presented during the sample gap carries the same control at the same rect, and the gate would have read that as rest and dispatched onto the sheet (#2438's transition, guarding the other half of the same decision). A measured look now carries the surface it was measured on, and two looks agree only on one surface. The unmeasured refusals also pin what they withhold — no `movedToRect` when no look measured a position — and the `@ref` branch of the published recovery is now asserted on a real refusal rather than in the contracts suite the hint left.
1 parent 9299300 commit f167fb5

2 files changed

Lines changed: 70 additions & 5 deletions

File tree

‎src/commands/interaction/runtime/moving-target-gate.test.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,12 +271,50 @@ test('a ref the second observation cannot re-measure is refused, never self-agre
271271
const details = (error as { details?: Record<string, unknown> }).details;
272272
assert.equal(details?.reason, INTERACTION_ERROR_REASONS.targetStillMoving);
273273
assert.equal(details?.dispatched, 'no');
274+
// Nothing was measured, so nothing is reported: a caller reading `movedToRect` would take the
275+
// stale frame position for a fresh observation of where the target went.
276+
assert.equal(details?.movedToRect, undefined);
277+
// The `@ref` branch of the recovery, asserted on a real refusal rather than on the string: the
278+
// hint's only test was the contracts unit suite, and the hint now lives beside this refusal.
279+
assert.match(String(details?.hint), /snapshot -i/);
280+
assert.match(String(details?.hint), /ref frame/);
274281
return true;
275282
},
276283
);
277284
assert.deepEqual(device.taps(), [], 'an unmeasured position licenses no tap');
278285
});
279286

287+
test('a surface that arrives between the two looks is not rest', async () => {
288+
// Rest is one position on one surface. The screening already refuses to compare positions across
289+
// surfaces for exactly the #2438 reason — a system sheet can carry the same control at the same
290+
// rect — and the two looks that certify rest are the same comparison. Without the surface check, a
291+
// sheet presented during the sample gap would let a press land on it while reporting agreement.
292+
const sheet = 'com.apple.springboard';
293+
const device = createGuardedDevice({
294+
stored: slideTree(MID_FLIGHT_X),
295+
trees: [
296+
// The resolution's own capture, then the two looks: the first on the app's content at rest,
297+
// the second on a sheet presented during the sample gap — same control, same rect, another
298+
// surface.
299+
slideTree(MID_FLIGHT_X),
300+
slideTree(RESTING_X),
301+
slideTree(RESTING_X, { surfaceBundleId: sheet }),
302+
],
303+
...DISTURBING_HISTORY,
304+
});
305+
306+
await assert.rejects(
307+
() => device.press('selector'),
308+
(error: unknown) => {
309+
const details = (error as { details?: Record<string, unknown> }).details;
310+
assert.equal(details?.reason, INTERACTION_ERROR_REASONS.targetStillMoving);
311+
assert.equal(details?.dispatched, 'no');
312+
return true;
313+
},
314+
);
315+
assert.deepEqual(device.taps(), [], 'the sheet never received a tap');
316+
});
317+
280318
test('a target the wait loses is refused by resolution, not by a motion refusal', async () => {
281319
// The recheck asked the same question again and the answer was "no such element". That is a more
282320
// accurate account of a vanished target than "still moving" would be, and the caller's retry is a
@@ -346,6 +384,9 @@ test('a replacement that took the ref at the frame coordinates is refused, not a
346384
(error: unknown) => {
347385
const details = (error as { details?: Record<string, unknown> }).details;
348386
assert.equal(details?.reason, INTERACTION_ERROR_REASONS.targetStillMoving);
387+
// A look that measured no target reports no position: the frame rect is the value the whole
388+
// window exists to doubt, and publishing it as `movedToRect` would read as a fresh observation.
389+
assert.equal(details?.movedToRect, undefined);
349390
return true;
350391
},
351392
);

‎src/commands/interaction/runtime/moving-target-gate.ts‎

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -151,26 +151,45 @@ async function resolveAtRestTarget(
151151
if (!first.measured) throw motionRefusal(resolved, undefined, now(runtime) - waitedFrom);
152152
await sleep(runtime, MOVING_TARGET_GATE_SAMPLE_GAP_MS);
153153
const second = await reObserveTarget(runtime, options, params);
154-
if (second.measured && movingTargetRectsStable(first.rect, second.rect)) return second.resolved;
154+
if (second.measured && looksAgree(first, second)) return second.resolved;
155155
throw motionRefusal(resolved, second.rect, now(runtime) - waitedFrom);
156156
}
157157

158+
/**
159+
* Rest is one position on one surface. A system sheet presented between the two looks changes what
160+
* the coordinates mean even when the rect repeats (#2438), which is why the screening compares
161+
* surfaces too; the same line has to hold between the two looks that certify rest.
162+
*/
163+
function looksAgree(first: MovingTargetMeasured, second: MovingTargetMeasured): boolean {
164+
return (
165+
first.surfaceBundleId === second.surfaceBundleId &&
166+
movingTargetRectsStable(first.rect, second.rect)
167+
);
168+
}
169+
158170
/** One look at the target: what it resolved to, and whether that look measured a position. */
159171
type MovingTargetObservation = MovingTargetMeasurement & {
160172
resolved: ResolvedInteractionTarget;
161173
};
162174

163175
/** Whether a look produced a position of its own, or nothing to compare against. */
164176
type MovingTargetMeasurement =
165-
/** The position this observation's own capture reported for the target. */
166-
| { measured: true; rect: Rect }
177+
/** The position this observation's own capture reported for the target, on that capture's surface. */
178+
| MovingTargetMeasured
167179
/**
168180
* No position of its own. Two of these would "agree" trivially — rest certified by a
169181
* non-measurement, the one outcome the gate must never produce — so the first of them ends the
170182
* recheck, and a refusal reports no `movedToRect` rather than a stale one.
171183
*/
172184
| { measured: false; rect: undefined };
173185

186+
/** A look that measured the target: where it put it, and on which surface. */
187+
type MovingTargetMeasured = {
188+
measured: true;
189+
rect: Rect;
190+
surfaceBundleId: string | undefined;
191+
};
192+
174193
/**
175194
* One observation of the same target, asked as the original question rather than a narrower one.
176195
*
@@ -190,8 +209,11 @@ async function reObserveTarget(
190209
if (options.target.kind !== 'ref') {
191210
const resolved = await resolveInteractionTarget(runtime, options, params);
192211
const rect = resolvedRect(resolved);
212+
// A selector resolution measured its target on the surface it resolved against, which it
213+
// publishes as `preAction` along with the tree.
214+
const surfaceBundleId = resolved.preAction?.surfaceBundleId;
193215
return rect
194-
? { measured: true, resolved, rect }
216+
? { measured: true, resolved, rect, surfaceBundleId }
195217
: { measured: false, resolved, rect: undefined };
196218
}
197219
const { snapshot } = await captureInteractionSnapshot(runtime, options, true);
@@ -226,7 +248,9 @@ function measureRefObservation(
226248
captured !== undefined &&
227249
movingTargetRectsStable(captured.rect, rect) &&
228250
localIdentitiesEqual(readNodeLocalIdentity(captured), readNodeLocalIdentity(dispatchedNode));
229-
return measured ? { measured: true, rect } : { measured: false, rect: undefined };
251+
return measured
252+
? { measured: true, rect, surfaceBundleId: snapshot.iosSystemSurfaceBundleId }
253+
: { measured: false, rect: undefined };
230254
}
231255

232256
function resolvedRect(resolved: ResolvedInteractionTarget): Rect | undefined {

0 commit comments

Comments
 (0)