Skip to content
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

DOMNodeComparator swallows comments #107

Open
theseer opened this issue Jul 27, 2023 · 3 comments
Open

DOMNodeComparator swallows comments #107

theseer opened this issue Jul 27, 2023 · 3 comments

Comments

@theseer
Copy link

theseer commented Jul 27, 2023

In DOMNodeComparator::nodeToText we call $node->C14n(); without any parameters.

That results in all Comment (<!-- ... -->) nodes to be stripped.

I'd consider that a bug.

Given we still have #87, and maybe #71, we should probably have a look into how this comparator works :)

@theseer
Copy link
Author

theseer commented Jul 27, 2023

Additionally, the DOMNodeComparator::nodeToText has a bool parameter $canonicalize suggesting it could be optional. But the only caller, DOMNodeComparator::assertEquals, hardcodes it to true in both cases.

I guess we really need to refactor this one :)

@theseer
Copy link
Author

theseer commented Jul 27, 2023

Maybe we can use some of PR #88 while doing so.

@WinterSilence
Copy link

@theseer #97

SnakeCharm3r pushed a commit to SnakeCharm3r/comparator that referenced this issue Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants