Load about blank doc to set window #630
Conversation
| @@ -125,6 +125,10 @@ fn createTarget(cmd: anytype) !void { | |||
| bc.target_id = target_id; | |||
|
|
|||
| var page = try bc.session.createPage(); | |||
| // Navigate to about:blank such that the window / state.document are created and set | |||
| // Playwright uses these before navigating to a real URL to detect when they are removed. | |||
| const url = try @import("../../url.zig").URL.parse("about:blank", ""); | |||
There was a problem hiding this comment.
You could maybe create a const URL.blank like the existing URL.empty (in url.zig)
| @@ -335,6 +335,8 @@ pub const Page = struct { | |||
|
|
|||
| // if the url is about:blank, nothing to do. | |||
| if (std.mem.eql(u8, "about:blank", request_url.raw)) { | |||
| var about_blank = AboutBlank{}; | |||
| try self.loadHTMLDoc(&about_blank, "utf-8"); | |||
There was a problem hiding this comment.
loadHTMLDoc does a lot of stuff you probably don't need (like walking through the the DOM, finding script tabs, setting up click events, ...), but then it probably does stuff you need, let setting the window's html_doc.
I wonder if the behavior can be split into two functions, loadHTMLDoc and processHTMLDoc. You'd only call loadHTMLDoc, and the navigate flow would call loadHTMLDoc followed by processHTMLDoc...might require reorganizing things a little. I'm just thining about the few microseconds that'll be spent on this - not sure it's worth it.
| @@ -335,6 +335,8 @@ pub const Page = struct { | |||
|
|
|||
| // if the url is about:blank, nothing to do. | |||
| if (std.mem.eql(u8, "about:blank", request_url.raw)) { | |||
| var about_blank = AboutBlank{}; | |||
There was a problem hiding this comment.
AboutBlank is pretty clever, but this would work too I think:
var fbs = std.io.fixedBufferStream("");
try self.loadHTMLDoc(fbs.reader(), "utf-8");|
|
||
| // Even about:blank should set the window and document | ||
| const page = ctx.cdp().browser_context.?.session.page.?; | ||
| try std.testing.expect(page.state.document != null); |
There was a problem hiding this comment.
There's a const testing = @import("testing.zig"); in this file.
It's src/cdp/testing.zig which includes src/testing.zig, which extends std.testing. There's no expect there, but you can add it if you want.I tend to dislike it because it gives less useful errors, but I agree here it's the only real option.
In testing.zig
pub const expect = std.testing.expect;in cdp/testing.zig
pub const expect = base.expect;Or not, I think this code here is fine, just wanted to FYI about the existing testing files.
Playwright uses the window.document to detect when it is removed from the DOM tree.
Before this change we did not load any DOM tree so these were null pointers.
This makes sure that whenever we navigate to about:blank that a default DOM tree is still loaded and the window / document are set.
Note this behavior is in alignment with the spec: https://html.spec.whatwg.org/#read-html