From 1d806475c46a66c5a2f303e074012ee3ce8fbadc Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Mon, 27 Apr 2026 07:08:01 +0200 Subject: [PATCH 1/5] page: make handleJavaScriptDialog drive confirm/prompt return values Page.handleJavaScriptDialog previously responded -32000 "No dialog is showing" regardless of whether a dialog was open, leaving CDP clients no way to influence the JS-side return value of confirm() / prompt(). PR #2085 wired up the Page.javascriptDialogOpening event but explicitly deferred the return-value override since true Chrome semantics require suspending V8 mid-execution. Add a pre-arm model that fits the auto-dismiss architecture without runtime suspension: handleJavaScriptDialog stashes {accept, promptText} on the BrowserContext; when the next JS dialog dispatches the javascript_dialog_opening notification, the listener pops the stash and fills it into the dispatch's response output param so Window.confirm / prompt return the CDP client's choice. Without a pre-arm, headless auto-dismiss values from PR #2085 are preserved (confirm->false, prompt->null, alert->void). Closes #2260 --- src/Notification.zig | 14 +++ src/browser/tests/cdp/dialog.html | 1 + src/browser/webapi/Window.zig | 16 ++- src/cdp/CDP.zig | 7 ++ src/cdp/domains/page.zig | 163 ++++++++++++++++++++++++++++-- 5 files changed, 193 insertions(+), 8 deletions(-) create mode 100644 src/browser/tests/cdp/dialog.html diff --git a/src/Notification.zig b/src/Notification.zig index d01d29ce..caa8c20d 100644 --- a/src/Notification.zig +++ b/src/Notification.zig @@ -197,6 +197,20 @@ pub const JavascriptDialogOpening = struct { url: [:0]const u8, message: []const u8, dialog_type: []const u8, + // Output param. The CDP listener may set this from a pre-armed response + // queued by Page.handleJavaScriptDialog. The dispatcher (alert/confirm/ + // prompt in Window.zig) reads it back to decide what to return to JS. + // Headless mode auto-dismisses if no listener fills it in: confirm→false, + // prompt→null, alert→void (default-zero DialogResponse). + response: *DialogResponse, +}; + +pub const DialogResponse = struct { + accept: bool = false, + // Set when the CDP client sent a `promptText` with `accept: true`. Memory + // is owned by whoever filled in the response (typically the BrowserContext + // arena) and must outlive a single dispatch call. + prompt_text: ?[]const u8 = null, }; pub fn init(allocator: Allocator) !*Notification { diff --git a/src/browser/tests/cdp/dialog.html b/src/browser/tests/cdp/dialog.html new file mode 100644 index 00000000..ac172584 --- /dev/null +++ b/src/browser/tests/cdp/dialog.html @@ -0,0 +1 @@ +

dialog test page

diff --git a/src/browser/webapi/Window.zig b/src/browser/webapi/Window.zig index 31efb8ff..38fc37cc 100644 --- a/src/browser/webapi/Window.zig +++ b/src/browser/webapi/Window.zig @@ -44,6 +44,7 @@ const Element = @import("Element.zig"); const CSSStyleProperties = @import("css/CSSStyleProperties.zig"); const CustomElementRegistry = @import("CustomElementRegistry.zig"); const Selection = @import("Selection.zig"); +const Notification = @import("../../Notification.zig"); const log = lp.log; const IS_DEBUG = builtin.mode == .Debug; @@ -918,31 +919,42 @@ pub const JsApi = struct { pub const alert = bridge.function(struct { fn alert(_: *const Window, message: ?[]const u8, frame: *Frame) void { + var response: Notification.DialogResponse = .{}; frame._session.notification.dispatch(.javascript_dialog_opening, &.{ .url = frame.url, .message = message orelse "", .dialog_type = "alert", + .response = &response, }); + // Return value is void; we still pop a pre-armed response so the + // CDP client's pre-arm doesn't leak across to the next dialog. } }.alert, .{}); pub const confirm = bridge.function(struct { fn confirm(_: *const Window, message: ?[]const u8, frame: *Frame) bool { + var response: Notification.DialogResponse = .{}; frame._session.notification.dispatch(.javascript_dialog_opening, &.{ .url = frame.url, .message = message orelse "", .dialog_type = "confirm", + .response = &response, }); - return false; + return response.accept; } }.confirm, .{}); pub const prompt = bridge.function(struct { fn prompt(_: *const Window, message: ?[]const u8, _: ?[]const u8, frame: *Frame) ?[]const u8 { + var response: Notification.DialogResponse = .{}; frame._session.notification.dispatch(.javascript_dialog_opening, &.{ .url = frame.url, .message = message orelse "", .dialog_type = "prompt", + .response = &response, }); - return null; + if (!response.accept) return null; + // promptText omitted with accept=true is "" per CDP spec + // (https://chromedevtools.github.io/devtools-protocol/tot/Page/#method-handleJavaScriptDialog). + return response.prompt_text orelse ""; } }.prompt, .{}); diff --git a/src/cdp/CDP.zig b/src/cdp/CDP.zig index 16a08cc9..6bfff798 100644 --- a/src/cdp/CDP.zig +++ b/src/cdp/CDP.zig @@ -386,6 +386,13 @@ pub const BrowserContext = struct { notification: *Notification, + // Pre-armed response for the next JS dialog (alert/confirm/prompt). + // Set by Page.handleJavaScriptDialog; consumed (and cleared) when the + // next javascript_dialog_opening notification is dispatched. Strings + // are duplicated into self.arena so they outlive the CDP command's + // own message arena. + pending_dialog_response: ?Notification.DialogResponse = null, + fn init(self: *BrowserContext, id: []const u8, cdp: *CDP) !void { const allocator = cdp.allocator; diff --git a/src/cdp/domains/page.zig b/src/cdp/domains/page.zig index b0aae79e..ff589d32 100644 --- a/src/cdp/domains/page.zig +++ b/src/cdp/domains/page.zig @@ -675,19 +675,56 @@ fn sendPageLifecycle(bc: *CDP.BrowserContext, name: []const u8, timestamp: u64, } // https://chromedevtools.github.io/devtools-protocol/tot/Page/#method-handleJavaScriptDialog +// +// Lightpanda's dialogs auto-dismiss in headless mode (window.alert/confirm/ +// prompt return immediately rather than blocking the JS runtime). This means +// the standard CDP flow — open dialog, wait for client, resume JS with the +// client's choice — would require suspending V8 mid-execution, which the +// engine isn't structured for today (#2082 / PR #2085 deferred this work). +// +// To still let CDP clients influence the return value we use a pre-arm +// model: the client sends Page.handleJavaScriptDialog *before* triggering +// the JS that opens the dialog. We stash {accept, promptText} on the +// BrowserContext; when the next dialog dispatches the +// `javascript_dialog_opening` notification, the listener pops the stash and +// fills it into the dispatch's response output param. Window.alert/confirm/ +// prompt then return that value. +// +// Without a pre-armed response, behavior is unchanged from PR #2085: +// confirm→false, prompt→null, alert→void. fn handleJavaScriptDialog(cmd: *CDP.Command) !void { - // Dialogs auto-dismiss in headless mode. By the time the CDP client - // sends this command, the dialog has already returned and there is - // no pending dialog to accept or dismiss. - _ = try cmd.params(struct { + const params = (try cmd.params(struct { accept: bool, promptText: ?[]const u8 = null, - }); - return cmd.sendError(-32000, "No dialog is showing", .{}); + })) orelse return error.InvalidParams; + + const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; + + // Duplicate promptText into the BrowserContext arena so it outlives the + // CDP command's own message arena (the dialog may fire on a later command). + const prompt_text: ?[]const u8 = if (params.promptText) |t| + try bc.arena.dupe(u8, t) + else + null; + + bc.pending_dialog_response = .{ + .accept = params.accept, + .prompt_text = prompt_text, + }; + + return cmd.sendResult(null, .{}); } // https://chromedevtools.github.io/devtools-protocol/tot/Page/#event-javascriptDialogOpening pub fn javascriptDialogOpening(bc: anytype, event: *const Notification.JavascriptDialogOpening) !void { + // Pop any pre-armed response onto the dispatch's output param so the + // calling alert/confirm/prompt returns the CDP client's choice. Cleared + // unconditionally — a stash applies to exactly one dialog. + if (bc.pending_dialog_response) |pending| { + event.response.* = pending; + bc.pending_dialog_response = null; + } + const session_id = bc.session_id orelse return; var cdp = bc.cdp; @@ -1054,3 +1091,117 @@ test "cdp.frame: addScriptToEvaluateOnNewDocument" { try testing.expectEqual(2, try test_val.toI32()); } } + +test "cdp.page: handleJavaScriptDialog accepts/dismisses without an open dialog" { + var ctx = try testing.context(); + defer ctx.deinit(); + + { + // Without a BrowserContext: error (matches other Page handlers' shape). + try ctx.processMessage(.{ .id = 1, .method = "Page.handleJavaScriptDialog", .params = .{ .accept = true } }); + try ctx.expectSentError(-31998, "BrowserContextNotLoaded", .{ .id = 1 }); + } + + _ = try ctx.loadBrowserContext(.{ .id = "BID-D1", .url = "cdp/dialog.html", .target_id = "FID-000000000X".* }); + + { + // Pre-arming with accept=true succeeds (was -32000 "No dialog is showing" + // before this fix). Headless browsers auto-dismiss, so the CDP client + // sends Page.handleJavaScriptDialog *before* the JS that opens the + // dialog — handler stashes the response on the BrowserContext. + try ctx.processMessage(.{ .id = 2, .method = "Page.handleJavaScriptDialog", .params = .{ .accept = true } }); + try ctx.expectSentResult(null, .{ .id = 2 }); + } + + { + // Pre-arming with accept=false also succeeds. + try ctx.processMessage(.{ .id = 3, .method = "Page.handleJavaScriptDialog", .params = .{ .accept = false } }); + try ctx.expectSentResult(null, .{ .id = 3 }); + } + + { + // Pre-arming with a promptText also succeeds. The string is dup'd into + // the BrowserContext arena so it survives until the dialog dispatches. + try ctx.processMessage(.{ .id = 4, .method = "Page.handleJavaScriptDialog", .params = .{ .accept = true, .promptText = "hello" } }); + try ctx.expectSentResult(null, .{ .id = 4 }); + } +} + +test "cdp.page: handleJavaScriptDialog controls confirm/prompt/alert return values" { + var ctx = try testing.context(); + defer ctx.deinit(); + + var bc = try ctx.loadBrowserContext(.{ .id = "BID-D2", .url = "cdp/dialog.html", .target_id = "FID-000000000X".* }); + + const frame = bc.session.currentFrame() orelse unreachable; + var ls: js.Local.Scope = undefined; + frame.js.localScope(&ls); + defer ls.deinit(); + + // ---- confirm: accept=true makes confirm() return true ---- + try ctx.processMessage(.{ .id = 1, .method = "Page.handleJavaScriptDialog", .params = .{ .accept = true } }); + try ctx.expectSentResult(null, .{ .id = 1 }); + + const c_accept = try ls.local.exec("confirm('proceed?')", null); + try testing.expectEqual(true, c_accept.toBool()); + try ctx.expectSentEvent("Page.javascriptDialogOpening", .{ + .message = "proceed?", + .type = "confirm", + .hasBrowserHandler = false, + .defaultPrompt = "", + }, .{ .session_id = "SID-X" }); + + // ---- confirm: accept=false makes confirm() return false ---- + try ctx.processMessage(.{ .id = 2, .method = "Page.handleJavaScriptDialog", .params = .{ .accept = false } }); + try ctx.expectSentResult(null, .{ .id = 2 }); + + const c_dismiss = try ls.local.exec("confirm('again?')", null); + try testing.expectEqual(false, c_dismiss.toBool()); + + // ---- confirm: no pre-arm preserves PR #2085 default (false) ---- + const c_default = try ls.local.exec("confirm('default?')", null); + try testing.expectEqual(false, c_default.toBool()); + + // ---- prompt: accept=true with promptText returns the text ---- + try ctx.processMessage(.{ .id = 3, .method = "Page.handleJavaScriptDialog", .params = .{ .accept = true, .promptText = "hello" } }); + try ctx.expectSentResult(null, .{ .id = 3 }); + + const p_text = try ls.local.exec("prompt('name?')", null); + const p_text_str = try p_text.toStringSlice(); + try testing.expectEqualSlices(u8, "hello", p_text_str); + + // ---- prompt: accept=true without promptText returns "" per CDP spec ---- + try ctx.processMessage(.{ .id = 4, .method = "Page.handleJavaScriptDialog", .params = .{ .accept = true } }); + try ctx.expectSentResult(null, .{ .id = 4 }); + + const p_empty = try ls.local.exec("prompt('name?')", null); + const p_empty_str = try p_empty.toStringSlice(); + try testing.expectEqualSlices(u8, "", p_empty_str); + + // ---- prompt: accept=false makes prompt() return null ---- + try ctx.processMessage(.{ .id = 5, .method = "Page.handleJavaScriptDialog", .params = .{ .accept = false } }); + try ctx.expectSentResult(null, .{ .id = 5 }); + + const p_dismiss = try ls.local.exec("prompt('cancel?')", null); + try testing.expect(p_dismiss.isNull()); + + // ---- prompt: no pre-arm preserves PR #2085 default (null) ---- + const p_default = try ls.local.exec("prompt('default?')", null); + try testing.expect(p_default.isNull()); + + // ---- alert: dispatches the event but has no return value to override ---- + try ctx.processMessage(.{ .id = 6, .method = "Page.handleJavaScriptDialog", .params = .{ .accept = true } }); + try ctx.expectSentResult(null, .{ .id = 6 }); + _ = try ls.local.exec("alert('important')", null); + try ctx.expectSentEvent("Page.javascriptDialogOpening", .{ + .message = "important", + .type = "alert", + }, .{ .session_id = "SID-X" }); + + // ---- pending response is consumed by exactly one dialog ---- + // After the alert above pops the pre-arm, the next confirm sees no pending + // and falls back to the default (false) — the alert MUST clear pending so + // the response doesn't leak across dialogs. + const c_after_alert = try ls.local.exec("confirm('leak?')", null); + try testing.expectEqual(false, c_after_alert.toBool()); +} From d4479c0383830b0366b90cccf00cde56a6436efc Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Mon, 27 Apr 2026 23:08:56 +0200 Subject: [PATCH 2/5] ci: retrigger after pre-existing demo-scripts flake (lightpanda-io/demo#167) From 8cc82d1d6470cf056e34fa43b325fbc59a93954c Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Wed, 29 Apr 2026 02:04:20 +0200 Subject: [PATCH 3/5] lp: move handleJavaScriptDialog pre-arm from Page to LP namespace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per upstream review (#2261): Page.handleJavaScriptDialog is a standard CDP method that reactive Chrome-style drivers send in response to a Page.javascriptDialogOpening event. Repurposing it for the proactive pre-arm flow risks surprising those drivers — the next dialog they trigger would be blindly accepted. Move the pre-arm handler to LP.handleJavaScriptDialog so only Lightpanda-aware clients opt in. Page.handleJavaScriptDialog reverts to the upstream stub returning -32000 "No dialog is showing" so the reactive surface is unchanged. The Page.javascriptDialogOpening event listener still pops the BrowserContext stash and fills the response output param — only the setter moved. Tests rename cdp.page: → cdp.lp: and target LP.handleJavaScriptDialog; behavior coverage is identical. --- src/cdp/domains/lp.zig | 163 ++++++++++++++++++++++++++++++++++++++ src/cdp/domains/page.zig | 165 ++++----------------------------------- 2 files changed, 177 insertions(+), 151 deletions(-) diff --git a/src/cdp/domains/lp.zig b/src/cdp/domains/lp.zig index 7f5defbd..95a04133 100644 --- a/src/cdp/domains/lp.zig +++ b/src/cdp/domains/lp.zig @@ -41,6 +41,7 @@ pub fn processMessage(cmd: *CDP.Command) !void { fillNode, scrollNode, waitForSelector, + handleJavaScriptDialog, }, cmd.input.action) orelse return error.UnknownMethod; switch (action) { @@ -54,6 +55,7 @@ pub fn processMessage(cmd: *CDP.Command) !void { .fillNode => return fillNode(cmd), .scrollNode => return scrollNode(cmd), .waitForSelector => return waitForSelector(cmd), + .handleJavaScriptDialog => return handleJavaScriptDialog(cmd), } } @@ -293,6 +295,53 @@ fn waitForSelector(cmd: anytype) !void { }, .{}); } +// Lightpanda-namespaced pre-arm for window.alert/confirm/prompt return values. +// +// Standard CDP drivers send Page.handleJavaScriptDialog *reactively* in +// response to a Page.javascriptDialogOpening event — the dialog suspends +// JS, the client picks accept/dismiss, the runtime resumes. Lightpanda's +// dialogs auto-dismiss in headless mode (window.alert/confirm/prompt +// return immediately rather than blocking V8), so by the time the event +// reaches the client, JS has already returned. A reactive call has +// nothing left to influence — full Chrome-faithful behavior would +// require V8 suspension, which #2082 / PR #2085 deferred. +// +// LP.handleJavaScriptDialog gives Lightpanda-aware clients a *proactive* +// opt-in: the client sets {accept, promptText} *before* triggering the JS +// that opens the dialog. The handler stashes the response on the +// BrowserContext; when the next dialog dispatches the +// `javascript_dialog_opening` notification, the listener in page.zig +// pops the stash and fills it into the dispatch's response output param. +// window.alert/confirm/prompt then return that value. +// +// Page.handleJavaScriptDialog continues to return -32000 "No dialog is +// showing" so reactive Chrome-style drivers see no semantic change. +// +// Without a pre-armed response, behavior is unchanged from PR #2085: +// confirm→false, prompt→null, alert→void. +fn handleJavaScriptDialog(cmd: anytype) !void { + const params = (try cmd.params(struct { + accept: bool, + promptText: ?[]const u8 = null, + })) orelse return error.InvalidParam; + + const bc = cmd.browser_context orelse return error.NoBrowserContext; + + // Duplicate promptText into the BrowserContext arena so it outlives the + // CDP command's own message arena (the dialog may fire on a later command). + const prompt_text: ?[]const u8 = if (params.promptText) |t| + try bc.arena.dupe(u8, t) + else + null; + + bc.pending_dialog_response = .{ + .accept = params.accept, + .prompt_text = prompt_text, + }; + + return cmd.sendResult(null, .{}); +} + const testing = @import("../testing.zig"); test "cdp.lp: getMarkdown" { var ctx = try testing.context(); @@ -441,3 +490,117 @@ test "cdp.lp: waitForSelector" { const err_obj = (try ctx.getSentMessage(2)).?.object.get("error").?.object; try testing.expect(err_obj.get("code") != null); } + +test "cdp.lp: handleJavaScriptDialog accepts/dismisses without an open dialog" { + var ctx = try testing.context(); + defer ctx.deinit(); + + { + // Without a BrowserContext: error (matches other LP handlers' shape). + try ctx.processMessage(.{ .id = 1, .method = "LP.handleJavaScriptDialog", .params = .{ .accept = true } }); + try ctx.expectSentError(-31998, "NoBrowserContext", .{ .id = 1 }); + } + + _ = try ctx.loadBrowserContext(.{ .id = "BID-D1", .url = "cdp/dialog.html", .target_id = "FID-000000000X".* }); + + { + // Pre-arming with accept=true succeeds. Headless browsers auto-dismiss, + // so the CDP client sends LP.handleJavaScriptDialog *before* the JS + // that opens the dialog — handler stashes the response on the + // BrowserContext. + try ctx.processMessage(.{ .id = 2, .method = "LP.handleJavaScriptDialog", .params = .{ .accept = true } }); + try ctx.expectSentResult(null, .{ .id = 2 }); + } + + { + // Pre-arming with accept=false also succeeds. + try ctx.processMessage(.{ .id = 3, .method = "LP.handleJavaScriptDialog", .params = .{ .accept = false } }); + try ctx.expectSentResult(null, .{ .id = 3 }); + } + + { + // Pre-arming with a promptText also succeeds. The string is dup'd into + // the BrowserContext arena so it survives until the dialog dispatches. + try ctx.processMessage(.{ .id = 4, .method = "LP.handleJavaScriptDialog", .params = .{ .accept = true, .promptText = "hello" } }); + try ctx.expectSentResult(null, .{ .id = 4 }); + } +} + +test "cdp.lp: handleJavaScriptDialog controls confirm/prompt/alert return values" { + var ctx = try testing.context(); + defer ctx.deinit(); + + var bc = try ctx.loadBrowserContext(.{ .id = "BID-D2", .url = "cdp/dialog.html", .target_id = "FID-000000000X".* }); + + const frame = bc.session.currentFrame() orelse unreachable; + var ls: lp.js.Local.Scope = undefined; + frame.js.localScope(&ls); + defer ls.deinit(); + + // ---- confirm: accept=true makes confirm() return true ---- + try ctx.processMessage(.{ .id = 1, .method = "LP.handleJavaScriptDialog", .params = .{ .accept = true } }); + try ctx.expectSentResult(null, .{ .id = 1 }); + + const c_accept = try ls.local.exec("confirm('proceed?')", null); + try testing.expectEqual(true, c_accept.toBool()); + try ctx.expectSentEvent("Page.javascriptDialogOpening", .{ + .message = "proceed?", + .type = "confirm", + .hasBrowserHandler = false, + .defaultPrompt = "", + }, .{ .session_id = "SID-X" }); + + // ---- confirm: accept=false makes confirm() return false ---- + try ctx.processMessage(.{ .id = 2, .method = "LP.handleJavaScriptDialog", .params = .{ .accept = false } }); + try ctx.expectSentResult(null, .{ .id = 2 }); + + const c_dismiss = try ls.local.exec("confirm('again?')", null); + try testing.expectEqual(false, c_dismiss.toBool()); + + // ---- confirm: no pre-arm preserves PR #2085 default (false) ---- + const c_default = try ls.local.exec("confirm('default?')", null); + try testing.expectEqual(false, c_default.toBool()); + + // ---- prompt: accept=true with promptText returns the text ---- + try ctx.processMessage(.{ .id = 3, .method = "LP.handleJavaScriptDialog", .params = .{ .accept = true, .promptText = "hello" } }); + try ctx.expectSentResult(null, .{ .id = 3 }); + + const p_text = try ls.local.exec("prompt('name?')", null); + const p_text_str = try p_text.toStringSlice(); + try testing.expectEqualSlices(u8, "hello", p_text_str); + + // ---- prompt: accept=true without promptText returns "" per CDP spec ---- + try ctx.processMessage(.{ .id = 4, .method = "LP.handleJavaScriptDialog", .params = .{ .accept = true } }); + try ctx.expectSentResult(null, .{ .id = 4 }); + + const p_empty = try ls.local.exec("prompt('name?')", null); + const p_empty_str = try p_empty.toStringSlice(); + try testing.expectEqualSlices(u8, "", p_empty_str); + + // ---- prompt: accept=false makes prompt() return null ---- + try ctx.processMessage(.{ .id = 5, .method = "LP.handleJavaScriptDialog", .params = .{ .accept = false } }); + try ctx.expectSentResult(null, .{ .id = 5 }); + + const p_dismiss = try ls.local.exec("prompt('cancel?')", null); + try testing.expect(p_dismiss.isNull()); + + // ---- prompt: no pre-arm preserves PR #2085 default (null) ---- + const p_default = try ls.local.exec("prompt('default?')", null); + try testing.expect(p_default.isNull()); + + // ---- alert: dispatches the event but has no return value to override ---- + try ctx.processMessage(.{ .id = 6, .method = "LP.handleJavaScriptDialog", .params = .{ .accept = true } }); + try ctx.expectSentResult(null, .{ .id = 6 }); + _ = try ls.local.exec("alert('important')", null); + try ctx.expectSentEvent("Page.javascriptDialogOpening", .{ + .message = "important", + .type = "alert", + }, .{ .session_id = "SID-X" }); + + // ---- pending response is consumed by exactly one dialog ---- + // After the alert above pops the pre-arm, the next confirm sees no pending + // and falls back to the default (false) — the alert MUST clear pending so + // the response doesn't leak across dialogs. + const c_after_alert = try ls.local.exec("confirm('leak?')", null); + try testing.expectEqual(false, c_after_alert.toBool()); +} diff --git a/src/cdp/domains/page.zig b/src/cdp/domains/page.zig index 8281484b..654eb92b 100644 --- a/src/cdp/domains/page.zig +++ b/src/cdp/domains/page.zig @@ -689,51 +689,27 @@ fn sendPageLifecycle(bc: *CDP.BrowserContext, name: []const u8, timestamp: u64, } // https://chromedevtools.github.io/devtools-protocol/tot/Page/#method-handleJavaScriptDialog -// -// Lightpanda's dialogs auto-dismiss in headless mode (window.alert/confirm/ -// prompt return immediately rather than blocking the JS runtime). This means -// the standard CDP flow — open dialog, wait for client, resume JS with the -// client's choice — would require suspending V8 mid-execution, which the -// engine isn't structured for today (#2082 / PR #2085 deferred this work). -// -// To still let CDP clients influence the return value we use a pre-arm -// model: the client sends Page.handleJavaScriptDialog *before* triggering -// the JS that opens the dialog. We stash {accept, promptText} on the -// BrowserContext; when the next dialog dispatches the -// `javascript_dialog_opening` notification, the listener pops the stash and -// fills it into the dispatch's response output param. Window.alert/confirm/ -// prompt then return that value. -// -// Without a pre-armed response, behavior is unchanged from PR #2085: -// confirm→false, prompt→null, alert→void. fn handleJavaScriptDialog(cmd: *CDP.Command) !void { - const params = (try cmd.params(struct { + // Dialogs auto-dismiss in headless mode. By the time the CDP client + // sends this command, the dialog has already returned and there is + // no pending dialog to accept or dismiss. + // + // Lightpanda-aware clients that want to control confirm/prompt return + // values can pre-arm a response via LP.handleJavaScriptDialog instead + // (see src/cdp/domains/lp.zig). + _ = try cmd.params(struct { accept: bool, promptText: ?[]const u8 = null, - })) orelse return error.InvalidParams; - - const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; - - // Duplicate promptText into the BrowserContext arena so it outlives the - // CDP command's own message arena (the dialog may fire on a later command). - const prompt_text: ?[]const u8 = if (params.promptText) |t| - try bc.arena.dupe(u8, t) - else - null; - - bc.pending_dialog_response = .{ - .accept = params.accept, - .prompt_text = prompt_text, - }; - - return cmd.sendResult(null, .{}); + }); + return cmd.sendError(-32000, "No dialog is showing", .{}); } // https://chromedevtools.github.io/devtools-protocol/tot/Page/#event-javascriptDialogOpening pub fn javascriptDialogOpening(bc: anytype, event: *const Notification.JavascriptDialogOpening) !void { - // Pop any pre-armed response onto the dispatch's output param so the - // calling alert/confirm/prompt returns the CDP client's choice. Cleared - // unconditionally — a stash applies to exactly one dialog. + // Pop any response pre-armed via LP.handleJavaScriptDialog onto the + // dispatch's output param so the calling alert/confirm/prompt returns + // the CDP client's choice. Cleared unconditionally — a stash applies + // to exactly one dialog. if (bc.pending_dialog_response) |pending| { event.response.* = pending; bc.pending_dialog_response = null; @@ -1354,116 +1330,3 @@ test "cdp.frame: addScriptToEvaluateOnNewDocument" { } } -test "cdp.page: handleJavaScriptDialog accepts/dismisses without an open dialog" { - var ctx = try testing.context(); - defer ctx.deinit(); - - { - // Without a BrowserContext: error (matches other Page handlers' shape). - try ctx.processMessage(.{ .id = 1, .method = "Page.handleJavaScriptDialog", .params = .{ .accept = true } }); - try ctx.expectSentError(-31998, "BrowserContextNotLoaded", .{ .id = 1 }); - } - - _ = try ctx.loadBrowserContext(.{ .id = "BID-D1", .url = "cdp/dialog.html", .target_id = "FID-000000000X".* }); - - { - // Pre-arming with accept=true succeeds (was -32000 "No dialog is showing" - // before this fix). Headless browsers auto-dismiss, so the CDP client - // sends Page.handleJavaScriptDialog *before* the JS that opens the - // dialog — handler stashes the response on the BrowserContext. - try ctx.processMessage(.{ .id = 2, .method = "Page.handleJavaScriptDialog", .params = .{ .accept = true } }); - try ctx.expectSentResult(null, .{ .id = 2 }); - } - - { - // Pre-arming with accept=false also succeeds. - try ctx.processMessage(.{ .id = 3, .method = "Page.handleJavaScriptDialog", .params = .{ .accept = false } }); - try ctx.expectSentResult(null, .{ .id = 3 }); - } - - { - // Pre-arming with a promptText also succeeds. The string is dup'd into - // the BrowserContext arena so it survives until the dialog dispatches. - try ctx.processMessage(.{ .id = 4, .method = "Page.handleJavaScriptDialog", .params = .{ .accept = true, .promptText = "hello" } }); - try ctx.expectSentResult(null, .{ .id = 4 }); - } -} - -test "cdp.page: handleJavaScriptDialog controls confirm/prompt/alert return values" { - var ctx = try testing.context(); - defer ctx.deinit(); - - var bc = try ctx.loadBrowserContext(.{ .id = "BID-D2", .url = "cdp/dialog.html", .target_id = "FID-000000000X".* }); - - const frame = bc.session.currentFrame() orelse unreachable; - var ls: js.Local.Scope = undefined; - frame.js.localScope(&ls); - defer ls.deinit(); - - // ---- confirm: accept=true makes confirm() return true ---- - try ctx.processMessage(.{ .id = 1, .method = "Page.handleJavaScriptDialog", .params = .{ .accept = true } }); - try ctx.expectSentResult(null, .{ .id = 1 }); - - const c_accept = try ls.local.exec("confirm('proceed?')", null); - try testing.expectEqual(true, c_accept.toBool()); - try ctx.expectSentEvent("Page.javascriptDialogOpening", .{ - .message = "proceed?", - .type = "confirm", - .hasBrowserHandler = false, - .defaultPrompt = "", - }, .{ .session_id = "SID-X" }); - - // ---- confirm: accept=false makes confirm() return false ---- - try ctx.processMessage(.{ .id = 2, .method = "Page.handleJavaScriptDialog", .params = .{ .accept = false } }); - try ctx.expectSentResult(null, .{ .id = 2 }); - - const c_dismiss = try ls.local.exec("confirm('again?')", null); - try testing.expectEqual(false, c_dismiss.toBool()); - - // ---- confirm: no pre-arm preserves PR #2085 default (false) ---- - const c_default = try ls.local.exec("confirm('default?')", null); - try testing.expectEqual(false, c_default.toBool()); - - // ---- prompt: accept=true with promptText returns the text ---- - try ctx.processMessage(.{ .id = 3, .method = "Page.handleJavaScriptDialog", .params = .{ .accept = true, .promptText = "hello" } }); - try ctx.expectSentResult(null, .{ .id = 3 }); - - const p_text = try ls.local.exec("prompt('name?')", null); - const p_text_str = try p_text.toStringSlice(); - try testing.expectEqualSlices(u8, "hello", p_text_str); - - // ---- prompt: accept=true without promptText returns "" per CDP spec ---- - try ctx.processMessage(.{ .id = 4, .method = "Page.handleJavaScriptDialog", .params = .{ .accept = true } }); - try ctx.expectSentResult(null, .{ .id = 4 }); - - const p_empty = try ls.local.exec("prompt('name?')", null); - const p_empty_str = try p_empty.toStringSlice(); - try testing.expectEqualSlices(u8, "", p_empty_str); - - // ---- prompt: accept=false makes prompt() return null ---- - try ctx.processMessage(.{ .id = 5, .method = "Page.handleJavaScriptDialog", .params = .{ .accept = false } }); - try ctx.expectSentResult(null, .{ .id = 5 }); - - const p_dismiss = try ls.local.exec("prompt('cancel?')", null); - try testing.expect(p_dismiss.isNull()); - - // ---- prompt: no pre-arm preserves PR #2085 default (null) ---- - const p_default = try ls.local.exec("prompt('default?')", null); - try testing.expect(p_default.isNull()); - - // ---- alert: dispatches the event but has no return value to override ---- - try ctx.processMessage(.{ .id = 6, .method = "Page.handleJavaScriptDialog", .params = .{ .accept = true } }); - try ctx.expectSentResult(null, .{ .id = 6 }); - _ = try ls.local.exec("alert('important')", null); - try ctx.expectSentEvent("Page.javascriptDialogOpening", .{ - .message = "important", - .type = "alert", - }, .{ .session_id = "SID-X" }); - - // ---- pending response is consumed by exactly one dialog ---- - // After the alert above pops the pre-arm, the next confirm sees no pending - // and falls back to the default (false) — the alert MUST clear pending so - // the response doesn't leak across dialogs. - const c_after_alert = try ls.local.exec("confirm('leak?')", null); - try testing.expectEqual(false, c_after_alert.toBool()); -} From 6211b218130039e67f6956094b73e91ebea71823 Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Wed, 29 Apr 2026 02:52:50 +0200 Subject: [PATCH 4/5] dom: route click into form submission MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Frame.handleClick's .input arm matched only _input_type == .submit and fell through for .image, so clicking an image button fired the click event but never scheduled a navigation or dispatched the form's submit event. Per HTML §4.10.18.6.4 image buttons are submit buttons and must submit their owning form. The submitForm path already passes the input element as the submitter, and FormData.collectForm already handles image submitters by appending `name.x` and `name.y` (or bare `x`/`y` for unnamed inputs) coordinate entries to the form-data set. Only the routing in handleClick was missing. Coordinates are 0 for programmatic .click() per the spec. Closes #2311 --- src/browser/Frame.zig | 6 +- .../element/html/input_image_submit.html | 109 ++++++++++++++++++ src/browser/webapi/element/html/Input.zig | 1 + 3 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 src/browser/tests/element/html/input_image_submit.html diff --git a/src/browser/Frame.zig b/src/browser/Frame.zig index 12bb8c38..18341512 100644 --- a/src/browser/Frame.zig +++ b/src/browser/Frame.zig @@ -3648,7 +3648,11 @@ pub fn handleClick(self: *Frame, target: *Node) !void { }, .input => |input| { try element.focus(self); - if (input._input_type == .submit) { + // Per HTML §4.10.18.6.4 "Image Button state (type=image)", clicking an + // image button submits its form. The form-data set already gets the + // submitter's coordinate fields appended via FormData.collectForm + // (see src/browser/webapi/net/FormData.zig). + if (input._input_type == .submit or input._input_type == .image) { return self.submitForm(element, input.getForm(self), .{}); } }, diff --git a/src/browser/tests/element/html/input_image_submit.html b/src/browser/tests/element/html/input_image_submit.html new file mode 100644 index 00000000..f4909949 --- /dev/null +++ b/src/browser/tests/element/html/input_image_submit.html @@ -0,0 +1,109 @@ + + + +
+ + +
+ +
+ + +
+ +
+ +
+ +
+ +
+ +
+ +
+ + + + + + + + + + + + + diff --git a/src/browser/webapi/element/html/Input.zig b/src/browser/webapi/element/html/Input.zig index f5fe00d5..d38710f2 100644 --- a/src/browser/webapi/element/html/Input.zig +++ b/src/browser/webapi/element/html/Input.zig @@ -1114,6 +1114,7 @@ const testing = @import("../../../../testing.zig"); test "WebApi: HTML.Input" { try testing.htmlRunner("element/html/input.html", .{}); try testing.htmlRunner("element/html/input_click.html", .{}); + try testing.htmlRunner("element/html/input_image_submit.html", .{}); try testing.htmlRunner("element/html/input_radio.html", .{}); try testing.htmlRunner("element/html/input-attrs.html", .{}); } From 7dccf59ec285430c4b2dd428e3cea690c1c9f4bb Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Wed, 29 Apr 2026 03:45:09 +0200 Subject: [PATCH 5/5] zig fmt linting --- src/cdp/domains/page.zig | 1 - 1 file changed, 1 deletion(-) diff --git a/src/cdp/domains/page.zig b/src/cdp/domains/page.zig index 654eb92b..28a2fc8f 100644 --- a/src/cdp/domains/page.zig +++ b/src/cdp/domains/page.zig @@ -1329,4 +1329,3 @@ test "cdp.frame: addScriptToEvaluateOnNewDocument" { try testing.expectEqual(2, try test_val.toI32()); } } -