Skip to content

Fix bug #79701: getElementById does not correctly work with duplicate definitions #14349

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

This is a long standing bug: IDs aren't properly tracked causing either outdated or plain incorrect results from getElementById.

This PR implements a pragmatic solution in which we still try to use the ID lookup table to a degree, but only as a performance boost not as a "single source of truth". Full details are explained in the getElementById code.

This is marked as WIP for now as I need to do further validations. But this will be the main idea.

@nielsdos nielsdos force-pushed the dom-fix-getelementbyid branch 3 times, most recently from 671de4f to 1892be2 Compare May 31, 2024 18:13
@nielsdos nielsdos marked this pull request as ready for review May 31, 2024 18:13
@nielsdos nielsdos requested a review from Girgias May 31, 2024 18:13
@nielsdos nielsdos force-pushed the dom-fix-getelementbyid branch from 1892be2 to d8a2b66 Compare May 31, 2024 18:22
… definitions

This is a long standing bug: IDs aren't properly tracked causing either
outdated or plain incorrect results from getElementById.

This PR implements a pragmatic solution in which we still try to use the
ID lookup table to a degree, but only as a performance boost not as a
"single source of truth". Full details are explained in the
getElementById code.
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.

Logic and test expectations look correct

@nielsdos nielsdos closed this in 8dc2391 Jun 1, 2024
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.

2 participants