Fix arginfo violation in removeChild() (#14717)

It was possible to return false without throwing an exception.
This is even wrong in "old DOM" because we expect either a NOT_FOUND_ERR
or NO_MODIFICATION_ALLOWED_ERR according to the documentation.
A side effect of this patch is that it prioritises NOT_FOUND_ERR over
NO_MODIFICATION_ALLOWED_ERR but I think that's fine.

Author: nielsdos

ext/dom/node.c

@@ -1229,22 +1229,18 @@ static void dom_node_remove_child(INTERNAL_FUNCTION_PARAMETERS, zend_class_entry
 
 	DOM_GET_OBJ(nodep, ZEND_THIS, xmlNodePtr, intern);
 
-	if (!dom_node_children_valid(nodep)) {
-		RETURN_FALSE;
-	}
-
 	DOM_GET_OBJ(child, node, xmlNodePtr, childobj);
 
 	bool stricterror = dom_get_strict_error(intern->document);
 
-	if (dom_node_is_read_only(nodep) == SUCCESS ||
-		(child->parent != NULL && dom_node_is_read_only(child->parent) == SUCCESS)) {
-		php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, stricterror);
+	if (!nodep->children || child->parent != nodep) {
+		php_dom_throw_error(NOT_FOUND_ERR, stricterror);
 		RETURN_FALSE;
 	}
 
-	if (!nodep->children || child->parent != nodep) {
-		php_dom_throw_error(NOT_FOUND_ERR, stricterror);
+	if (dom_node_is_read_only(nodep) == SUCCESS ||
+		(child->parent != NULL && dom_node_is_read_only(child->parent) == SUCCESS)) {
+		php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, stricterror);
 		RETURN_FALSE;
 	}
 

ext/dom/tests/modern/xml/Node_removeChild_from_comment.phpt

@@ -0,0 +1,20 @@
+--TEST--
+Removing a child from a comment should result in NOT_FOUND_ERR
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+$dom = Dom\XMLDocument::createFromString('<root><!-- comment --><child/></root>');
+$comment = $dom->documentElement->firstChild;
+$child = $comment->nextSibling;
+
+try {
+    $comment->removeChild($child);
+} catch (DOMException $e) {
+    echo $e->getMessage(), "\n";
+}
+
+?>
+--EXPECT--
+Not Found Error