Skip to content

Commit 07cda48

Browse files
ndosschendossche
committed
dom: Fix ID spec compliance strictness
1 parent f11ea2a commit 07cda48

File tree

7 files changed

+102
-31
lines changed

7 files changed

+102
-31
lines changed

‎ext/dom/element.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -394,8 +394,7 @@ static void dom_check_register_attribute_id(xmlAttrPtr attr, php_libxml_ref_obj
394394
{
395395
dom_mark_ids_modified(document);
396396

397-
if (attr->atype != XML_ATTRIBUTE_ID && attr->doc->type == XML_HTML_DOCUMENT_NODE && attr->ns == NULL && xmlStrEqual(attr->name, BAD_CAST "id")) {
398-
/* To respect XML's ID behaviour, we only do this registration for HTML documents. */
397+
if (attr->atype != XML_ATTRIBUTE_ID && attr->ns == NULL && xmlStrEqual(attr->name, BAD_CAST "id")) {
399398
attr->atype = XML_ATTRIBUTE_ID;
400399
}
401400
}

‎ext/dom/tests/bug79701/remove_attribute.phpt‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ Bug #79701 (getElementById does not correctly work with duplicate definitions) -
44
dom
55
--FILE--
66
<?php
7-
$dom = Dom\XMLDocument::createFromString(<<<XML
7+
$dom = new DOMDocument;
8+
$dom->loadXML(<<<XML
89
<root>
910
<test1 xml:id="x"/>
1011
<test2 xml:id="x"/>
@@ -16,6 +17,6 @@ $dom->getElementById('x')->removeAttribute('xml:id');
1617
var_dump($dom->getElementById('x')?->nodeName);
1718
?>
1819
--EXPECTF--
19-
Warning: Dom\XMLDocument::createFromString(): ID x already defined in Entity, line: 3 in %s on line %d
20+
Warning: DOMDocument::loadXML(): ID x already defined in Entity, line: 3 in %s on line %d
2021
string(5) "test1"
2122
NULL

‎ext/dom/tests/bug79701/set_attr_value.phpt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ dom
66
<?php
77
foreach (["value", "nodeValue"] as $property) {
88
echo "--- Testing property \$$property ---\n";
9-
$dom = Dom\XMLDocument::createFromString(<<<XML
9+
$dom = new DOMDocument;
10+
$dom->loadXML(<<<XML
1011
<root>
1112
<test1 xml:id="x"/>
1213
<test2 xml:id="y"/>

‎ext/dom/tests/bug79701/set_attribute_xml.phpt‎

Lines changed: 52 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,31 +4,31 @@ Bug #79701 (getElementById does not correctly work with duplicate definitions) -
44
dom
55
--FILE--
66
<?php
7-
function test($dom, $fn) {
7+
function test($dom, $prefix, $fn) {
88
$test1 = $dom->getElementById('x');
99
$test2 = $dom->getElementById('y');
1010

1111
echo "--- After resetting test1's id ---\n";
1212

13-
$fn($test1, 'xml:id', 'y');
13+
$fn($test1, $prefix . 'id', 'y');
1414
var_dump($dom->getElementById('x')?->nodeName);
1515
var_dump($dom->getElementById('y')?->nodeName);
1616

1717
echo "--- After resetting test2's id ---\n";
1818

19-
$fn($test2, 'xml:id', 'x');
19+
$fn($test2, $prefix . 'id', 'x');
2020
var_dump($dom->getElementById('x')?->nodeName);
2121
var_dump($dom->getElementById('y')?->nodeName);
2222

2323
echo "--- After resetting test1's id ---\n";
2424

25-
$fn($test1, 'xml:id', 'z');
25+
$fn($test1, $prefix . 'id', 'z');
2626
var_dump($dom->getElementById('x')?->nodeName);
2727
var_dump($dom->getElementById('y')?->nodeName);
2828

2929
echo "--- After resetting test2's id ---\n";
3030

31-
$fn($test2, 'xml:id', 'z');
31+
$fn($test2, $prefix . 'id', 'z');
3232
var_dump($dom->getElementById('x')?->nodeName);
3333
var_dump($dom->getElementById('y')?->nodeName);
3434

@@ -51,30 +51,43 @@ $common_xml = <<<XML
5151
</root>
5252
XML;
5353

54-
echo "\n=== DOMDocument: setAttribute ===\n\n";
54+
echo "\n=== DOMDocument: setAttribute (prefixed) ===\n\n";
5555

5656
$dom = new DOMDocument;
5757
$dom->loadXML($common_xml);
58-
test($dom, fn ($element, $name, $value) => $element->setAttribute($name, $value));
58+
test($dom, "xml:", fn ($element, $name, $value) => $element->setAttribute($name, $value));
5959

60-
echo "\n=== DOMDocument: setAttributeNS ===\n\n";
60+
echo "\n=== DOMDocument: setAttributeNS (prefixed) ===\n\n";
6161

6262
$dom = new DOMDocument;
6363
$dom->loadXML($common_xml);
64-
test($dom, fn ($element, $name, $value) => $element->setAttributeNS(getNamespace($name), $name, $value));
64+
test($dom, "xml:", fn ($element, $name, $value) => $element->setAttributeNS(getNamespace($name), $name, $value));
6565

66-
echo "\n=== Dom\\XMLDocument: setAttribute ===\n\n";
66+
echo "\n=== Dom\\XMLDocument: setAttribute (prefixed) ===\n\n";
6767

6868
$dom = Dom\XMLDocument::createFromString($common_xml);
69-
test($dom, fn ($element, $name, $value) => $element->setAttribute($name, $value));
69+
test($dom, "xml:", fn ($element, $name, $value) => $element?->setAttribute($name, $value));
70+
71+
echo "\n=== Dom\\XMLDocument: setAttributeNS (prefixed) ===\n\n";
72+
73+
$dom = Dom\XMLDocument::createFromString($common_xml);
74+
test($dom, "xml:", fn ($element, $name, $value) => $element?->setAttribute(getNamespace($name), $name, $value));
75+
76+
echo "\n=== Dom\\XMLDocument: setAttribute ===\n\n";
7077

71-
echo "\n=== Dom\\XMLDocument: setAttributeNS ===\n\n";
78+
$common_xml = <<<XML
79+
<root>
80+
<test1 id="x"/>
81+
<test2 id="y"/>
82+
</root>
83+
XML;
7284

7385
$dom = Dom\XMLDocument::createFromString($common_xml);
74-
test($dom, fn ($element, $name, $value) => $element->setAttributeNS(getNamespace($name), $name, $value));
86+
test($dom, "", fn ($element, $name, $value) => $element?->setAttribute($name, $value));
87+
7588
?>
7689
--EXPECT--
77-
=== DOMDocument: setAttribute ===
90+
=== DOMDocument: setAttribute (prefixed) ===
7891

7992
--- After resetting test1's id ---
8093
NULL
@@ -91,7 +104,7 @@ NULL
91104
--- Get id z ---
92105
string(5) "test1"
93106

94-
=== DOMDocument: setAttributeNS ===
107+
=== DOMDocument: setAttributeNS (prefixed) ===
95108

96109
--- After resetting test1's id ---
97110
NULL
@@ -108,24 +121,41 @@ NULL
108121
--- Get id z ---
109122
string(5) "test1"
110123

111-
=== Dom\XMLDocument: setAttribute ===
124+
=== Dom\XMLDocument: setAttribute (prefixed) ===
112125

113126
--- After resetting test1's id ---
114127
NULL
115-
string(5) "test1"
128+
NULL
116129
--- After resetting test2's id ---
117-
string(5) "test2"
118-
string(5) "test1"
130+
NULL
131+
NULL
119132
--- After resetting test1's id ---
120-
string(5) "test2"
133+
NULL
121134
NULL
122135
--- After resetting test2's id ---
123136
NULL
124137
NULL
125138
--- Get id z ---
126-
string(5) "test1"
139+
NULL
140+
141+
=== Dom\XMLDocument: setAttributeNS (prefixed) ===
127142

128-
=== Dom\XMLDocument: setAttributeNS ===
143+
--- After resetting test1's id ---
144+
NULL
145+
NULL
146+
--- After resetting test2's id ---
147+
NULL
148+
NULL
149+
--- After resetting test1's id ---
150+
NULL
151+
NULL
152+
--- After resetting test2's id ---
153+
NULL
154+
NULL
155+
--- Get id z ---
156+
NULL
157+
158+
=== Dom\XMLDocument: setAttribute ===
129159

130160
--- After resetting test1's id ---
131161
NULL

‎ext/dom/tests/modern/css_selectors/closest.phpt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ dom
88
$xml = <<<XML
99
<root>
1010
<a/>
11-
<div class="foo" xml:id="div1">
12-
<div xml:id="div2">
13-
<div class="bar" xml:id="div3"/>
11+
<div class="foo" id="div1">
12+
<div id="div2">
13+
<div class="bar" id="div3"/>
1414
</div>
1515
</div>
1616
</root>
@@ -20,7 +20,7 @@ $dom = DOM\XMLDocument::createFromString($xml);
2020

2121
function test($el, $selector) {
2222
echo "--- Selector: $selector ---\n";
23-
var_dump($el->closest($selector)?->getAttribute('xml:id'));
23+
var_dump($el->closest($selector)?->getAttribute('id'));
2424
}
2525

2626
test($dom->getElementById('div3'), 'div');

‎ext/dom/tests/modern/xml/id_registration_strictness.phpt‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
--TEST--
2+
Strictness of ID registration
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
$dom = Dom\XMLDocument::createFromString('<root><child0 xml:id="a"/><child1 id="a"/><child2 id="b"/></root>');
9+
var_dump($dom->getElementById('a')?->tagName);
10+
var_dump($dom->getElementById('b')?->tagName);
11+
$dom->documentElement->setAttribute('id', 'a');
12+
var_dump($dom->getElementById('a')?->tagName);
13+
$dom->documentElement->setAttribute('id', '');
14+
var_dump($dom->getElementById('a')?->tagName);
15+
$dom->documentElement->setAttribute('ID', 'a');
16+
var_dump($dom->getElementById('a')?->tagName);
17+
18+
?>
19+
--EXPECT--
20+
string(6) "child1"
21+
string(6) "child2"
22+
string(4) "root"
23+
string(6) "child1"
24+
string(6) "child1"

‎ext/dom/xml_document.c‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,22 @@ void dom_mark_namespaces_as_attributes_too(php_dom_libxml_ns_mapper *ns_mapper,
8383
xmlNodePtr node = doc->children;
8484
while (node != NULL) {
8585
if (node->type == XML_ELEMENT_NODE) {
86+
/* Apply ID rules of WHATWG DOM. */
87+
for (xmlAttrPtr attr = node->properties; attr != NULL; attr = attr->next) {
88+
if (attr->atype == XML_ATTRIBUTE_ID) {
89+
if (attr->ns != NULL || !xmlStrEqual(attr->name, BAD_CAST "id")) {
90+
xmlRemoveID(doc, attr);
91+
}
92+
} else if (attr->atype == 0 && attr->ns == NULL && xmlStrEqual(attr->name, BAD_CAST "id")) {
93+
bool should_free;
94+
xmlChar *value = php_libxml_attr_value(attr, &should_free);
95+
xmlAddID(NULL, doc, value, attr);
96+
if (UNEXPECTED(should_free)) {
97+
xmlFree(value);
98+
}
99+
}
100+
}
101+
86102
php_dom_ns_compat_mark_attribute_list(ns_mapper, node);
87103
}
88104

0 commit comments

Comments
 (0)