From 13638ff50b71176e1baf75687b14ef17a8091317 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 17 Sep 2026 12:13:53 +0800 Subject: [PATCH] internal: Always correct SemanticTree context Applies the frame-ownership pass to SemanticTree, copying what we did for StyleManager (1). SemanticTree doesn't visit iframes, so the frame of the root is the frame/frame._style_manager we need to target for all visited nodes. Like #3536, it's up to the callers to (a) get the correct frame and (b) decide what to do on a frameless-node. (1) https://github.com/lightpanda-io/browser/pull/3536 --- src/SemanticTree.zig | 51 +++++-------------- .../tests/cdp/semantic_tree_frame_a.html | 2 - .../tests/cdp/semantic_tree_frame_b.html | 1 - .../tests/cdp/semantic_tree_iframe.html | 6 +++ .../tests/cdp/semantic_tree_iframe_child.html | 4 ++ src/browser/tools.zig | 6 ++- src/server/cdp/domains/lp.zig | 45 ++++++++++++++-- 7 files changed, 68 insertions(+), 47 deletions(-) delete mode 100644 src/browser/tests/cdp/semantic_tree_frame_a.html delete mode 100644 src/browser/tests/cdp/semantic_tree_frame_b.html create mode 100644 src/browser/tests/cdp/semantic_tree_iframe.html create mode 100644 src/browser/tests/cdp/semantic_tree_iframe_child.html diff --git a/src/SemanticTree.zig b/src/SemanticTree.zig index d98bf7d8e..ad1e92581 100644 --- a/src/SemanticTree.zig +++ b/src/SemanticTree.zig @@ -38,13 +38,14 @@ const Self = @This(); dom_node: *Node, registry: *NodeRegistry, -frame: *Frame, +frame: *Frame, // we never visit iframes, every node we visit is in the same frame as dom_node arena: std.mem.Allocator, prune: bool = true, interactive_only: bool = false, max_depth: u32 = std.math.maxInt(u32) - 1, pub fn jsonStringify(self: @This(), jw: *std.json.Stringify) error{WriteFailed}!void { + assertOwns(self.frame, self.dom_node); var visitor = JsonVisitor{ .jw = jw, .tree = self }; var xpath_buffer: std.ArrayList(u8) = .empty; const listener_targets = interactive.buildListenerTargetMap(self.frame, self.arena) catch |err| { @@ -56,7 +57,6 @@ pub fn jsonStringify(self: @This(), jw: *std.json.Stringify) error{WriteFailed}! .xpath_buffer = &xpath_buffer, .listener_targets = listener_targets, .label_index = &label_index, - .owner_frame = self.dom_node.ownerFrame(self.frame) orelse self.frame, }; self.walk(&ctx, self.dom_node, null, &visitor, 1, 0) catch |err| { log.err(.app, "semantic tree json dump failed", .{ .err = err }); @@ -65,6 +65,7 @@ pub fn jsonStringify(self: @This(), jw: *std.json.Stringify) error{WriteFailed}! } pub fn textStringify(self: @This(), writer: *std.Io.Writer) error{WriteFailed}!void { + assertOwns(self.frame, self.dom_node); var visitor = TextVisitor{ .writer = writer, .tree = self, .depth = 0 }; var xpath_buffer: std.ArrayList(u8) = .empty; const listener_targets = interactive.buildListenerTargetMap(self.frame, self.arena) catch |err| { @@ -76,7 +77,6 @@ pub fn textStringify(self: @This(), writer: *std.Io.Writer) error{WriteFailed}!v .xpath_buffer = &xpath_buffer, .listener_targets = listener_targets, .label_index = &label_index, - .owner_frame = self.dom_node.ownerFrame(self.frame) orelse self.frame, }; self.walk(&ctx, self.dom_node, null, &visitor, 1, 0) catch |err| { log.err(.app, "semantic tree text dump failed", .{ .err = err }); @@ -108,7 +108,6 @@ const WalkContext = struct { xpath_buffer: *std.ArrayList(u8), listener_targets: interactive.ListenerTargetMap, label_index: *Label.LabelByForIndex, - owner_frame: *Frame, // node's own frame, not the caller's (the dumped frame when the node's document has none) }; fn walk( @@ -132,7 +131,7 @@ fn walk( // Hidden subtrees are never entered, so below the root only the // element's own display matters. - const style_manager = &ctx.owner_frame._style_manager; + const style_manager = &self.frame._style_manager; const hidden = if (current_depth == 0) style_manager.isHidden(el, .{}) else @@ -658,6 +657,7 @@ pub fn getNodeDetails( registry: *NodeRegistry, frame: *Frame, ) !NodeDetails { + assertOwns(frame, node); const cdp_node = try registry.register(node); const axn = AXNode.fromNode(node); const role = try axn.getRole(); @@ -733,6 +733,13 @@ pub fn getNodeDetails( }; } +fn assertOwns(frame: *const Frame, node: *const Node) void { + if (comptime lp.IS_DEBUG == false) { + return; + } + std.debug.assert(node.ownerFrame(frame) == frame); +} + const testing = @import("testing.zig"); test "SemanticTree backendDOMNodeId" { @@ -759,40 +766,6 @@ test "SemanticTree backendDOMNodeId" { try testing.expect(std.mem.indexOf(u8, json_str, "\"backendDOMNodeId\":") != null); } -test "SemanticTree: styles come from the node's own frame" { - var registry: NodeRegistry = .init(testing.allocator); - defer registry.deinit(); - - // The caller's frame hides #inner; the frame that actually owns the walked - // subtree does not. A backendNodeId lookup can hand us a node from another - // frame, so the walk must not use the caller's stylesheets. - var page_a = try testing.pageTest("cdp/semantic_tree_frame_a.html", .{}); - defer page_a.close(); - var page_b = try testing.pageTest("cdp/semantic_tree_frame_b.html", .{}); - defer page_b.close(); - - const frame_a = page_a.frame().?; - const frame_b = page_b.frame().?; - - const target = (try frame_b.window._document.querySelector(.wrap("#target"), frame_b)).?.asNode(); - - const st: Self = .{ - .dom_node = target, - .registry = ®istry, - .frame = frame_a, - .arena = testing.arena_allocator, - .prune = false, - .interactive_only = false, - .max_depth = std.math.maxInt(u32) - 1, - }; - - var aw: std.Io.Writer.Allocating = .init(testing.allocator); - defer aw.deinit(); - - try st.textStringify(&aw.writer); - try testing.expect(std.mem.indexOf(u8, aw.written(), "inner-b") != null); -} - test "SemanticTree max_depth" { var registry: NodeRegistry = .init(testing.allocator); defer registry.deinit(); diff --git a/src/browser/tests/cdp/semantic_tree_frame_a.html b/src/browser/tests/cdp/semantic_tree_frame_a.html deleted file mode 100644 index 5753081f1..000000000 --- a/src/browser/tests/cdp/semantic_tree_frame_a.html +++ /dev/null @@ -1,2 +0,0 @@ - -

