Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions src/runtime/webview/ChromeBackend.h
Original file line number Diff line number Diff line change
Expand Up @@ -277,27 +277,37 @@
// passed as JSON-escaped arguments at the end — avoids Chrome's
// callFunctionOn dance for one string + one number.
//
// The predicate: attached + has size + in viewport + stable for 2 frames
// + elementFromPoint at center returns the element (not obscured). Returns
// the center coords as [cx, cy]; throws on timeout.
// The predicate: attached + has size + in viewport + stable across two
// distinct animation frames + elementFromPoint at center returns the element
// (not obscured). Returns the center coords as [cx, cy]; throws on timeout.
//
// Every sample is taken after an `await rAF`, and a match only counts when
// the rAF timestamp advanced, so a just-started animation cannot look stable
// at its from-keyframe (Playwright's _checkElementIsStable does the same).
// The rAF wait races a setTimeout bound to the deadline so the timeout
// contract holds when the renderer is not producing frames.
constexpr ASCIILiteral kActionabilityIIFE = R"js((async (sel, timeout) => {
const deadline = performance.now() + timeout;
let last;
let last, lastT;
for (;;) {
const t = await new Promise(f => {
const id = setTimeout(f, Math.max(0, deadline - performance.now()));
requestAnimationFrame(t => { clearTimeout(id); f(t); });

Check warning on line 295 in src/runtime/webview/ChromeBackend.h

View check run for this annotation

Claude / Claude Code Review

setTimeout deadline race applied to actionability but not sibling scrollTo scripts

The new setTimeout-vs-rAF race ("the timeout contract holds when the renderer is not producing frames") is added to `kActionabilityIIFE`/`kActionabilityJS` but not to the adjacent `kScrollToIIFE` (ChromeBackend.h:316-324) and `kScrollToJS` (WebViewHost.cpp:396-404), which retain the bare `await new Promise(f => requestAnimationFrame(f))` — so `scrollTo(selector, {timeout})` still hangs past its deadline under the exact condition the new comment documents. This is pre-existing (the old actionabil
Comment thread
robobun marked this conversation as resolved.
Resolved
});
if (performance.now() > deadline) throw "timeout waiting for '" + sel + "' to be actionable";
const el = document.querySelector(sel);
if (el) {
const r = el.getBoundingClientRect();
const cx = r.left + r.width / 2, cy = r.top + r.height / 2;
if (r.width > 0 && r.height > 0 && cx >= 0 && cy >= 0 && cx < innerWidth && cy < innerHeight) {
if (last && last.l === r.left && last.t === r.top && last.w === r.width && last.h === r.height) {
if (last && t !== lastT && last.l === r.left && last.t === r.top && last.w === r.width && last.h === r.height) {
const hit = document.elementFromPoint(cx, cy);
if (hit === el || el.contains(hit)) return [cx, cy];
}
last = { l: r.left, t: r.top, w: r.width, h: r.height };
} else last = undefined;
} else last = undefined;
if (performance.now() > deadline) throw "timeout waiting for '" + sel + "' to be actionable";
await new Promise(f => requestAnimationFrame(f));
lastT = t;
}
}))js"_s;

