From 4cfe55bd224cf4b2ff2035350128b54baadd310f Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Thu, 13 Jul 2023 13:08:19 +0200 Subject: [PATCH 1/3] Split off some methods so they can be reused in different places --- ext/dom/element.c | 58 +++++++++++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 24 deletions(-) diff --git a/ext/dom/element.c b/ext/dom/element.c index 8acbac3f964e0..6fad82ba1c31a 100644 --- a/ext/dom/element.c +++ b/ext/dom/element.c @@ -355,6 +355,15 @@ PHP_METHOD(DOMElement, getAttributeNames) } /* }}} end DOMElement::getAttributeNames() */ +static xmlNodePtr dom_create_attribute(xmlNodePtr nodep, const char *name, const char* value) +{ + if (xmlStrEqual((xmlChar *)name, (xmlChar *)"xmlns")) { + return (xmlNodePtr) xmlNewNs(nodep, (xmlChar *)value, NULL); + } else { + return (xmlNodePtr) xmlSetProp(nodep, (xmlChar *) name, (xmlChar *)value); + } +} + /* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core-ID-F68F082 Since: */ @@ -405,23 +414,40 @@ PHP_METHOD(DOMElement, setAttribute) } - if (xmlStrEqual((xmlChar *)name, (xmlChar *)"xmlns")) { - if (xmlNewNs(nodep, (xmlChar *)value, NULL)) { - RETURN_TRUE; - } - } else { - attr = (xmlNodePtr)xmlSetProp(nodep, (xmlChar *) name, (xmlChar *)value); - } + attr = dom_create_attribute(nodep, name, value); if (!attr) { zend_argument_value_error(1, "must be a valid XML attribute"); RETURN_THROWS(); } + if (attr->type == XML_NAMESPACE_DECL) { + RETURN_TRUE; + } DOM_RET_OBJ(attr, &ret, intern); } /* }}} end dom_element_set_attribute */ +static bool dom_remove_attribute(xmlNodePtr attrp) +{ + ZEND_ASSERT(attrp != NULL); + switch (attrp->type) { + case XML_ATTRIBUTE_NODE: + if (php_dom_object_get_data(attrp) == NULL) { + node_list_unlink(attrp->children); + xmlUnlinkNode(attrp); + xmlFreeProp((xmlAttrPtr)attrp); + } else { + xmlUnlinkNode(attrp); + } + break; + case XML_NAMESPACE_DECL: + return false; + EMPTY_SWITCH_DEFAULT_CASE(); + } + return true; +} + /* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core-ID-6D6AC0F9 Since: */ @@ -450,23 +476,7 @@ PHP_METHOD(DOMElement, removeAttribute) RETURN_FALSE; } - switch (attrp->type) { - case XML_ATTRIBUTE_NODE: - if (php_dom_object_get_data(attrp) == NULL) { - node_list_unlink(attrp->children); - xmlUnlinkNode(attrp); - xmlFreeProp((xmlAttrPtr)attrp); - } else { - xmlUnlinkNode(attrp); - } - break; - case XML_NAMESPACE_DECL: - RETURN_FALSE; - default: - break; - } - - RETURN_TRUE; + RETURN_BOOL(dom_remove_attribute(attrp)); } /* }}} end dom_element_remove_attribute */ From 949961960fad3c780357335428e08e8213a97670 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Thu, 13 Jul 2023 13:16:40 +0200 Subject: [PATCH 2/3] Implement DOMElement::toggleAttribute() ref: https://dom.spec.whatwg.org/#dom-element-toggleattribute --- NEWS | 1 + UPGRADING | 1 + ext/dom/element.c | 106 ++++++++++++ ext/dom/php_dom.stub.php | 2 + ext/dom/php_dom_arginfo.h | 9 +- ext/dom/tests/DOMElement_toggleAttribute.phpt | 151 ++++++++++++++++++ 6 files changed, 269 insertions(+), 1 deletion(-) create mode 100644 ext/dom/tests/DOMElement_toggleAttribute.phpt diff --git a/NEWS b/NEWS index 32cd1b6b1dc54..f875e9d3af022 100644 --- a/NEWS +++ b/NEWS @@ -31,6 +31,7 @@ PHP NEWS . Added DOMNode::isEqualNode(). (nielsdos) . Added DOMElement::insertAdjacentElement() and DOMElement::insertAdjacentText(). (nielsdos) + . Added DOMElement::toggleAttribute(). (nielsdos) - FPM: . Added warning to log when fpm socket was not registered on the expected diff --git a/UPGRADING b/UPGRADING index 50eb6a08f9ae7..786c0e62e607f 100644 --- a/UPGRADING +++ b/UPGRADING @@ -281,6 +281,7 @@ PHP 8.3 UPGRADE NOTES . Added DOMNode::isEqualNode(). . Added DOMElement::insertAdjacentElement() and DOMElement::insertAdjacentText(). + . Added DOMElement::toggleAttribute(). - JSON: . Added json_validate(), which returns whether the json is valid for diff --git a/ext/dom/element.c b/ext/dom/element.c index 6fad82ba1c31a..048c71c936642 100644 --- a/ext/dom/element.c +++ b/ext/dom/element.c @@ -1470,5 +1470,111 @@ PHP_METHOD(DOMElement, insertAdjacentText) } } /* }}} end DOMElement::insertAdjacentText */ +/* {{{ URL: https://dom.spec.whatwg.org/#dom-element-toggleattribute +Since: +*/ +PHP_METHOD(DOMElement, toggleAttribute) +{ + char *qname, *qname_tmp = NULL; + size_t qname_length; + bool force, force_is_null = true; + xmlNodePtr thisp; + zval *id; + dom_object *intern; + bool retval; + + if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|b!", &qname, &qname_length, &force, &force_is_null) == FAILURE) { + RETURN_THROWS(); + } + + DOM_GET_THIS_OBJ(thisp, id, xmlNodePtr, intern); + + /* Step 1 */ + if (xmlValidateName((xmlChar *) qname, 0) != 0) { + php_dom_throw_error(INVALID_CHARACTER_ERR, 1); + RETURN_THROWS(); + } + + /* Step 2 */ + if (thisp->doc->type == XML_HTML_DOCUMENT_NODE && (thisp->ns == NULL || xmlStrEqual(thisp->ns->href, (const xmlChar *) "http://www.w3.org/1999/xhtml"))) { + qname_tmp = zend_str_tolower_dup_ex(qname, qname_length); + if (qname_tmp != NULL) { + qname = qname_tmp; + } + } + + /* Step 3 */ + xmlNodePtr attribute = dom_get_dom1_attribute(thisp, (xmlChar *) qname); + + /* Step 4 */ + if (attribute == NULL) { + /* Step 4.1 */ + if (force_is_null || force) { + /* The behaviour for namespaces isn't defined by spec, but this is based on observing browers behaviour. + * It follows the same rules when you'd manually add an attribute using the other APIs. */ + int len; + const xmlChar *split = xmlSplitQName3((const xmlChar *) qname, &len); + if (split == NULL || strncmp(qname, "xmlns:", len + 1) != 0) { + /* unqualified name, or qualified name with no xml namespace declaration */ + dom_create_attribute(thisp, qname, ""); + } else { + /* qualified name with xml namespace declaration */ + xmlNewNs(thisp, (const xmlChar *) "", (const xmlChar *) (qname + len + 1)); + } + retval = true; + goto out; + } + /* Step 4.2 */ + retval = false; + goto out; + } + + /* Step 5 */ + if (force_is_null || !force) { + if (attribute->type == XML_NAMESPACE_DECL) { + /* The behaviour isn't defined by spec, but by observing browsers I found + * that you can remove the nodes, but they'll get reconciled. + * So if any reference was left to the namespace, the only effect is that + * the definition is potentially moved closer to the element using it. + * If no reference was left, it is actually removed. */ + xmlNsPtr ns = (xmlNsPtr) attribute; + if (thisp->nsDef == ns) { + thisp->nsDef = ns->next; + } else if (thisp->nsDef != NULL) { + xmlNsPtr prev = thisp->nsDef; + xmlNsPtr cur = prev->next; + while (cur) { + if (cur == ns) { + prev->next = cur->next; + break; + } + prev = cur; + cur = cur->next; + } + } + + ns->next = NULL; + dom_set_old_ns(thisp->doc, ns); + dom_reconcile_ns(thisp->doc, thisp); + } else { + /* TODO: in the future when namespace bugs are fixed, + * the above if-branch should be merged into this called function + * such that the removal will work properly with all APIs. */ + dom_remove_attribute(attribute); + } + retval = false; + goto out; + } + + /* Step 6 */ + retval = true; + +out: + if (qname_tmp) { + efree(qname_tmp); + } + RETURN_BOOL(retval); +} +/* }}} end DOMElement::prepend */ #endif diff --git a/ext/dom/php_dom.stub.php b/ext/dom/php_dom.stub.php index e02036b586baa..29aaf07e4b177 100644 --- a/ext/dom/php_dom.stub.php +++ b/ext/dom/php_dom.stub.php @@ -642,6 +642,8 @@ public function setIdAttributeNS(string $namespace, string $qualifiedName, bool /** @tentative-return-type */ public function setIdAttributeNode(DOMAttr $attr, bool $isId): void {} + public function toggleAttribute(string $qualifiedName, ?bool $force = null): bool {} + public function remove(): void {} /** @param DOMNode|string $nodes */ diff --git a/ext/dom/php_dom_arginfo.h b/ext/dom/php_dom_arginfo.h index 796554a189e3f..28bc38296369d 100644 --- a/ext/dom/php_dom_arginfo.h +++ b/ext/dom/php_dom_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 850ab297bd3e6162e0497769cace87a41e8e8a00 */ + * Stub hash: 3a37adaf011606d10ae1fa12ce135a23b3e07cf4 */ ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(arginfo_dom_import_simplexml, 0, 1, DOMElement, 0) ZEND_ARG_TYPE_INFO(0, node, IS_OBJECT, 0) @@ -282,6 +282,11 @@ ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_INFO_EX(arginfo_class_DOMElement_setId ZEND_ARG_TYPE_INFO(0, isId, _IS_BOOL, 0) ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_DOMElement_toggleAttribute, 0, 1, _IS_BOOL, 0) + ZEND_ARG_TYPE_INFO(0, qualifiedName, IS_STRING, 0) + ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, force, _IS_BOOL, 1, "null") +ZEND_END_ARG_INFO() + #define arginfo_class_DOMElement_remove arginfo_class_DOMChildNode_remove #define arginfo_class_DOMElement_before arginfo_class_DOMParentNode_append @@ -591,6 +596,7 @@ ZEND_METHOD(DOMElement, setAttributeNodeNS); ZEND_METHOD(DOMElement, setIdAttribute); ZEND_METHOD(DOMElement, setIdAttributeNS); ZEND_METHOD(DOMElement, setIdAttributeNode); +ZEND_METHOD(DOMElement, toggleAttribute); ZEND_METHOD(DOMElement, remove); ZEND_METHOD(DOMElement, before); ZEND_METHOD(DOMElement, after); @@ -817,6 +823,7 @@ static const zend_function_entry class_DOMElement_methods[] = { ZEND_ME(DOMElement, setIdAttribute, arginfo_class_DOMElement_setIdAttribute, ZEND_ACC_PUBLIC) ZEND_ME(DOMElement, setIdAttributeNS, arginfo_class_DOMElement_setIdAttributeNS, ZEND_ACC_PUBLIC) ZEND_ME(DOMElement, setIdAttributeNode, arginfo_class_DOMElement_setIdAttributeNode, ZEND_ACC_PUBLIC) + ZEND_ME(DOMElement, toggleAttribute, arginfo_class_DOMElement_toggleAttribute, ZEND_ACC_PUBLIC) ZEND_ME(DOMElement, remove, arginfo_class_DOMElement_remove, ZEND_ACC_PUBLIC) ZEND_ME(DOMElement, before, arginfo_class_DOMElement_before, ZEND_ACC_PUBLIC) ZEND_ME(DOMElement, after, arginfo_class_DOMElement_after, ZEND_ACC_PUBLIC) diff --git a/ext/dom/tests/DOMElement_toggleAttribute.phpt b/ext/dom/tests/DOMElement_toggleAttribute.phpt new file mode 100644 index 0000000000000..b466c9be0795f --- /dev/null +++ b/ext/dom/tests/DOMElement_toggleAttribute.phpt @@ -0,0 +1,151 @@ +--TEST-- +DOMElement::toggleAttribute() +--EXTENSIONS-- +dom +--FILE-- +loadHTML(''); +$xml = new DOMDocument(); +$xml->loadXML(''); + +try { + var_dump($html->documentElement->toggleAttribute("\0")); +} catch (DOMException $e) { + echo $e->getMessage(), "\n"; +} + +echo "--- Selected attribute tests (HTML) ---\n"; + +var_dump($html->documentElement->toggleAttribute("SELECTED", false)); +echo $html->saveHTML(); +var_dump($html->documentElement->toggleAttribute("SELECTED")); +echo $html->saveHTML(); +var_dump($html->documentElement->toggleAttribute("selected", true)); +echo $html->saveHTML(); +var_dump($html->documentElement->toggleAttribute("selected")); +echo $html->saveHTML(); + +echo "--- Selected attribute tests (XML) ---\n"; + +var_dump($xml->documentElement->toggleAttribute("SELECTED", false)); +echo $xml->saveXML(); +var_dump($xml->documentElement->toggleAttribute("SELECTED")); +echo $xml->saveXML(); +var_dump($xml->documentElement->toggleAttribute("selected", true)); +echo $xml->saveXML(); +var_dump($xml->documentElement->toggleAttribute("selected")); +echo $xml->saveXML(); + +echo "--- id attribute tests ---\n"; + +var_dump($html->getElementById("test") === NULL); +var_dump($html->documentElement->toggleAttribute("id")); +var_dump($html->getElementById("test") === NULL); + +echo "--- Namespace tests ---\n"; + +$dom = new DOMDocument(); +$dom->loadXML(""); + +echo "Toggling namespaces:\n"; +var_dump($dom->documentElement->toggleAttribute('xmlns')); +echo $dom->saveXML(); +var_dump($dom->documentElement->toggleAttribute('xmlns:anotherone')); +echo $dom->saveXML(); +var_dump($dom->documentElement->toggleAttribute('xmlns:anotherone')); +echo $dom->saveXML(); +var_dump($dom->documentElement->toggleAttribute('xmlns:foo')); +echo $dom->saveXML(); + +echo "Toggling namespaced attributes:\n"; +var_dump($dom->documentElement->toggleAttribute('test:test')); +var_dump($dom->documentElement->firstElementChild->toggleAttribute('foo:test')); +var_dump($dom->documentElement->firstElementChild->toggleAttribute('doesnotexist:test')); +echo $dom->saveXML(); + +echo "namespace of test:test = "; +var_dump($dom->documentElement->getAttributeNode('test:test')->namespaceURI); +echo "namespace of foo:test = "; +var_dump($dom->documentElement->firstElementChild->getAttributeNode('foo:test')->namespaceURI); +echo "namespace of doesnotexist:test = "; +var_dump($dom->documentElement->firstElementChild->getAttributeNode('doesnotexist:test')->namespaceURI); + +echo "Toggling namespaced attributes:\n"; +var_dump($dom->documentElement->toggleAttribute('test:test')); +var_dump($dom->documentElement->firstElementChild->toggleAttribute('foo:test')); +var_dump($dom->documentElement->firstElementChild->toggleAttribute('doesnotexist:test')); +echo $dom->saveXML(); + +echo "Checking toggled namespace:\n"; +var_dump($dom->documentElement->getAttribute('xmlns:anotheron')); + +?> +--EXPECT-- +Invalid Character Error +--- Selected attribute tests (HTML) --- +bool(false) + + +bool(true) + + +bool(true) + + +bool(false) + + +--- Selected attribute tests (XML) --- +bool(false) + + + +bool(true) + + + +bool(true) + + + +bool(false) + + + +--- id attribute tests --- +bool(false) +bool(false) +bool(true) +--- Namespace tests --- +Toggling namespaces: +bool(false) + + +bool(false) + + +bool(true) + + +bool(false) + + +Toggling namespaced attributes: +bool(true) +bool(true) +bool(true) + + +namespace of test:test = NULL +namespace of foo:test = string(8) "some:ns2" +namespace of doesnotexist:test = NULL +Toggling namespaced attributes: +bool(false) +bool(false) +bool(false) + + +Checking toggled namespace: +string(0) "" From 9f1905572687d5b0775a82266e495f377b40d65c Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 17 Jul 2023 18:57:34 +0200 Subject: [PATCH 3/3] Amend the test with more cases --- ext/dom/tests/DOMElement_toggleAttribute.phpt | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/ext/dom/tests/DOMElement_toggleAttribute.phpt b/ext/dom/tests/DOMElement_toggleAttribute.phpt index b466c9be0795f..ed29be899dac2 100644 --- a/ext/dom/tests/DOMElement_toggleAttribute.phpt +++ b/ext/dom/tests/DOMElement_toggleAttribute.phpt @@ -8,7 +8,7 @@ dom $html = new DOMDocument(); $html->loadHTML(''); $xml = new DOMDocument(); -$xml->loadXML(''); +$xml->loadXML(''); try { var_dump($html->documentElement->toggleAttribute("\0")); @@ -58,11 +58,14 @@ var_dump($dom->documentElement->toggleAttribute('xmlns:anotherone')); echo $dom->saveXML(); var_dump($dom->documentElement->toggleAttribute('xmlns:foo')); echo $dom->saveXML(); +var_dump($dom->documentElement->toggleAttribute('xmlns:nope', false)); +echo $dom->saveXML(); echo "Toggling namespaced attributes:\n"; var_dump($dom->documentElement->toggleAttribute('test:test')); var_dump($dom->documentElement->firstElementChild->toggleAttribute('foo:test')); var_dump($dom->documentElement->firstElementChild->toggleAttribute('doesnotexist:test')); +var_dump($dom->documentElement->firstElementChild->toggleAttribute('doesnotexist:test2', false)); echo $dom->saveXML(); echo "namespace of test:test = "; @@ -76,6 +79,8 @@ echo "Toggling namespaced attributes:\n"; var_dump($dom->documentElement->toggleAttribute('test:test')); var_dump($dom->documentElement->firstElementChild->toggleAttribute('foo:test')); var_dump($dom->documentElement->firstElementChild->toggleAttribute('doesnotexist:test')); +var_dump($dom->documentElement->firstElementChild->toggleAttribute('doesnotexist:test2', true)); +var_dump($dom->documentElement->firstElementChild->toggleAttribute('doesnotexist:test3', false)); echo $dom->saveXML(); echo "Checking toggled namespace:\n"; @@ -100,19 +105,15 @@ bool(false) --- Selected attribute tests (XML) --- bool(false) - bool(true) - bool(true) - bool(false) - --- id attribute tests --- bool(false) @@ -132,10 +133,14 @@ bool(true) bool(false) +bool(false) + + Toggling namespaced attributes: bool(true) bool(true) bool(true) +bool(false) namespace of test:test = NULL @@ -145,7 +150,9 @@ Toggling namespaced attributes: bool(false) bool(false) bool(false) +bool(true) +bool(false) - + Checking toggled namespace: string(0) ""