Skip to content

Fix GH-20281: \Dom\Document::getElementById() is inconsistent after nodes are removed #20283

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.

Sign up for GitHub

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
+19 −1
Closed

Fix GH-20281: \Dom\Document::getElementById() is inconsistent after nodes are removed #20283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion ext/dom/html5_parser.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,10 @@ static lexbor_libxml2_bridge_status lexbor_libxml2_bridge_convert(

/* xmlIsID does some other stuff too that is irrelevant here. */
if (local_name_length == 2 && local_name[0] == 'i' && local_name[1] == 'd' && attr->node.ns == LXB_NS_HTML) {
xmlAddID(NULL, lxml_doc, value, lxml_attr);
if (xmlAddID(NULL, lxml_doc, value, lxml_attr) == 0) {
/* If the ID already exists, the ID attribute still needs to be marked as an ID. */
Copy link
Member

Choose a reason for hiding this comment

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

just confused by one thing though, would it happen only if xmlAddID 1st arg was not NULL ?

Copy link
Member Author

Choose a reason for hiding this comment

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

The first argument is a validation context. The only influence it has is in error reporting: if you pass a non-NULL pointer the error handler of that validation context will be used to emit an error saying there's a duplicate ID.

lxml_attr->atype = XML_ATTRIBUTE_ID;
}
}

/* libxml2 doesn't support line numbers on this anyway, it derives them instead, so don't bother */
Expand Down
15 changes: 15 additions & 0 deletions ext/dom/tests/modern/html/interactions/gh20281.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
--TEST--
GH-20281 (\Dom\Document::getElementById() is inconsistent after nodes are removed)
--EXTENSIONS--
dom
--CREDITS--
cscott
--FILE--
<?php
$d = \Dom\HTMLDocument::createFromString('<p id="a">b</p><p id="a">c</p>', LIBXML_NOERROR);
$p = $d->getElementById('a');
$p->remove();
echo $d->getElementById('a')->textContent, "\n";
?>
--EXPECT--
c