Address review comments

Author: nielsdos

ext/dom/php_dom.c

@@ -750,8 +750,9 @@ PHP_MINIT_FUNCTION(dom)
 	memcpy(&dom_token_list_object_handlers, &dom_object_handlers, sizeof(zend_object_handlers));
 	dom_token_list_object_handlers.offset = XtOffsetOf(dom_token_list_object, dom.std);
 	dom_token_list_object_handlers.free_obj = dom_token_list_free_obj;
-	/* The IDL has the [SameObject] constraint, which is incompatible with cloning because it imposes that there is only
-	 * one instance per parent object. */
+	/* The Web IDL (Web Interface Description Language - https://webidl.spec.whatwg.org) has the [SameObject] constraint
+	 * for this object, which is incompatible with cloning because it imposes that there is only one instance
+	 * per parent object. */
 	dom_token_list_object_handlers.clone_obj = NULL;
 	dom_token_list_object_handlers.read_dimension = dom_token_list_read_dimension;
 	dom_token_list_object_handlers.has_dimension = dom_token_list_has_dimension;

ext/dom/tests/modern/token_list/dimensions_error.phpt

@@ -8,22 +8,30 @@ dom
 $dom = DOM\XMLDocument::createFromString('<root class="A B C"/>');
 $list = $dom->documentElement->classList;
 
-try {
-    $list[new stdClass];
-} catch (TypeError $e) {
-    echo $e->getMessage(), "\n";
-}
+$testOffsets = [
+    new stdClass,
+    [],
+    fopen("php://output", "w"),
+];
 
-try {
-    isset($list[new stdClass]);
-} catch (TypeError $e) {
-    echo $e->getMessage(), "\n";
-}
+foreach ($testOffsets as $offset) {
+    try {
+        $list[$offset];
+    } catch (TypeError $e) {
+        echo $e->getMessage(), "\n";
+    }
 
-try {
-    empty($list[new stdClass]);
-} catch (TypeError $e) {
-    echo $e->getMessage(), "\n";
+    try {
+        isset($list[$offset]);
+    } catch (TypeError $e) {
+        echo $e->getMessage(), "\n";
+    }
+
+    try {
+        empty($list[$offset]);
+    } catch (TypeError $e) {
+        echo $e->getMessage(), "\n";
+    }
 }
 
 try {
@@ -33,8 +41,17 @@ try {
 }
 
 ?>
---EXPECT--
+--EXPECTF--
 Cannot access offset of type stdClass on Dom\TokenList
 Cannot access offset of type stdClass in isset or empty
 Cannot access offset of type stdClass in isset or empty
+Cannot access offset of type array on Dom\TokenList
+Cannot access offset of type array in isset or empty
+Cannot access offset of type array in isset or empty
+
+Warning: Resource ID#%d used as offset, casting to integer (%d) in %s on line %d
+
+Warning: Resource ID#%d used as offset, casting to integer (%d) in %s on line %d
+
+Warning: Resource ID#%d used as offset, casting to integer (%d) in %s on line %d
 Cannot append to Dom\TokenList

ext/dom/tests/modern/token_list/toggle.phpt

@@ -45,6 +45,8 @@ var_dump($list->toggle("C"));
 
 echo $dom->saveXML(), "\n";
 
+echo "--- Toggle E ---\n";
+
 $list->value = 'E';
 $list->toggle('E');
 
@@ -76,5 +78,6 @@ bool(true)
 bool(false)
 <?xml version="1.0" encoding="UTF-8"?>
 <root class="D"/>
+--- Toggle E ---
 <?xml version="1.0" encoding="UTF-8"?>
 <root class=""/>

ext/dom/token_list.c

@@ -257,7 +257,20 @@ static void dom_token_list_item_read(dom_token_list_object *token_list, zval *re
 		ZVAL_STR_COPY(retval, str_index);
 	} else {
 		/* Not an out of bounds ValueError, but NULL, as according to spec.
-		 * This was a design choice to allow for constructs like `item(x) ?? ...` */
+		 * This design choice allows for constructs like `item(x) ?? ...`
+		 *
+		 * In particular:
+		 * https://dom.spec.whatwg.org/#interface-domtokenlist states DOMTokenList implements iterable<DOMString>.
+		 * From https://webidl.spec.whatwg.org/#idl-iterable:
+		 *   If a single type parameter is given,
+		 *   then the interface has a value iterator and provides values of the specified type.
+		 * This applies, and reading the definition of value iterator means we should support indexed properties.
+		 * From https://webidl.spec.whatwg.org/#dfn-support-indexed-properties:
+		 *   An interface that defines an indexed property getter is said to support indexed properties.
+		 * And indexed property getter is defined here: https://webidl.spec.whatwg.org/#dfn-indexed-property-getter
+		 * Down below in their note they give an example of how an out-of-bounds access evaluates to undefined,
+		 * which would map to NULL for us.
+		 * This would also be consistent with how out-of-bounds array accesses in PHP result in NULL. */
 		ZVAL_NULL(retval);
 	}
 }