-
Notifications
You must be signed in to change notification settings - Fork 286
Add support for CDP's DOM.requestChildNodes #888
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| const params = (try cmd.params(struct { | ||
| nodeId: Node.Id, | ||
| depth: i32 = 1, | ||
| pierce: bool = false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May want to warn or so when pierce is set to true that this is not implemented yet
| registry: *Registry, | ||
|
|
||
| pub const Opts = struct {}; | ||
| pub const Opts = struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DOM.getDocument I believe uses the undocumented value 3 by default. So may be good to update the other users in this pr as well, and connect the parameters.
For DOM.describeNode the default is 1, however it would be good to wire it in.
performSearch and querySelector most likely use the default, but I did not check to confirm
| log.err(.cdp, "node writeChildren", .{ .err = err }); | ||
| return error.OutOfMemory; | ||
| }; | ||
| } else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assume that the meaning of the depth value 1, 2, 3 has been confirmed for both cases
| var registry = self.registry; | ||
| const child_nodes = try parser.nodeGetChildNodes(node._node); | ||
| const child_count = try parser.nodeListLength(child_nodes); | ||
| const full_child = self.depth < 0 or self.depth < depth; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const full_child = self.depth < 0 or self.depth < depth; | |
| const full_child = self.depth < 0 or depth < self.depth; |
My understanding is that if self.depth is 1 we only get one layer of nodes.
So in the exclude_root == true case that would be a single list of nodes without any of their children populated.
So I would expect this function to be called with the parameter depth: usize set to 1 (now called with 0).
In the exclude_root == false I would expect writeChildren not to be called at all as just the root node would be written. So we'd need this if statement also in the toJSON function.
I would not be surpised if I'm off by one tho.
#866