Merge pull request capricorn86#829 from btea/task/827-insertBefore-re…

…ferenceNode capricorn86#827@patch: Distinguish between referenceNode values that are undefin…
lukasbach · Apr 21, 2023 · 1a827e3 · 1a827e3
2 parents a3ae49d + 8e67309
commit 1a827e3
Show file tree

Hide file tree

Showing 7 changed files with 64 additions and 33 deletions.
diff --git a/packages/happy-dom/src/nodes/document-fragment/DocumentFragment.ts b/packages/happy-dom/src/nodes/document-fragment/DocumentFragment.ts
@@ -171,6 +171,12 @@ export default class DocumentFragment extends Node implements IDocumentFragment
 	 * @override
 	 */
 	public override insertBefore(newNode: INode, referenceNode: INode | null): INode {
+		if (arguments.length < 2) {
+			throw new TypeError(
+				`Failed to execute 'insertBefore' on 'Node': 2 arguments required, but only ${arguments.length} present.`
+			);
+		}
+
 		// We do not call super here as this will be handled by ElementUtility to improve performance by avoiding validation and other checks.
 		return ElementUtility.insertBefore(this, newNode, referenceNode);
 	}

diff --git a/packages/happy-dom/src/nodes/document/Document.ts b/packages/happy-dom/src/nodes/document/Document.ts
@@ -637,6 +637,12 @@ export default class Document extends Node implements IDocument {
 	 * @override
 	 */
 	public override insertBefore(newNode: INode, referenceNode: INode | null): INode {
+		if (arguments.length < 2) {
+			throw new TypeError(
+				`Failed to execute 'insertBefore' on 'Node': 2 arguments required, but only ${arguments.length} present.`
+			);
+		}
+
 		// We do not call super here as this will be handled by ElementUtility to improve performance by avoiding validation and other checks.
 		return ElementUtility.insertBefore(this, newNode, referenceNode);
 	}

diff --git a/packages/happy-dom/src/nodes/element/Element.ts b/packages/happy-dom/src/nodes/element/Element.ts
@@ -386,6 +386,12 @@ export default class Element extends Node implements IElement {
 	 * @override
 	 */
 	public override insertBefore(newNode: INode, referenceNode: INode | null): INode {
+		if (arguments.length < 2) {
+			throw new TypeError(
+				`Failed to execute 'insertBefore' on 'Node': 2 arguments required, but only ${arguments.length} present.`
+			);
+		}
+
 		// We do not call super here as this will be handled by ElementUtility to improve performance by avoiding validation and other checks.
 		return ElementUtility.insertBefore(this, newNode, referenceNode);
 	}

diff --git a/packages/happy-dom/src/nodes/element/ElementUtility.ts b/packages/happy-dom/src/nodes/element/ElementUtility.ts
@@ -123,7 +123,8 @@ export default class ElementUtility {
 		referenceNode: INode | null,
 		options?: { disableAncestorValidation?: boolean }
 	): INode {
-		if (newNode.nodeType === NodeTypeEnum.elementNode) {
+		// NodeUtility.insertBefore() will call appendChild() for the scenario where "referenceNode" is "null" or "undefined"
+		if (newNode.nodeType === NodeTypeEnum.elementNode && referenceNode) {
 			if (
 				!options?.disableAncestorValidation &&
 				NodeUtility.isInclusiveAncestor(newNode, ancestorNode)
@@ -152,36 +153,30 @@ export default class ElementUtility {
 				}
 			}
 
-			// Node.ts will call appendChild() for the scenario where "referenceNode" is "null"
+			if (referenceNode.nodeType === NodeTypeEnum.elementNode) {
+				const index = ancestorNode.children.indexOf(<IElement>referenceNode);
+				if (index !== -1) {
+					ancestorNode.children.splice(index, 0, <IElement>newNode);
+				}
+			} else {
+				ancestorNode.children.length = 0;
 
-			if (referenceNode) {
-				if (referenceNode.nodeType === NodeTypeEnum.elementNode) {
-					const index = ancestorNode.children.indexOf(<IElement>referenceNode);
-					if (index !== -1) {
-						ancestorNode.children.splice(index, 0, <IElement>newNode);
+				for (const node of ancestorNode.childNodes) {
+					if (node === referenceNode) {
+						ancestorNode.children.push(<IElement>newNode);
 					}
-				} else {
-					ancestorNode.children.length = 0;
-
-					for (const node of ancestorNode.childNodes) {
-						if (node === referenceNode) {
-							ancestorNode.children.push(<IElement>newNode);
-						}
-						if (node.nodeType === NodeTypeEnum.elementNode) {
-							ancestorNode.children.push(<IElement>node);
-						}
+					if (node.nodeType === NodeTypeEnum.elementNode) {
+						ancestorNode.children.push(<IElement>node);
 					}
 				}
 			}
 
-			if (referenceNode || referenceNode === null) {
-				for (const attribute of NAMED_ITEM_ATTRIBUTES) {
-					if ((<Element>newNode)._attributes[attribute]) {
-						(<HTMLCollection<IHTMLElement>>ancestorNode.children)._appendNamedItem(
-							<IHTMLElement>newNode,
-							(<Element>newNode)._attributes[attribute].value
-						);
-					}
+			for (const attribute of NAMED_ITEM_ATTRIBUTES) {
+				if ((<Element>newNode)._attributes[attribute]) {
+					(<HTMLCollection<IHTMLElement>>ancestorNode.children)._appendNamedItem(
+						<IHTMLElement>newNode,
+						(<Element>newNode)._attributes[attribute].value
+					);
 				}
 			}
 

diff --git a/packages/happy-dom/src/nodes/node/Node.ts b/packages/happy-dom/src/nodes/node/Node.ts
@@ -322,6 +322,11 @@ export default class Node extends EventTarget implements INode {
 	 * @returns Inserted node.
 	 */
 	public insertBefore(newNode: INode, referenceNode: INode | null): INode {
+		if (arguments.length < 2) {
+			throw new TypeError(
+				`Failed to execute 'insertBefore' on 'Node': 2 arguments required, but only ${arguments.length} present.`
+			);
+		}
 		return NodeUtility.insertBefore(this, newNode, referenceNode);
 	}
 

diff --git a/packages/happy-dom/src/nodes/node/NodeUtility.ts b/packages/happy-dom/src/nodes/node/NodeUtility.ts
@@ -164,18 +164,13 @@ export default class NodeUtility {
 			return newNode;
 		}
 
-		if (referenceNode === null) {
+		// If the referenceNode is null or undefined, then the newNode should be appended to the ancestorNode.
+		// According to spec only null is valid, but browsers support undefined as well.
+		if (!referenceNode) {
 			ancestorNode.appendChild(newNode);
 			return newNode;
 		}
 
-		if (!referenceNode) {
-			throw new DOMException(
-				"Failed to execute 'insertBefore' on 'Node': 2 arguments required, but only 1 present.",
-				'TypeError'
-			);
-		}
-
 		if (ancestorNode.childNodes.indexOf(referenceNode) === -1) {
 			throw new DOMException(
 				"Failed to execute 'insertBefore' on 'Node': The node before which the new node is to be inserted is not a child of this node."

diff --git a/packages/happy-dom/test/nodes/node/Node.test.ts b/packages/happy-dom/test/nodes/node/Node.test.ts
@@ -547,6 +547,24 @@ describe('Node', () => {
 			);
 		});
 
+		it('If reference node is null or undefined, the newNode should be inserted at the end of the peer node.', () => {
+			const child1 = document.createElement('span');
+			const child2 = document.createElement('span');
+			const newNode = document.createElement('span');
+			const newNode1 = document.createElement('span');
+			const parent = document.createElement('div');
+
+			parent.appendChild(child1);
+			parent.appendChild(child2);
+			parent.insertBefore(newNode, null);
+			parent.insertBefore(newNode1, undefined);
+
+			expect(parent.childNodes[0]).toBe(child1);
+			expect(parent.childNodes[1]).toBe(child2);
+			expect(parent.childNodes[2]).toBe(newNode);
+			expect(parent.childNodes[3]).toBe(newNode1);
+		});
+
 		it('Throws an exception if reference node is not child of parent node.', () => {
 			const referenceNode = document.createElement('span');
 			const newNode = document.createElement('span');