From 07cda48640f4b78a2519a0da6a97835e682e46f5 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 12 Jul 2025 20:44:33 +0200 Subject: [PATCH] dom: Fix ID spec compliance strictness --- ext/dom/element.c | 3 +- ext/dom/tests/bug79701/remove_attribute.phpt | 5 +- ext/dom/tests/bug79701/set_attr_value.phpt | 3 +- ext/dom/tests/bug79701/set_attribute_xml.phpt | 74 +++++++++++++------ .../tests/modern/css_selectors/closest.phpt | 8 +- .../xml/id_registration_strictness.phpt | 24 ++++++ ext/dom/xml_document.c | 16 ++++ 7 files changed, 102 insertions(+), 31 deletions(-) create mode 100644 ext/dom/tests/modern/xml/id_registration_strictness.phpt diff --git a/ext/dom/element.c b/ext/dom/element.c index ea18d0a817ede..41e08e798638f 100644 --- a/ext/dom/element.c +++ b/ext/dom/element.c @@ -394,8 +394,7 @@ static void dom_check_register_attribute_id(xmlAttrPtr attr, php_libxml_ref_obj { dom_mark_ids_modified(document); - if (attr->atype != XML_ATTRIBUTE_ID && attr->doc->type == XML_HTML_DOCUMENT_NODE && attr->ns == NULL && xmlStrEqual(attr->name, BAD_CAST "id")) { - /* To respect XML's ID behaviour, we only do this registration for HTML documents. */ + if (attr->atype != XML_ATTRIBUTE_ID && attr->ns == NULL && xmlStrEqual(attr->name, BAD_CAST "id")) { attr->atype = XML_ATTRIBUTE_ID; } } diff --git a/ext/dom/tests/bug79701/remove_attribute.phpt b/ext/dom/tests/bug79701/remove_attribute.phpt index 88fcc38c4563b..f5852f843aa98 100644 --- a/ext/dom/tests/bug79701/remove_attribute.phpt +++ b/ext/dom/tests/bug79701/remove_attribute.phpt @@ -4,7 +4,8 @@ Bug #79701 (getElementById does not correctly work with duplicate definitions) - dom --FILE-- loadXML(<< @@ -16,6 +17,6 @@ $dom->getElementById('x')->removeAttribute('xml:id'); var_dump($dom->getElementById('x')?->nodeName); ?> --EXPECTF-- -Warning: Dom\XMLDocument::createFromString(): ID x already defined in Entity, line: 3 in %s on line %d +Warning: DOMDocument::loadXML(): ID x already defined in Entity, line: 3 in %s on line %d string(5) "test1" NULL diff --git a/ext/dom/tests/bug79701/set_attr_value.phpt b/ext/dom/tests/bug79701/set_attr_value.phpt index 096e8b878c577..088b7fea4346a 100644 --- a/ext/dom/tests/bug79701/set_attr_value.phpt +++ b/ext/dom/tests/bug79701/set_attr_value.phpt @@ -6,7 +6,8 @@ dom loadXML(<< diff --git a/ext/dom/tests/bug79701/set_attribute_xml.phpt b/ext/dom/tests/bug79701/set_attribute_xml.phpt index 437b00a6da0c6..47eb6dec97c40 100644 --- a/ext/dom/tests/bug79701/set_attribute_xml.phpt +++ b/ext/dom/tests/bug79701/set_attribute_xml.phpt @@ -4,31 +4,31 @@ Bug #79701 (getElementById does not correctly work with duplicate definitions) - dom --FILE-- getElementById('x'); $test2 = $dom->getElementById('y'); echo "--- After resetting test1's id ---\n"; - $fn($test1, 'xml:id', 'y'); + $fn($test1, $prefix . 'id', 'y'); var_dump($dom->getElementById('x')?->nodeName); var_dump($dom->getElementById('y')?->nodeName); echo "--- After resetting test2's id ---\n"; - $fn($test2, 'xml:id', 'x'); + $fn($test2, $prefix . 'id', 'x'); var_dump($dom->getElementById('x')?->nodeName); var_dump($dom->getElementById('y')?->nodeName); echo "--- After resetting test1's id ---\n"; - $fn($test1, 'xml:id', 'z'); + $fn($test1, $prefix . 'id', 'z'); var_dump($dom->getElementById('x')?->nodeName); var_dump($dom->getElementById('y')?->nodeName); echo "--- After resetting test2's id ---\n"; - $fn($test2, 'xml:id', 'z'); + $fn($test2, $prefix . 'id', 'z'); var_dump($dom->getElementById('x')?->nodeName); var_dump($dom->getElementById('y')?->nodeName); @@ -51,30 +51,43 @@ $common_xml = << XML; -echo "\n=== DOMDocument: setAttribute ===\n\n"; +echo "\n=== DOMDocument: setAttribute (prefixed) ===\n\n"; $dom = new DOMDocument; $dom->loadXML($common_xml); -test($dom, fn ($element, $name, $value) => $element->setAttribute($name, $value)); +test($dom, "xml:", fn ($element, $name, $value) => $element->setAttribute($name, $value)); -echo "\n=== DOMDocument: setAttributeNS ===\n\n"; +echo "\n=== DOMDocument: setAttributeNS (prefixed) ===\n\n"; $dom = new DOMDocument; $dom->loadXML($common_xml); -test($dom, fn ($element, $name, $value) => $element->setAttributeNS(getNamespace($name), $name, $value)); +test($dom, "xml:", fn ($element, $name, $value) => $element->setAttributeNS(getNamespace($name), $name, $value)); -echo "\n=== Dom\\XMLDocument: setAttribute ===\n\n"; +echo "\n=== Dom\\XMLDocument: setAttribute (prefixed) ===\n\n"; $dom = Dom\XMLDocument::createFromString($common_xml); -test($dom, fn ($element, $name, $value) => $element->setAttribute($name, $value)); +test($dom, "xml:", fn ($element, $name, $value) => $element?->setAttribute($name, $value)); + +echo "\n=== Dom\\XMLDocument: setAttributeNS (prefixed) ===\n\n"; + +$dom = Dom\XMLDocument::createFromString($common_xml); +test($dom, "xml:", fn ($element, $name, $value) => $element?->setAttribute(getNamespace($name), $name, $value)); + +echo "\n=== Dom\\XMLDocument: setAttribute ===\n\n"; -echo "\n=== Dom\\XMLDocument: setAttributeNS ===\n\n"; +$common_xml = << + + + +XML; $dom = Dom\XMLDocument::createFromString($common_xml); -test($dom, fn ($element, $name, $value) => $element->setAttributeNS(getNamespace($name), $name, $value)); +test($dom, "", fn ($element, $name, $value) => $element?->setAttribute($name, $value)); + ?> --EXPECT-- -=== DOMDocument: setAttribute === +=== DOMDocument: setAttribute (prefixed) === --- After resetting test1's id --- NULL @@ -91,7 +104,7 @@ NULL --- Get id z --- string(5) "test1" -=== DOMDocument: setAttributeNS === +=== DOMDocument: setAttributeNS (prefixed) === --- After resetting test1's id --- NULL @@ -108,24 +121,41 @@ NULL --- Get id z --- string(5) "test1" -=== Dom\XMLDocument: setAttribute === +=== Dom\XMLDocument: setAttribute (prefixed) === --- After resetting test1's id --- NULL -string(5) "test1" +NULL --- After resetting test2's id --- -string(5) "test2" -string(5) "test1" +NULL +NULL --- After resetting test1's id --- -string(5) "test2" +NULL NULL --- After resetting test2's id --- NULL NULL --- Get id z --- -string(5) "test1" +NULL + +=== Dom\XMLDocument: setAttributeNS (prefixed) === -=== Dom\XMLDocument: setAttributeNS === +--- After resetting test1's id --- +NULL +NULL +--- After resetting test2's id --- +NULL +NULL +--- After resetting test1's id --- +NULL +NULL +--- After resetting test2's id --- +NULL +NULL +--- Get id z --- +NULL + +=== Dom\XMLDocument: setAttribute === --- After resetting test1's id --- NULL diff --git a/ext/dom/tests/modern/css_selectors/closest.phpt b/ext/dom/tests/modern/css_selectors/closest.phpt index 363ebe9f2fca1..0229bb8cfbec1 100644 --- a/ext/dom/tests/modern/css_selectors/closest.phpt +++ b/ext/dom/tests/modern/css_selectors/closest.phpt @@ -8,9 +8,9 @@ dom $xml = << -
-
-
+
+
+
@@ -20,7 +20,7 @@ $dom = DOM\XMLDocument::createFromString($xml); function test($el, $selector) { echo "--- Selector: $selector ---\n"; - var_dump($el->closest($selector)?->getAttribute('xml:id')); + var_dump($el->closest($selector)?->getAttribute('id')); } test($dom->getElementById('div3'), 'div'); diff --git a/ext/dom/tests/modern/xml/id_registration_strictness.phpt b/ext/dom/tests/modern/xml/id_registration_strictness.phpt new file mode 100644 index 0000000000000..8a4a84302b903 --- /dev/null +++ b/ext/dom/tests/modern/xml/id_registration_strictness.phpt @@ -0,0 +1,24 @@ +--TEST-- +Strictness of ID registration +--EXTENSIONS-- +dom +--FILE-- +'); +var_dump($dom->getElementById('a')?->tagName); +var_dump($dom->getElementById('b')?->tagName); +$dom->documentElement->setAttribute('id', 'a'); +var_dump($dom->getElementById('a')?->tagName); +$dom->documentElement->setAttribute('id', ''); +var_dump($dom->getElementById('a')?->tagName); +$dom->documentElement->setAttribute('ID', 'a'); +var_dump($dom->getElementById('a')?->tagName); + +?> +--EXPECT-- +string(6) "child1" +string(6) "child2" +string(4) "root" +string(6) "child1" +string(6) "child1" diff --git a/ext/dom/xml_document.c b/ext/dom/xml_document.c index 4d941de0f0686..397a97393950b 100644 --- a/ext/dom/xml_document.c +++ b/ext/dom/xml_document.c @@ -83,6 +83,22 @@ void dom_mark_namespaces_as_attributes_too(php_dom_libxml_ns_mapper *ns_mapper, xmlNodePtr node = doc->children; while (node != NULL) { if (node->type == XML_ELEMENT_NODE) { + /* Apply ID rules of WHATWG DOM. */ + for (xmlAttrPtr attr = node->properties; attr != NULL; attr = attr->next) { + if (attr->atype == XML_ATTRIBUTE_ID) { + if (attr->ns != NULL || !xmlStrEqual(attr->name, BAD_CAST "id")) { + xmlRemoveID(doc, attr); + } + } else if (attr->atype == 0 && attr->ns == NULL && xmlStrEqual(attr->name, BAD_CAST "id")) { + bool should_free; + xmlChar *value = php_libxml_attr_value(attr, &should_free); + xmlAddID(NULL, doc, value, attr); + if (UNEXPECTED(should_free)) { + xmlFree(value); + } + } + } + php_dom_ns_compat_mark_attribute_list(ns_mapper, node); }