page-a

diff --git a/src/browser/tests/cdp/semantic_tree_frame_b.html b/src/browser/tests/cdp/semantic_tree_frame_b.html deleted file mode 100644 index 967adff52..000000000 --- a/src/browser/tests/cdp/semantic_tree_frame_b.html +++ /dev/null @@ -1 +0,0 @@ -

inner-b

diff --git a/src/browser/tests/cdp/semantic_tree_iframe.html b/src/browser/tests/cdp/semantic_tree_iframe.html new file mode 100644 index 000000000..d4da16a32 --- /dev/null +++ b/src/browser/tests/cdp/semantic_tree_iframe.html @@ -0,0 +1,6 @@ + + + + +

parent-probe

+ diff --git a/src/browser/tests/cdp/semantic_tree_iframe_child.html b/src/browser/tests/cdp/semantic_tree_iframe_child.html new file mode 100644 index 000000000..59f1e8264 --- /dev/null +++ b/src/browser/tests/cdp/semantic_tree_iframe_child.html @@ -0,0 +1,4 @@ + + + +

child-probe

diff --git a/src/browser/tools.zig b/src/browser/tools.zig index a1e90042a..5f90c164a 100644 --- a/src/browser/tools.zig +++ b/src/browser/tools.zig @@ -1430,11 +1430,12 @@ fn execTree(arena: std.mem.Allocator, session: *lp.Session, registry: *NodeRegis const page = try ensurePage(session, registry, args.url, args.timeout); const root_node = (try resolveOptionalNode(registry, args.backendNodeId)) orelse page.document.asNode(); + const frame = root_node.ownerFrame(page) orelse return ToolError.NodeNotFound; const st = lp.SemanticTree{ .dom_node = root_node, .registry = registry, - .frame = page, + .frame = frame, .arena = arena, .prune = true, .max_depth = args.maxDepth orelse std.math.maxInt(u32) - 1, @@ -1453,7 +1454,8 @@ fn execNodeDetails(arena: std.mem.Allocator, session: *lp.Session, registry: *No const node = registry.lookup_by_id.get(args.backendNodeId) orelse return ToolError.NodeNotFound; - const details = lp.SemanticTree.getNodeDetails(arena, node.dom, registry, page) catch + const frame = node.dom.ownerFrame(page) orelse return ToolError.NodeNotFound; + const details = lp.SemanticTree.getNodeDetails(arena, node.dom, registry, frame) catch return ToolError.InternalError; return renderJson(arena, &details); } diff --git a/src/server/cdp/domains/lp.zig b/src/server/cdp/domains/lp.zig index 716eb5a46..bbcf957a5 100644 --- a/src/server/cdp/domains/lp.zig +++ b/src/server/cdp/domains/lp.zig @@ -127,12 +127,13 @@ fn getSemanticTree(cmd: anytype) !void { const params = (try cmd.params(Params)) orelse Params{}; const bc = cmd.browser_context orelse return error.NoBrowserContext; - const frame = bc.mainFrame() orelse return error.FrameNotLoaded; + const root = bc.mainFrame() orelse return error.FrameNotLoaded; const dom_node = if (params.backendNodeId) |nodeId| (bc.node_registry.lookup_by_id.get(nodeId) orelse return error.InvalidNodeId).dom else - frame.document.asNode(); + root.document.asNode(); + const frame = dom_node.ownerFrame(root) orelse return error.InvalidNodeId; var st = SemanticTree{ .dom_node = dom_node, @@ -269,9 +270,10 @@ fn getNodeDetails(cmd: anytype) !void { const params = (try cmd.params(Params)) orelse return error.InvalidParam; const bc = cmd.browser_context orelse return error.NoBrowserContext; - const frame = bc.mainFrame() orelse return error.FrameNotLoaded; + const root = bc.mainFrame() orelse return error.FrameNotLoaded; const node = (bc.node_registry.lookup_by_id.get(params.backendNodeId) orelse return error.InvalidNodeId).dom; + const frame = node.ownerFrame(root) orelse return error.InvalidNodeId; const details = SemanticTree.getNodeDetails(cmd.arena, node, &bc.node_registry, frame) catch return error.InternalError; @@ -616,6 +618,43 @@ test "cdp.lp: dump formats, strip and scoping" { try testing.expect((try dumpReply(&ctx, 9)).get("error") != null); } +// A backendNodeId can name a node in a child frame while the handler only +// knows the root. Labels, datalists and styles must come from the node's own +// document: the parent reuses every id and hides `.probe`. +test "cdp.lp: semantic tree and node details read the node's own frame" { + var ctx = try testing.context(); + defer ctx.deinit(); + + const bc = try ctx.loadBrowserContext(.{ .id = "BID-T", .url = "cdp/semantic_tree_iframe.html", .target_id = "FID-000000000T".* }); + const root = bc.mainFrame() orelse unreachable; + const child = root.child_frames.items[0]; + + const html = (child.document.getDocumentElement() orelse unreachable).asNode(); + const input = (try child.document.querySelector(.wrap("input"), child)).?.asNode(); + const html_id = (try bc.node_registry.register(html)).id; + const input_id = (try bc.node_registry.register(input)).id; + + try ctx.processMessage(.{ .id = 1, .method = "LP.getSemanticTree", .params = .{ .backendNodeId = html_id, .format = "text", .prune = false } }); + const tree = (try dumpReply(&ctx, 1)).get("result").?.object.get("semanticTree").?.string; + try testing.expect(std.mem.indexOf(u8, tree, "child-label") != null); + try testing.expect(std.mem.indexOf(u8, tree, "child-option") != null); + try testing.expect(std.mem.indexOf(u8, tree, "child-probe") != null); + try testing.expect(std.mem.indexOf(u8, tree, "parent-") == null); + + try ctx.processMessage(.{ .id = 2, .method = "LP.getNodeDetails", .params = .{ .backendNodeId = input_id } }); + const details = (try dumpReply(&ctx, 2)).get("result").?.object.get("nodeDetails").?.object; + try testing.expectEqual("child-label", details.get("name").?.string); + try testing.expectEqual("child-option", details.get("options").?.array.items[0].object.get("value").?.string); + + // A document with no frame has no styles or layout to describe. + const frameless = try root._factory.genericDocument(.{}); + const frameless_id = (try bc.node_registry.register(frameless.asNode())).id; + try ctx.processMessage(.{ .id = 3, .method = "LP.getSemanticTree", .params = .{ .backendNodeId = frameless_id } }); + try ctx.expectSentError(-31998, "InvalidNodeId", .{ .id = 3 }); + try ctx.processMessage(.{ .id = 4, .method = "LP.getNodeDetails", .params = .{ .backendNodeId = frameless_id } }); + try ctx.expectSentError(-31998, "InvalidNodeId", .{ .id = 4 }); +} + test "cdp.lp: getInteractiveElements" { var ctx = try testing.context(); defer ctx.deinit();