Fixed template element referencing, appending attribute nodes removed

lib/DOM/DOMException.php

@@ -8,6 +8,7 @@ namespace MensBeam\HTML;
 
 class DOMException extends \Exception {
     // From PHP's DOMException; keeping error codes consistent
+    const HIERARCHY_REQUEST_ERROR = 3;
     const WRONG_DOCUMENT = 4;
     const INVALID_CHARACTER = 5;
     const NO_MODIFICATION_ALLOWED = 7;
@@ -18,6 +19,7 @@ class DOMException extends \Exception {
     const OUTER_HTML_FAILED_NOPARENT = 102;
 
     protected static $messages = [
+          3 => 'Hierarchy request error; supplied node is not allowed here',
           4 => 'Supplied node does not belong to this document',
           5 => 'Invalid character',
           7 => 'Modification not allowed here',

lib/DOM/Document.php

@@ -19,12 +19,6 @@ class Document extends \DOMDocument {
     public $mangledElements = false;
     public $quirksMode = self::NO_QUIRKS_MODE;
 
-    // An array of all template elements created in the document
-    // This exists because values of properties on derived DOM classes
-    //   are lost unless at least one PHP reference is kept for the
-    //   element somewhere in userspace. This is that somewhere.
-    protected $templateElements = [];
-
     public function __construct() {
         parent::__construct();
 
@@ -35,6 +29,21 @@ class Document extends \DOMDocument {
         $this->registerNodeClass('DOMText', '\MensBeam\HTML\Text');
     }
 
+    public function appendChild($node) {
+        # If node is not a DocumentFragment, DocumentType, Element, Text,
+        # ProcessingInstruction, or Comment node then throw a "HierarchyRequestError"
+        # DOMException.
+        if (!$node instanceof DocumentFragment && !$node instanceof \DOMDocumentType && !$node instanceof Element &&!$node instanceof Text && !$node instanceof ProcessingInstruction && !$node instanceof Comment) {
+            throw new DOMException(DOMException::HIERARCHY_REQUEST_ERROR);
+        }
+
+        $result = parent::appendChild($node);
+        if ($result !== false && $result instanceof TemplateElement) {
+            ElementRegistry::set($result);
+        }
+        return $result;
+    }
+
     public function createAttribute($name) {
         return $this->createAttributeNS(null, $name);
     }
@@ -75,7 +84,8 @@ class Document extends \DOMDocument {
                 $e = parent::createElementNS($namespaceURI, $qualifiedName, $value);
             } else {
                 $e = new TemplateElement($this, $qualifiedName, $value);
-                $this->templateElements[] = $e;
+                // Template elements need to have a reference kept in userland
+                ElementRegistry::set($e);
                 $e->content = $this->createDocumentFragment();
             }
 
@@ -98,6 +108,26 @@ class Document extends \DOMDocument {
         return false;
     }
 
+    public function insertBefore($node, $child = null) {
+        # If node is not a DocumentFragment, DocumentType, Element, Text,
+        # ProcessingInstruction, or Comment node then throw a "HierarchyRequestError"
+        # DOMException.
+        if (!$node instanceof DocumentFragment && !$node instanceof \DOMDocumentType && !$node instanceof Element &&!$node instanceof Text && !$node instanceof ProcessingInstruction && !$node instanceof Comment) {
+            throw new DOMException(DOMException::HIERARCHY_REQUEST_ERROR);
+        }
+
+        $result = parent::insertBefore($node, $child);
+        if ($result !== false) {
+            if ($result instanceof TemplateElement) {
+                ElementRegistry::set($result);
+            }
+            if ($child instanceof TemplateElement) {
+                ElementRegistry::delete($child);
+            }
+        }
+        return $result;
+    }
+
     public function load($filename, $options = null, ?string $encodingOrContentType = null): bool {
         $data = Parser::fetchFile($filename, $encodingOrContentType);
         if (!$data) {
@@ -122,6 +152,27 @@ class Document extends \DOMDocument {
         return false;
     }
 
+    public function removeChild($child) {
+        $result = parent::removeChild($child);
+        if ($result !== false && $result instanceof TemplateElement) {
+            ElementRegistry::delete($child);
+        }
+        return $result;
+    }
+
+    public function replaceChild($node, $child) {
+        $result = parent::replaceChild($node, $child);
+        if ($result !== false) {
+            if ($result instanceof TemplateElement) {
+                ElementRegistry::set($child);
+            }
+            if ($child instanceof TemplateElement) {
+                ElementRegistry::delete($child);
+            }
+        }
+        return $result;
+    }
+
     public function save($filename, $options = null) {
         return file_put_contents($filename, $this->serialize());
     }

lib/DOM/Element.php

@@ -12,32 +12,18 @@ class Element extends \DOMElement {
     protected $_classList;
 
     public function appendChild($node) {
-        $fixID = false;
-        if ($node instanceof \DOMAttr && $node->namespaceURI === null) {
-            if ($node->name === 'id') {
-                $fixID = true;
-            }
-            // If appending a class attribute node, and classList has been invoked set
-            // the class using classList instead of appending the attribute node. Will
-            // return the created node instead. TokenList appends an attribute node
-            // internally to set the class attribute, so to prevent an infinite call loop
-            // from occurring, a check between the normalized value and classList's
-            // serialized value is performed. The spec is vague on how this is supposed to
-            // be handled.
-            elseif ($this->_classList !== null && $node->name === 'class' && preg_replace(Data::WHITESPACE_REGEX, ' ', $node->value) !== $this->_classList->value) {
-                $this->_classList->value = $node->value;
-                return $this->getAttributeNode('class');
-            }
+        # If node is not a DocumentFragment, DocumentType, Element, Text,
+        # ProcessingInstruction, or Comment node then throw a "HierarchyRequestError"
+        # DOMException.
+        if (!$node instanceof DocumentFragment && !$node instanceof \DOMDocumentType && !$node instanceof Element &&!$node instanceof Text && !$node instanceof ProcessingInstruction && !$node instanceof Comment) {
+            throw new DOMException(DOMException::HIERARCHY_REQUEST_ERROR);
         }
 
-        $node = parent::appendChild($node);
-
-        // Fix id attributes when appending id attribute nodes.
-        if ($fixID) {
-            $this->setIdAttribute('id', true);
+        $result = parent::appendChild($node);
+        if ($result !== false && $result instanceof TemplateElement) {
+            ElementRegistry::set($result);
         }
-
-        return $node;
+        return $result;
     }
 
     public function getAttribute($name) {
@@ -62,6 +48,47 @@ class Element extends \DOMElement {
         return $value;
     }
 
+    public function insertBefore($node, $child = null) {
+        # If node is not a DocumentFragment, DocumentType, Element, Text,
+        # ProcessingInstruction, or Comment node then throw a "HierarchyRequestError"
+        # DOMException.
+        if (!$node instanceof DocumentFragment && !$node instanceof \DOMDocumentType && !$node instanceof Element &&!$node instanceof Text && !$node instanceof ProcessingInstruction && !$node instanceof Comment) {
+            throw new DOMException(DOMException::HIERARCHY_REQUEST_ERROR);
+        }
+
+        $result = parent::insertBefore($node, $child);
+        if ($result !== false) {
+            if ($result instanceof TemplateElement) {
+                ElementRegistry::set($result);
+            }
+            if ($child instanceof TemplateElement) {
+                ElementRegistry::delete($child);
+            }
+        }
+        return $result;
+    }
+
+    public function removeChild($child) {
+        $result = parent::removeChild($child);
+        if ($result !== false && $result instanceof TemplateElement) {
+            ElementRegistry::delete($child);
+        }
+        return $result;
+    }
+
+    public function replaceChild($node, $child) {
+        $result = parent::replaceChild($node, $child);
+        if ($result !== false) {
+            if ($result instanceof TemplateElement) {
+                ElementRegistry::set($child);
+            }
+            if ($child instanceof TemplateElement) {
+                ElementRegistry::delete($child);
+            }
+        }
+        return $result;
+    }
+
     public function setAttribute($name, $value) {
         $this->setAttributeNS(null, $name, $value);
     }
@@ -110,17 +137,35 @@ class Element extends \DOMElement {
     }
 
     public function setAttributeNode(\DOMAttr $attribute) {
-        parent::setAttributeNode($attribute);
-        if ($attribute->name === 'id') {
-            $this->setIdAttribute($attribute->name, true);
-        }
+        return setAttributeNodeNS($attribute, null);
     }
 
     public function setAttributeNodeNS(\DOMAttr $attribute) {
-        parent::setAttributeNodeNS($attribute);
-        if ($attribute->name === 'id' && $attribute->namespaceURI === null) {
+        $fixId = false;
+        if ($attribute->namespaceURI === null) {
+            if ($attribute->name === 'id') {
+                $fixId = true;
+            }
+            // If appending a class attribute node, and classList has been invoked set
+            // the class using classList instead of appending the attribute node. Will
+            // return the created node instead. TokenList appends an attribute node
+            // internally to set the class attribute, so to prevent an infinite call loop
+            // from occurring, a check between the normalized value and classList's
+            // serialized value is performed. The spec is vague on how this is supposed to
+            // be handled.
+            elseif ($this->_classList !== null && $node->name === 'class' && preg_replace(Data::WHITESPACE_REGEX, ' ', $node->value) !== $this->_classList->value) {
+                $this->_classList->value = $node->value;
+                return $this->getAttributeNode('class');
+            }
+        }
+
+        $result = parent::setAttributeNodeNS($attribute);
+
+        if ($fixId) {
             $this->setIdAttribute($attribute->name, true);
         }
+
+        return $result;
     }
 
     public function __get(string $prop) {

lib/DOM/ElementRegistry.php

@@ -0,0 +1,42 @@
+<?php
+/** @license MIT
+ * Copyright 2017 , Dustin Wilson, J. King et al.
+ * See LICENSE and AUTHORS files for details */
+
+declare(strict_types=1);
+namespace MensBeam\HTML;
+
+// This is a write-only map of elements which need to be kept in memory; it
+// exists because values of properties on derived DOM classes are lost unless at
+// least one PHP reference is kept for the element somewhere in userspace. This
+// is that somewhere. It is at present only used for TemplateElements.
+class ElementRegistry {
+    public static $_storage = [];
+
+    public static function delete(Element $element) {
+        foreach (self::$_storage as $k => $v) {
+            if ($v->isSameNode($element)) {
+                unset(self::$_storage[$k]);
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    public static function has(Element $element) {
+        foreach (self::$_storage as $v) {
+            if ($v->isSameNode($element)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    public static function set(Element $element) {
+        if (!self::has($element)) {
+            self::$_storage[] = $element;
+        }
+    }
+}

lib/DOM/TokenList.php

@@ -287,7 +287,7 @@ class TokenList implements \ArrayAccess, \Countable, \Iterator {
         $element = $this->element->get();
         $class = $element->ownerDocument->createAttribute($this->localName);
         $class->value = $this->__toString();
-        $element->appendChild($class);
+        $element->setAttributeNode($class);
     }
 
     public function __get(string $prop) {