Skip to content

Implement DOMNode::isEqualNode() #11690

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

Closed
wants to merge 1 commit into from
Closed

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Jul 12, 2023

Since we still support obsoleted nodes in our implementation, this uses the old spec to match the old nodes; and this uses the new spec for nodes still defined in the living spec.

References:
https://dom.spec.whatwg.org/#dom-node-isequalnode (for everything still in the living spec) https://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/DOM3-Core.html#core-Node3-isEqualNode (for old nodes removed from the living spec)

Marking as a draft because I need to review it again myself first, but I want to let CI run already. Ready now

@nielsdos nielsdos marked this pull request as ready for review July 12, 2023 17:36
Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

The implementation looks sensible but didn't really look at the test output too closely so would like @theseer to have another look at it.

Also wondering if it makes sense to send a patch to libxml2 to implement this directly so that at one point we can drop this convoluted code.

Since we still support obsoleted nodes in our implementation, this uses
the old spec to match the old nodes; and this uses the new spec for
nodes still defined in the living spec.
When unclear, the behaviour was cross-verified with Firefox.

References:
https://dom.spec.whatwg.org/#dom-node-isequalnode (for everything still in the living spec)
https://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/DOM3-Core.html#core-Node3-isEqualNode (for old nodes removed from the living spec)
@theseer
Copy link
Contributor

theseer commented Jul 17, 2023

Sorry for taking long. As far as I understand the spec and the test output, I'd say this is correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants