Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
There was a problem hiding this comment.
Changes requested: the new wheel and pen drivers deliver what they promise (exact CSS-px wheel deltas on the 3× phone project, trusted pen pointers with pressure and scripted times), but in Chromium wheel() resolves before the page has received the step whenever the page's wheel listeners are all passive. With the self-test's first wheel case minus takePinches, the ctrl step is missing when the log is read right after the call and present 300 ms later, and nothing zooms, so the README's engine-limit row and the webInput.ts header misdescribe this as a ctrl-only rule. Please make the Chromium step wait until the page has it (or document that it resolves at dispatch), correct those notes and the test comment, and add a self-test that reads the log right after a step on a page with only passive wheel listeners. Separately, spec:AST-094 FR43 is not on main yet (#8646 is still open), so cite spec:AST-100 FR4 or land this after #8646.
[Automated review]
| await cdp.send('Input.dispatchMouseEvent', { | ||
| type: 'mouseWheel', | ||
| x, | ||
| y, | ||
| deltaX: dx * scale, | ||
| deltaY: dy * scale, | ||
| modifiers: ctrl ? 2 : 0, | ||
| }); |
There was a problem hiding this comment.
In Chromium this send resolves before the page has received the step whenever the page's wheel listeners are all passive, and every page with the input log has one. With the self-test's first wheel case minus takePinches, the ctrl step is missing when the log is read right after wheel() and present 300 ms later. Could wheel wait until the page has the step, for example by arming a one-shot capture-phase wheel listener with page.evaluateHandle before cdp.send and awaiting it with a timeout?
There was a problem hiding this comment.
Fixed in ad9e942: wheel arms a one-shot passive capture listener before the dispatch and waits until the page has the step (a 2 s limit, then it throws), in WebKit too. A new self-test reads the log right after each step on the input log's all-passive page: on 9e328e7 it fails (length 0), now it passes.
| | Lifting one finger while others stay down | yes | pointer events right, but that `touchend` lists no remaining `touches`: skip with `PARTIAL_LIFT_SKIP` | | ||
| | Click synthesised from a touch | yes | only from `driver.tap` (WebKit's tap gesture), not from a raw touch sequence | | ||
| | Curve sampling, storyboards, input log | yes | yes | | ||
| | Wheel steps, exact CSS px deltas (`webInput.ts`) | yes (a ctrl step reaches only a page with a non-passive wheel listener) | desktop context only (`isMobile: false`): skip with `WHEEL_SKIP` | |
There was a problem hiding this comment.
With this config (Playwright 1.61.1, Chromium 149, phone and desktop contexts), a ctrl step on a page with only passive wheel listeners does reach the page (ctrlKey: true) and nothing zooms: the visual viewport scale and devicePixelRatio are unchanged. It just arrives after wheel() resolves, like any step on such a page; a non-passive listener only makes Chromium dispatch it before wheel() returns. The same wording is in webInput.ts lines 19-22 and in the self-test comment.
There was a problem hiding this comment.
Fixed in ad9e942: right, it was delivery timing, not Control; a ctrl step reaches an all-passive page too and nothing zooms. The README row, the webInput.ts header and the self-test comment now say that wheel waits for the page there; the ctrl-only note is gone.
| | --------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `scripts.ts` | Touch scripts as data: `tap`, `drag` (hold, rest), `flick` (exact final-move speed, velocity profiles), `holdThenDrag`, `pinch`, `rotate`, `path`, `merge` (a second finger mid-gesture), `releaseVelocity`, `toTouchEvents`. | | ||
| | `touch.ts` | `touchDriver(page).play(script)`: the script as real engine input. `installInputLog` / `readInputLog`: what the page received. | | ||
| | `webInput.ts` | `webInputDriver(page)`: `wheel({x, y, dx, dy, ctrl})` and `pen(points)` as real engine input, for every-pointer proofs (`spec:AST-094` FR43); read back through the input log. | |
There was a problem hiding this comment.
Fixed in ad9e942: the README and webInput.ts cite spec:AST-100 FR4 now.
… into the FR43 harness helper
There was a problem hiding this comment.
Changes requested: wheel() now resolves once the page has the step and its deltas arrive exact, but in Chromium pen() sends its press with no page round trip, so a stroke that starts right after a navigation can press at 0.41× its coordinates (the page gets pointerdown at 41.02, 41.02 for a press at 100, 100: 402/980, the page scale of the about:blank page each test starts on in the phone project). The new pen self-test hits this intermittently on its first assertion (harness.interaction.browser.spec.ts:332), which would make the Chromium interaction lane flaky. Please have pen() wait for the page to present a frame before the press (two requestAnimationFrames through page.evaluate, as openStory does); with that wait in place every press landed where scripted. Optionally, note in the engine-limits table that WebKit's wheel step also moves the mouse (a pointermove) and presses Control as a key, which Chromium's step does not.
[Automated review]
| for (const {type, point} of events) { | ||
| const wait = startWall + point.t - Date.now(); | ||
| if (wait > 0) { | ||
| await sleep(wait); | ||
| } | ||
| await cdp.send('Input.dispatchMouseEvent', { |
There was a problem hiding this comment.
In Chromium the press goes out with no page round trip before it (at t = 0 there is no wait), so a stroke that starts right after a navigation can be placed with the previous document's page scale. With the driver created before goto and the stroke sent right after it, 28 of 30 strokes pressed at (41.02, 41.02) instead of (100, 100), which is 402/980, the scale of about:blank in the phone project; the later moves landed where scripted. The self-test's pen case failed 3 of 88 runs the same way on its first assertion (harness.interaction.browser.spec.ts:332). Could pen wait for the page to present a frame before the press, for example:
await page.evaluate(
() =>
new Promise<void>(resolve =>
requestAnimationFrame(() => requestAnimationFrame(() => resolve())),
),
);With one page round trip or two frames before the stroke, 30 of 30 presses landed at (100, 100).
There was a problem hiding this comment.
Fixed in b246458: pen now waits for the page to present two frames before the press (the harness's own frames, past a frozen timer gate). A new self-test creates the driver before the navigation and strokes right after it: on ad9e942 it pressed at (41.02, 41.02) 9 of 10 times, now 20/20 at (100, 100). The README's WebKit wheel row also says the step moves the mouse there (a pointermove) and holds Control as a key.
…o frames) into the FR43 harness helper
There was a problem hiding this comment.
Recommending approval: wheel() now resolves with exact CSS-px deltas once the page has the step, and pen() presses where it is scripted even right after a navigation, with self-tests that fail if either wait is removed. One optional follow-up: Chromium's pen goes through the mouse pipeline, so a stroke over a native scroller pans nothing, raises no pointercancel and ends in a click; a row in the README's engine-limits table would keep pen proofs from relying on stylus-like panning.
[Automated review]
| | Click synthesised from a touch | yes | only from `driver.tap` (WebKit's tap gesture), not from a raw touch sequence | | ||
| | Curve sampling, storyboards, input log | yes | yes | | ||
| | Wheel steps, exact CSS px deltas (`webInput.ts`) | yes | desktop context only (`isMobile: false`): skip with `WHEEL_SKIP`; the step also moves the mouse there (a `pointermove`) and holds Control as a key | | ||
| | Pen strokes, pressure, scripted timestamps | yes | no: skip with `PEN_SKIP` | |
There was a problem hiding this comment.
Optional: Chromium's pen goes through the mouse pipeline, so it differs from a stylus on a phone or tablet in ways this table could state. A stroke over a native overflow: auto scroller pans nothing (scrollTop stays 0, against 457 px for the same motion as a touch drag), raises no pointercancel, selects the text it crosses and ends in a click. For example:
| Pen panning, `touch-action`, `pointercancel` | no: mouse pipeline (a drag over text selects it and ends in a click) | no: skip with `PEN_SKIP` |
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewNo new or modified components detected. Bundle Size SummaryNo component packages changed. Accessibility AuditStatus: No accessibility violations detected. Visual Regression55 of 502 shot(s) changed. A change here is a question, not a failure: check whether the after is the
and 35 more. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
…eta#8666 08a2a28) into the web-input drivers helper The harness files are 8d35612's verified union, byte for byte, plus meta#8535's change to the backward self-test, which this line carries; the two helpers hold the same content apart from that change. This line's own harness additions leave it, as no open pull request builds on them: the earlier busy-window control and pooled proof (now meta#8906), openStory waiting for Storybook to finish a render, the touch driver's synthesizesClicks and SCRIPTED_CLICK_SKIP, and the wheel-at-DPR-3 self-test.
There was a problem hiding this comment.
Changes requested: the wheel and pen drivers deliver exact, awaited wheel steps in both engines and correctly placed, timed Chromium pen strokes, but wheel()'s 2 s delivery limit runs on the page's setTimeout (webInput.ts:120), which a frozen harness timer gate holds, so a step that misses the main document hangs until the test timeout instead of throwing its named error. Please take that timer from __harnessTimerGate?.timeout when a gate is installed, as busy.ts does, and add a self-test that a missed step under a frozen gate throws within the limit. Optionally, read __deltas without splice(0) in the new device-pixel-ratio case, so a wrong delta shows in the failure instead of [].
[Automated review]
| ([box, ms]) => | ||
| Promise.race([ | ||
| box.received.then(() => true), | ||
| new Promise<boolean>(resolve => setTimeout(() => resolve(false), ms)), |
There was a problem hiding this comment.
With a frozen timer gate (installTimerGate then freezeTimers), this setTimeout is the gate's, so its callback is held: a wheel step that never reaches the main document (a point outside the viewport or over an iframe) leaves wheel() pending until the test timeout instead of throwing here after 2 s. Over an iframe in Chromium it throws after about 2 s without the gate and is still pending after 6 s with it frozen. The harness's other waits take the real timer (busy.ts); with the same lookup here the frozen case throws after 2 s and the wheel and pen cases still pass:
([box, ms]) => {
const later =
(window as unknown as {__harnessTimerGate?: {timeout?: typeof setTimeout}})
.__harnessTimerGate?.timeout ?? setTimeout;
return Promise.race([
box.received.then(() => true),
new Promise<boolean>(resolve => later(() => resolve(false), ms)),
]);
},| expect(driver.wheels).toBe(true); | ||
| await driver.wheel({x: 200, y: 300, dy: 100}); | ||
| // Read once the page has the step, not at a time. | ||
| await expect.poll(seen).toEqual([100]); |
There was a problem hiding this comment.
Optional: seen splices __deltas, so expect.poll reads a wrong delta once, discards it, and the failure prints []. With a step sent without the scale compensation the page received [33.333…] and the report said Array []. A non-destructive read (slice(), then index 1 for Playwright's step) would show what arrived.
There was a problem hiding this comment.
Recommending approval: wheel()'s 2 s delivery limit now runs on the timer gate's real timer, so a wheel step that misses the main document throws its named error even while freezeTimers holds the page's timers, and the new self-test fails without that change in both Chromium and WebKit. The wheel and pen drivers still deliver exact, awaited wheel steps and correctly placed, timed Chromium pen strokes. One optional follow-up: add the Pen over a native scroller limit to the webInput.ts header, whose SYNC note asks it to match the README's table.
[Automated review]
…ter proofs
spec:AST-094 FR43 has gestures attach for every pointer, so each FR43 proof needs a pen and a wheel as real engine input, not only a finger. webInputDriver(page) adds wheel({x, y, dx, dy, ctrl}) and pen(points): in Chromium through CDP Input.dispatchMouseEvent, with a wheel step's deltas scaled back so the page receives exactly its CSS px (Chromium divides them by the device scale factor times the visual viewport's scale: a raw 120 px step reached a phone page as 40), and a pen stroke with pointerType pen, pressure, tilt and scripted timestamps; in WebKit through page.mouse.wheel in a desktop context. WebKit's phone emulation has no wheel and Playwright has no pen for WebKit: the driver's wheels and pens flags say so, with WHEEL_SKIP and PEN_SKIP. The input log also records wheel steps and pointer pressure.
Rebased onto main as one change. Folds in the follow-ups:
- test(interaction-harness): wheel() resolves once the page has the step
- test(interaction-harness): pen() presses once the page has presented two frames
- docs(interaction-harness): the engine-limits table aligns again, and names what a Chromium pen does over a native scroller (meta#8666)
- test(interaction-harness): a wheel step at a device-pixel ratio of 3 reaches the page as exactly its delta, in both engines
- test(interaction-harness): a wheel step that misses the main document throws within its limit while a frozen timer gate holds the page's timers
- feat(interaction-harness): wheelBurst sends a run of wheel steps on its schedule as one gesture, with a named pacing error and one re-take
- docs(interaction-harness): the webInput.ts header names the pen-over-a-native-scroller limit, as the README's table does
be3368c to
be484eb
Compare
There was a problem hiding this comment.
Requesting changes: the drivers deliver exact, awaited wheel steps, whole wheel bursts and timed Chromium pen strokes, but in WebKit wheelBurst reports a page's own main-thread stall as a runner stall and retakeOnWheelPacing re-takes it, so a proof over a page whose wheel listener blocks for 150 ms on its first gesture passes with only a wheel-retake annotation, and a stall on every gesture fails with an error that blames the runner. Please re-take only gaps the page did not cause (or at least stop attributing them to the runner and state WebKit's limit in the README), and cover it with a WebKit case over a stalling page. Please also add a negative-control burst case: with the maxGapMs check removed, the burst case still passes in both engines, while a burst paced at 180 ms with maxGapMs: 100 throws as intended. Optionally, the 100 ms gesture-end bound the burst docs cite names a recognizer that is not in the tree yet.
[Automated review]
| } | ||
| const receivedGapsMs = got.stamps.slice(1).map((t, i) => t - got.stamps[i]); | ||
| if (maxGapMs != null && receivedGapsMs.some(gap => gap > maxGapMs)) { | ||
| throw new WheelPacingError(maxGapMs, receivedGapsMs); |
There was a problem hiding this comment.
In WebKit this also fires when the page itself stalls: WebKit stamps a step when its web process dispatches it, so a page whose wheel listener blocks its main thread for 150 ms gets a 150 to 161 ms gap here and a WheelPacingError saying the runner stalled (3 of 3 runs). Inside retakeOnWheelPacing, a stall on the first gesture is then re-taken and the proof passes with only a wheel-retake annotation; in Chromium the same page's stamps do not widen. Could the burst re-take only gaps the page did not cause, for example by timing the page's own handling of each step beside this capture listener, and otherwise fail with an error that names the page's stall?
| x: 200, | ||
| y: 300, | ||
| steps: Array.from({length: 20}, () => ({dx: 40})), | ||
| maxGapMs: 100, |
There was a problem hiding this comment.
With the maxGapMs check in untilRunReceived removed, this case still passes in both engines, so nothing proves the pacing detection the re-take relies on. A negative control beside it would, and it throws in both engines today (with WheelPacingError imported from ./index):
await expect(
driver.wheelBurst({
x: 200,
y: 300,
steps: Array.from({length: 4}, () => ({dx: 10})),
everyMs: 180,
maxGapMs: 100,
}),
).rejects.toBeInstanceOf(WheelPacingError);…as the page's, never re-taken In WebKit a step is stamped when it is dispatched, and dispatched only once the page's main thread is free, so a page whose wheel listener blocks split a burst and wheelBurst blamed the runner: retakeOnWheelPacing re-took it, and a proof over a page that stalled on its first gesture passed. The burst now notes when the page has finished with each step, and a gap that the page's own handling of the step before explains throws WheelPageStallError, which is never re-taken; only the remaining gaps throw WheelPacingError. The README states WebKit's limit. New self-tests: a burst paced at 180 ms against a 100 ms bound throws WheelPacingError in both engines, and a page that blocks 150 ms per step is never re-taken (WheelPageStallError in WebKit). The 100 ms bound is described as the proof's, not a recognizer's.
There was a problem hiding this comment.
Requesting changes: the drivers deliver exact, awaited wheel steps, whole wheel bursts and timed Chromium pen strokes, and a WebKit page whose own wheel listener stalls now fails as WheelPageStallError with a negative control beside the burst case, but wheelGapCauses (webInput.ts:144-147) credits the earlier step's whole stamp-to-handled time against a gap even when the page was free before the next step arrived. In Chromium, whose stamps the page cannot move, a burst over a page whose listener does nothing throws WheelPageStallError ("the page split the run, not the runner") and skips its re-take whenever the dispatch spaces two steps 115 to 120 ms apart, and in WebKit the same happens over a 30 ms listener with steps 125 ms apart. Please give a gap to the page only when the next step reached it right after the page finished the step before, never in Chromium, and add a case where such a dispatch stall throws WheelPacingError. Optionally, align the WheelPacingError doc with the README's note that page work outside the wheel listeners still counts as the dispatch's stall.
[Automated review]
| const handled = handledAt[i - 1] - stamps[i - 1]; | ||
| const handlingMs = Number.isFinite(handled) && handled > 0 ? handled : 0; | ||
| if (gapMs - handlingMs <= maxGapMs) { | ||
| pageStalls.push({gapMs, handlingMs}); |
There was a problem hiding this comment.
This credits the earlier step's whole stamp-to-handled time against the gap even when the page was free before the next step arrived. In Chromium, whose stamps the page cannot move, a burst over a page whose wheel listener does nothing, with steps 115 or 120 ms apart, throws WheelPageStallError (handlingMs 21 to 29 ms, all delivery latency) and retakeOnWheelPacing does not re-take it (6 of 6); in WebKit a 30 ms listener with steps 125 ms apart throws it while the page sat idle 74 to 99 ms before each step. Could a gap go to the page only when the next step reached it right after the page finished the step before (for example stamps[i] - handledAt[i - 1] within one frame), and never in Chromium?
| * The page stamped a wheel burst's steps further apart than the proof's | ||
| * `maxGapMs` while its main thread was free between them: the dispatch | ||
| * stalled before the step reached the page. WebKit stamps a step when its web | ||
| * process dispatches it, so a loaded runner splits what the driver sent as one | ||
| * gesture, and the proof's outcome is the runner's, not the component's. |
There was a problem hiding this comment.
Optional: the README says page work outside the wheel listeners (timers, frames) still counts as the dispatch's stall, but this doc says the main thread was free and the outcome is the runner's. In WebKit, a page whose frame callback blocks 150 ms after its first wheel step got WheelPacingError, was re-taken and passed with only a wheel-retake annotation (2 of 2). Saying what the README says here, and in retakeOnWheelPacing's doc, would keep a re-taken run from reading as the runner's.
…teners held the step back, and never in Chromium wheelGapCauses credited the earlier step's whole stamp-to-handled time against a gap even when the page was free before the next step arrived, so a dispatch stall over a page whose listener did a little work was blamed on the page and not re-taken. A gap is now the page's only in WebKit, and only when the next step reached the page within a frame of the listeners finishing the one before; in Chromium, whose stamps the page cannot move, every gap is the dispatch's. New cases: a 30 ms listener with steps 125 ms apart throws WheelPacingError in both engines, and the unit cases cover both engines. WheelPacingError's and retakeOnWheelPacing's docs say that other main-thread work between steps also counts as the dispatch's, as the README does.
There was a problem hiding this comment.
Recommending approval: wheelGapCauses now gives a wheel gap to the page only in WebKit and only when the next step arrived within a frame of the listeners finishing the one before, so a dispatch stall over a page that spends 30 ms on each step throws WheelPacingError and is re-taken in both engines, while a WebKit page whose listener blocks 150 ms still fails as WheelPageStallError, and the new case fails in both engines under the previous rule. One optional follow-up: WheelPacingError's message still says only that the dispatch stalled, though its doc and the README count the page's own frame and timer work there (a WebKit page whose frame callback blocks 150 ms after its first step is re-taken under that message), so naming that cause in the message would point proof authors at the page as well as the runner.
[Automated review]
| readonly receivedGapsMs: readonly number[], | ||
| ) { | ||
| super( | ||
| `webInputDriver: the page received the wheel burst with a gap of ${Math.max(...receivedGapsMs).toFixed(0)} ms, over its ${maxGapMs} ms (gaps ${receivedGapsMs.map(g => g.toFixed(0)).join(', ')} ms): the dispatch stalled before the page, so the page saw more than one gesture`, |
There was a problem hiding this comment.
Optional: the class doc above (lines 85-87) and the README now count other work on the page's main thread between steps (timers, frame callbacks) as this error, but the message still says only that the dispatch stalled. In WebKit, a page whose frame callback blocks 150 ms after the first step of the first attempt gets this text, is re-taken and passes (5 of 5). Naming that cause here too, for example the dispatch, or other work on the page's main thread between steps, stalled the run, so the page saw more than one gesture, would point a proof author at the page as well as the runner.
Why
Gesture proofs could drive only a finger. The every-pointer requirement (
spec:AST-100FR4: the same motion from touch, pen and mouse) and #8646, which has gestures attach for every pointer, need a pen stroke and a wheel step as real browser input. A hand-made wheel step was also wrong on phones: Chromium divides it by the device-pixel ratio, so a 120 px step reached a phone page as 40.What led to it
We are bringing Astryx's touch components to the behavior of native controls, and their proofs drive real browser input through the interaction harness. With gestures now accepting every pointer, the harness had to drive a pen and a wheel the same way it drives a finger.
Options and tradeoffs
new WheelEvent,new PointerEvent): untrusted, and a component can tell (isTrusted), so they prove nothing about real input.Why this choice
It is the only option whose events are trusted and go through the browser's own input pipeline, and it states each engine's limits instead of hiding them.
A run of wheel steps, as a trackpad page swipe sends, goes through
wheelBurst({x, y, steps, everyMs, maxGapMs}): it sends every step on its schedule without waiting for the one before, resolves once the page has the run's whole delta, and, pastmaxGapMs, throwsWheelPageStallErrorwhen the page's own wheel listeners held the main thread on the step before (WebKit dispatches the next step only once the page is free), andWheelPacingError, naming the gaps, when the dispatch stalled before the page.retakeOnWheelPacingre-takes a proof once, only onWheelPacingError, and lists the re-take; a stall the page caused fails the proof.How to check it
pnpm exec playwright test --config playwright.interaction.config.ts internal/interaction-harness/src/harness.interaction.browser.spec.ts --project chromium -g "wheel and pen input".wheel()resolves, as exactly its deltas, trusted, with Control when held, whether the page's wheel listeners are passive or not; a pen stroke arrives as trusted pen pointer events with its pressure and its scripted 100 ms between the first and last points.--project webkit, the desktop wheel cases pass and the phone wheel and pen cases skip, each naming why.pnpm exec vitest run internal/interaction-harness/src/webInput.pacing.test.ts: the re-take stays red when both attempts are off pace, never re-takes a product failure, and passes and is listed when an off-pace attempt is followed by a good one.-g "burst": a burst paced at 180 ms against a 100 ms bound throwsWheelPacingErrorin both engines, as does one paced at 125 ms over a page whose listener takes 30 ms (the page is free before each step arrives); a page that blocks 150 ms on each step is never re-taken (WheelPageStallErrorin WebKit, where a gap is the page's only when the step reached it within a frame of its listeners finishing the one before; in Chromium every gap is the dispatch's).wheel()missed the step when every listener was passive, and a stroke sent right after a navigation pressed at 41 px for 100.Evidence