Skip to content

Conversation

@nikneym
Copy link
Contributor

@nikneym nikneym commented Dec 5, 2025

This is the bare minimum required for a specific website, I'll add support for other variants once needed.

@nikneym nikneym requested a review from karlseguin December 5, 2025 13:09
@nikneym nikneym added the zigdom label Dec 5, 2025
}

// TODO: Compare `localName` and prefix.
switch (self._type) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could push this logic into each type, e.g.

if (self.as(Element).isEqualNode(other.as(Element)) == false) {
  return false;
}

const e1 = self.as(Element);
const e2 = other.as(Element);

const e1_tag = e1.getTagNameSpec(&page.buf);
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 you can use getTagNameDump() instead, which doesn't require a buffer

return false;
}

if (e1._namespace != e2._namespace or !std.mem.eql(u8, e1_tag, e2_tag) or iter1_count != iter2_count) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Both more efficient and easier to read if you group these a little better. Check the tags names and namespaces where you get them, before iterating through all the attributes and comparing htem.


return std.mem.eql(u8, c1.getData(), c2.getData());
},
else => @panic("NIY"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

log.warn(.browser, "not implemented", .{
    .type = self._type,
    .feature = "Node.isEqualNode",
});
return false;

else => @panic("NIY"),
}

// `Node.childNodes` allocate memory, this seems like a better option.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// `Node.childNodes` allocate memory, this seems like a better option.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, childrenIterator is meant for internal iteration within the Zig code, specifically to avoid the overhead of childNodes necessary to fulfill the webapi of that type.

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