mirror of
https://github.com/lightpanda-io/browser.git
synced 2026-06-11 09:35:59 -04:00
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
This commit is contained in:
@@ -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 {
|
||||
|
||||
1
src/browser/tests/cdp/dialog.html
Normal file
1
src/browser/tests/cdp/dialog.html
Normal file
@@ -0,0 +1 @@
|
||||
<p>dialog test page</p>
|
||||
@@ -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, .{});
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
|
||||
@@ -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());
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user