Expand Down
26 changes: 19 additions & 7 deletions src/runtime/webview/WebViewHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -351,30 +351,42 @@ void WebViewHost::doNativeClick(float x, float y, uint8_t button, uint8_t modifi
// page-side via callAsyncJavaScript: — WebKit awaits the returned Promise.
// One IPC roundtrip regardless of how many frames the poll takes.
//
// The predicate: attached + has size + in viewport + stable for 2 frames +
// elementFromPoint at center returns the element or a descendant (not
// obscured). Returns "cx,cy" on success; throws on timeout.
// The predicate: attached + has size + in viewport + stable across two
// distinct animation frames + elementFromPoint at center returns the element
// or a descendant (not obscured). Returns "cx,cy" on success; throws on
// timeout.
//
// Every sample is taken after an `await rAF`, and a match only counts when
// the rAF timestamp advanced, so a just-started animation cannot look stable
// at its from-keyframe (Playwright's _checkElementIsStable does the same).
// The rAF wait races a setTimeout bound to the deadline so the timeout
// contract holds when the renderer is not producing frames (headless WK
// without a display driver).
//
// Arguments `sel` and `timeout` are passed via the arguments: NSDictionary,
// not string-interpolated — the selector can contain any characters.
static constexpr const char* kActionabilityJS = R"js(
const deadline = performance.now() + timeout;
let last;
let last, lastT;
for (;;) {
const t = await new Promise(f => {
const id = setTimeout(f, Math.max(0, deadline - performance.now()));
requestAnimationFrame(t => { clearTimeout(id); f(t); });
});
if (performance.now() > deadline) throw "timeout waiting for '" + sel + "' to be actionable";
const el = document.querySelector(sel);
if (el) {
const r = el.getBoundingClientRect();
const cx = r.left + r.width / 2, cy = r.top + r.height / 2;
if (r.width > 0 && r.height > 0 && cx >= 0 && cy >= 0 && cx < innerWidth && cy < innerHeight) {
if (last && last.l === r.left && last.t === r.top && last.w === r.width && last.h === r.height) {
if (last && t !== lastT && last.l === r.left && last.t === r.top && last.w === r.width && last.h === r.height) {
const hit = document.elementFromPoint(cx, cy);
if (hit === el || el.contains(hit)) return cx + "," + cy;
}
last = { l: r.left, t: r.top, w: r.width, h: r.height };
} else last = undefined;
} else last = undefined;
if (performance.now() > deadline) throw "timeout waiting for '" + sel + "' to be actionable";
await new Promise(f => requestAnimationFrame(f));
lastT = t;
}
Comment thread
robobun marked this conversation as resolved.
Resolved
)js";

Expand Down
24 changes: 17 additions & 7 deletions test/js/bun/webview/webview-chrome.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -725,16 +725,26 @@ it("chrome: click(selector) waits for animation to stop", async () => {
html(`
<style>
@keyframes slide { from { left: 0; } to { left: 100px; } }
#mover { position: fixed; top: 50px; width: 60px; height: 60px;
animation: slide 80ms linear forwards; }
#mover { position: fixed; top: 50px; left: 0; width: 60px; height: 60px; }
</style>
<button id=mover onclick="window.__hit=this.getBoundingClientRect().left">mv</button>
<body style="margin:0" onclick="(window.__x ||= []).push(event.clientX)">
<div id=mover></div>
</body>
`),
);
// Stable-for-2-frames check — the click lands after the animation stops.
await view.click("#mover");
const left = Number(await view.evaluate("String(__hit)"));
expect(left).toBe(100);
// Restart the animation immediately before each click(): the element reads
// left=0 both before the next render (no effect yet) and at t=0 of the
// animation, so a sync-then-rAF stability pair would match and click at
// the from-position. The loop covers frame-scheduling variance.
for (let i = 0; i < 5; i++) {
await view.evaluate(
`(m => { m.style.animation = 'none'; void m.offsetHeight; m.style.animation = 'slide 100ms linear forwards'; })(document.getElementById('mover'))`,
);
await view.click("#mover");
}
// The animation ends at left=100; the element is 60px wide, so every click
// lands at clientX=130 once the slide has settled.
expect(await view.evaluate("__x")).toEqual(Array(5).fill(130));
});

// --- scrollTo variants -----------------------------------------------------
Expand Down
25 changes: 17 additions & 8 deletions test/js/bun/webview/webview.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -557,17 +557,26 @@ itRendering("click(selector) waits for element to stop animating", async () => {
encodeURIComponent(`
<style>
@keyframes slide { from { left: 0px; } to { left: 100px; } }
#mover { position: fixed; top: 50px; width: 60px; height: 60px;
animation: slide 100ms linear forwards; }
#mover { position: fixed; top: 50px; left: 0; width: 60px; height: 60px; }
</style>
<button id=mover onclick="window.__hit=this.getBoundingClientRect().left">mv</button>
<body style="margin:0" onclick="(window.__x ||= []).push(event.clientX)">
<div id=mover></div>
</body>
`),
);
// The stable-for-2-consecutive-frames check means we don't click until
// the animation stops. If we clicked mid-slide, __hit would be < 100.
await view.click("#mover");
const left = await view.evaluate("String(__hit)");
expect(Number(left)).toBe(100);
// Restart the animation immediately before each click(): the element reads
// left=0 both before the next render (no effect yet) and at t=0 of the
// animation, so a sync-then-rAF stability pair would match and click at
// the from-position. The loop covers frame-scheduling variance.
for (let i = 0; i < 5; i++) {
await view.evaluate(
`(m => { m.style.animation = 'none'; void m.offsetHeight; m.style.animation = 'slide 100ms linear forwards'; })(document.getElementById('mover'))`,
);
await view.click("#mover");
}
// The animation ends at left=100; the element is 60px wide, so every click
// lands at clientX=130 once the slide has settled.
expect(await view.evaluate("__x")).toEqual(Array(5).fill(130));
});

itRendering("click(selector) rejects on timeout when obscured", async () => {
Expand Down
Loading