Skip to content

Conversation

@krichprollsch
Copy link
Member

@krichprollsch krichprollsch commented Dec 30, 2025

Backport of #1244 for zigdom

fixes #1220
relates with #1217

@krichprollsch krichprollsch self-assigned this Dec 30, 2025
@krichprollsch krichprollsch marked this pull request as ready for review January 5, 2026 11:28
@krichprollsch krichprollsch mentioned this pull request Jan 5, 2026
if (value.value) |v| {
try w.objectField("value");
switch (v) {
.uint => try w.write(v.uint),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would work:

switch (v) {
  inline else => |v| v try w.write(v),
}

fn toJSON(self: *const Writer, node: *const Node, w: anytype) !void {
try w.beginArray();
const root = try AXNode.fromNode(node.dom);
if (try self.writeNode(node.id, root, w)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably reverse this, so that writeNode returns true if the children should be written, so that you can just do:

if (try self.writeNode(node.id, root, w)) {
    try self.writeNodeChildren(root, w);
}
return w.endArray();

dom: *DOMNode,
role_attr: ?[]const u8,

pub fn fromNode(dom: *DOMNode) !AXNode {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can't return an error. If you remove the error union here, then you can remove it from isIgnore

switch (dom_node._type) {
.document => |document| {
const uri = document.getURL(page);
try self.writeAXProperty(.{ .name = .url, .value = .{ .type = .string, .value = .{ .string = uri } } }, w);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the type field and union a redundant? Could you make it work with:

.{ .name = .url, .value = .{.string = uri } }

And use the std.meta.activeTag(value) in writeAXValue ? (renaming the uint tag to integer)

}

fn isHidden(elt: *DOMNode.Element) bool {
if (elt.getAttributeSafe("aria-hidden")) |value| {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Outside this PR, we could potentially optimize checking/fetching multiple attributes. (I think there's a number of small optimizations we might be able to do for attributes, and this multi-attribute check use-case is worth thinking about)

Base automatically changed from zigdom to main January 8, 2026 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants