Skip to content

Fix bug #77686: Removed elements are still returned by getElementById #11369

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

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Jun 4, 2023

From the moment an ID is created, libxml2's behaviour is to cache that element, even if that element is not yet attached to the document. Similarly, only upon destruction of the element the ID is actually removed by libxml2. Since libxml2 has such behaviour deeply ingrained in the library, and uses the cache for various purposes, it seems like a bad idea and lost cause to fight it. Instead, we'll simply walk the tree upwards to check if the node is attached to the document.

There was a previous PR attempt for this here: #6936. The approach in this PR is different and does not increase the cost of element removal.

From the moment an ID is created, libxml2's behaviour is to cache that element,
even if that element is not yet attached to the document. Similarly, only upon
destruction of the element the ID is actually removed by libxml2.
Since libxml2 has such behaviour deeply ingrained in the library, and uses the
cache for various purposes, it seems like a bad idea and lost cause to fight it.
Instead, we'll simply walk the tree upwards to check if the node is attached to
the document.
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.

LGTM

@nielsdos nielsdos closed this in 0e34ac8 Jun 4, 2023
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

Successfully merging this pull request may close these issues.

2 participants