Fix #77686: Immediately remove ID from DOMDocument when removing child element #6936
Conversation
Prior a removed node was still returned from DOMDocument::getElementById() until the DOMElement object was destroyed. Now null is returned from getElementById() even if a reference remains to the removed DOMElement object.
|
cc @beberlei |
|
I need to dig into this a little further to see what's going on, as re-appending the child then does not find the element by Id. Seems PHP is associating the ID with the document at element creation, however, I'm not sure this is the correct behavior. JavaScript behavior: let doc = document.implementation.createHTMLDocument();
let body = doc.getElementsByTagName('body')[0];
let div = doc.createElement('div');
div.setAttribute('id', 'y');
console.log(doc.getElementById('y')); // null
body.appendChild(div);
console.log(doc.getElementById('y').tagName); // "DIV"
body.removeChild(div);
console.log(doc.getElementById('y')); // nullCompared to the current PHP behavior: $doc = new DOMDocument;
$doc->loadHTML('<html><body></body></html>');
$body = $doc->getElementsByTagName('body')[0];
$div = $doc->createElement('div');
$div->setAttribute('id', 'y');
var_dump($doc->getElementById('y')->tagName); // "div"
$body->appendChild($div);
var_dump($doc->getElementById('y')->tagName); // "div"
$body->removeChild($div);
var_dump($doc->getElementById('y')->tagName); // "div" |
This might be intrinsic libxml2 behavior. :( Anyhow, we should also check the behavior wrt. duplicate IDs (cf. https://bugs.php.net/79701). |
|
Yes, I think it is intrinsic to libxml2. I was hoping to fix the behavior noted in this test in a Wikimedia project. At first glance it seemed simple, but digging further into the code it looks like libxml2 is doing this automatically. I could match JavaScript with a lot of extra work, but that's probably not a good idea both for maintainability and backward compatibility. Bug 79701 looks like a completely separate issue. Perhaps |
|
I am honestly still a bit overwhelmed by libxml inner workings and lucky to have found one corner that I did master, there are a bunch of thing sin ext/dom just to make libxml happy, for example |
In the code below, the current behavior of the last two calls to
DOMDocument::getElementById()is returning theDOMElementobjects that have been removed from the document unless theDOMElementobject is destroyed.With this patch,
nullis returned fromgetElementById()immediately after an element is removed from the document even if a reference remains to the removedDOMElementobject.This similar code below demonstrates the current inconsistency, where
nullis returned for the last two calls toDOMDocument::getElementById()because no reference is kept to the removedDOMElement.