From 99cfe62576e563234b928db9305290dfb3a96c03 Mon Sep 17 00:00:00 2001 From: Christian Bewernitz Date: Thu, 3 Nov 2022 08:53:15 +0100 Subject: [PATCH] fix: Properly check nodes before replacement (#457) which is slightly different from the checks required when inserting a node. Also fixes the missing `Document.ownerDocument`, which prevented proper deletion of childNodes from `Document`. Also fixes properly disconnecting removed children from it's previous parent. fixes #455 (only on the master branch for now, I want to test if this also takes care of #456) https://dom.spec.whatwg.org/#concept-node-ensure-pre-insertion-validity https://dom.spec.whatwg.org/#concept-node-replace --- CHANGELOG.md | 9 ++ lib/dom.js | 219 +++++++++++++++++++++++++++++++------- test/dom/document.test.js | 35 +++++- 3 files changed, 224 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f128b61c..ec23e9153 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,15 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.7.9](https://github.com/xmldom/xmldom/compare/0.7.8...0.7.9) + +### Fixed + +- Properly check nodes before replacement [`#457`](https://github.com/xmldom/xmldom/pull/457) / [`#455`](https://github.com/xmldom/xmldom/issues/455) / [`#456`](https://github.com/xmldom/xmldom/issues/456) + +Thank you, [@edemaine](https://github.com/edemaine), [@pedro-l9](https://github.com/pedro-l9), for your contributions + + ## [0.7.8](https://github.com/xmldom/xmldom/compare/0.7.7...0.7.8) ### Fixed diff --git a/lib/dom.js b/lib/dom.js index 6888e83ea..d3dfac00f 100644 --- a/lib/dom.js +++ b/lib/dom.js @@ -468,7 +468,7 @@ Node.prototype = { return _insertBefore(this,newChild,refChild); }, replaceChild:function(newChild, oldChild){//raises - this.insertBefore(newChild,oldChild); + _insertBefore(this, newChild,oldChild, assertPreReplacementValidityInDocument); if(oldChild){ this.removeChild(oldChild); } @@ -592,6 +592,7 @@ function _visitNode(node,callback){ function Document(){ + this.ownerDocument = this; } function _onAddAttribute(doc,el,newAttr){ @@ -628,6 +629,7 @@ function _onUpdateChild(doc,el,newChild){ child =child.nextSibling; } cs.length = i; + delete cs[cs.length]; } } } @@ -653,6 +655,9 @@ function _removeChild(parentNode,child){ }else{ parentNode.lastChild = previous; } + child.parentNode = null; + child.previousSibling = null; + child.nextSibling = null; _onUpdateChild(parentNode.ownerDocument,parentNode); return child; } @@ -730,8 +735,35 @@ function isElementInsertionPossible(doc, child) { var docTypeNode = find(parentChildNodes, isDocTypeNode); return !(child && docTypeNode && parentChildNodes.indexOf(docTypeNode) > parentChildNodes.indexOf(child)); } + /** + * Check if en element node can be inserted before `child`, or at the end if child is falsy, + * according to the presence and position of a doctype node on the same level. + * + * @param {Node} doc The document node + * @param {Node} child the node that would become the nextSibling if the element would be inserted + * @returns {boolean} `true` if an element can be inserted before child * @private + * https://dom.spec.whatwg.org/#concept-node-ensure-pre-insertion-validity + */ +function isElementReplacementPossible(doc, child) { + var parentChildNodes = doc.childNodes || []; + + function hasElementChildThatIsNotChild(node) { + return isElementNode(node) && node !== child; + } + + if (find(parentChildNodes, hasElementChildThatIsNotChild)) { + return false; + } + var docTypeNode = find(parentChildNodes, isDocTypeNode); + return !(child && docTypeNode && parentChildNodes.indexOf(docTypeNode) > parentChildNodes.indexOf(child)); +} + +/** + * @private + * Steps 1-5 of the checks before inserting and before replacing a child are the same. + * * @param {Node} parent the parent node to insert `node` into * @param {Node} node the node to insert * @param {Node=} child the node that should become the `nextSibling` of `node` @@ -739,18 +771,26 @@ function isElementInsertionPossible(doc, child) { * @throws DOMException for several node combinations that would create a DOM that is not well-formed. * @throws DOMException if `child` is provided but is not a child of `parent`. * @see https://dom.spec.whatwg.org/#concept-node-ensure-pre-insertion-validity + * @see https://dom.spec.whatwg.org/#concept-node-replace */ -function _insertBefore(parent, node, child) { +function assertPreInsertionValidity1to5(parent, node, child) { + // 1. If `parent` is not a Document, DocumentFragment, or Element node, then throw a "HierarchyRequestError" DOMException. if (!hasValidParentNodeType(parent)) { throw new DOMException(HIERARCHY_REQUEST_ERR, 'Unexpected parent node type ' + parent.nodeType); } + // 2. If `node` is a host-including inclusive ancestor of `parent`, then throw a "HierarchyRequestError" DOMException. + // not implemented! + // 3. If `child` is non-null and its parent is not `parent`, then throw a "NotFoundError" DOMException. if (child && child.parentNode !== parent) { throw new DOMException(NOT_FOUND_ERR, 'child not in parent'); } if ( + // 4. If `node` is not a DocumentFragment, DocumentType, Element, or CharacterData node, then throw a "HierarchyRequestError" DOMException. !hasInsertableNodeType(node) || + // 5. If either `node` is a Text node and `parent` is a document, // the sax parser currently adds top level text nodes, this will be fixed in 0.9.0 // || (node.nodeType === Node.TEXT_NODE && parent.nodeType === Node.DOCUMENT_NODE) + // or `node` is a doctype and `parent` is not a document, then throw a "HierarchyRequestError" DOMException. (isDocTypeNode(node) && parent.nodeType !== Node.DOCUMENT_NODE) ) { throw new DOMException( @@ -758,36 +798,137 @@ function _insertBefore(parent, node, child) { 'Unexpected node type ' + node.nodeType + ' for parent node type ' + parent.nodeType ); } +} + +/** + * @private + * Step 6 of the checks before inserting and before replacing a child are different. + * + * @param {Document} parent the parent node to insert `node` into + * @param {Node} node the node to insert + * @param {Node | undefined} child the node that should become the `nextSibling` of `node` + * @returns {Node} + * @throws DOMException for several node combinations that would create a DOM that is not well-formed. + * @throws DOMException if `child` is provided but is not a child of `parent`. + * @see https://dom.spec.whatwg.org/#concept-node-ensure-pre-insertion-validity + * @see https://dom.spec.whatwg.org/#concept-node-replace + */ +function assertPreInsertionValidityInDocument(parent, node, child) { var parentChildNodes = parent.childNodes || []; var nodeChildNodes = node.childNodes || []; - if (parent.nodeType === Node.DOCUMENT_NODE) { - if (node.nodeType === Node.DOCUMENT_FRAGMENT_NODE) { - var nodeChildElements = nodeChildNodes.filter(isElementNode); - if (nodeChildElements.length > 1 || find(nodeChildNodes, isTextNode)) { - throw new DOMException(HIERARCHY_REQUEST_ERR, 'More than one element or text in fragment'); - } - if (nodeChildElements.length === 1 && !isElementInsertionPossible(parent, child)) { - throw new DOMException(HIERARCHY_REQUEST_ERR, 'Element in fragment can not be inserted before doctype'); - } + + // DocumentFragment + if (node.nodeType === Node.DOCUMENT_FRAGMENT_NODE) { + var nodeChildElements = nodeChildNodes.filter(isElementNode); + // If node has more than one element child or has a Text node child. + if (nodeChildElements.length > 1 || find(nodeChildNodes, isTextNode)) { + throw new DOMException(HIERARCHY_REQUEST_ERR, 'More than one element or text in fragment'); } - if (isElementNode(node)) { - if (find(parentChildNodes, isElementNode) || !isElementInsertionPossible(parent, child)) { - throw new DOMException(HIERARCHY_REQUEST_ERR, 'Only one element can be added and only after doctype'); - } + // Otherwise, if `node` has one element child and either `parent` has an element child, + // `child` is a doctype, or `child` is non-null and a doctype is following `child`. + if (nodeChildElements.length === 1 && !isElementInsertionPossible(parent, child)) { + throw new DOMException(HIERARCHY_REQUEST_ERR, 'Element in fragment can not be inserted before doctype'); } - if (isDocTypeNode(node)) { - if (find(parentChildNodes, isDocTypeNode)) { - throw new DOMException(HIERARCHY_REQUEST_ERR, 'Only one doctype is allowed'); - } - var parentElementChild = find(parentChildNodes, isElementNode); - if (child && parentChildNodes.indexOf(parentElementChild) < parentChildNodes.indexOf(child)) { - throw new DOMException(HIERARCHY_REQUEST_ERR, 'Doctype can only be inserted before an element'); - } - if (!child && parentElementChild) { - throw new DOMException(HIERARCHY_REQUEST_ERR, 'Doctype can not be appended since element is present'); - } + } + // Element + if (isElementNode(node)) { + // `parent` has an element child, `child` is a doctype, + // or `child` is non-null and a doctype is following `child`. + if (!isElementInsertionPossible(parent, child)) { + throw new DOMException(HIERARCHY_REQUEST_ERR, 'Only one element can be added and only after doctype'); + } + } + // DocumentType + if (isDocTypeNode(node)) { + // `parent` has a doctype child, + if (find(parentChildNodes, isDocTypeNode)) { + throw new DOMException(HIERARCHY_REQUEST_ERR, 'Only one doctype is allowed'); + } + var parentElementChild = find(parentChildNodes, isElementNode); + // `child` is non-null and an element is preceding `child`, + if (child && parentChildNodes.indexOf(parentElementChild) < parentChildNodes.indexOf(child)) { + throw new DOMException(HIERARCHY_REQUEST_ERR, 'Doctype can only be inserted before an element'); + } + // or `child` is null and `parent` has an element child. + if (!child && parentElementChild) { + throw new DOMException(HIERARCHY_REQUEST_ERR, 'Doctype can not be appended since element is present'); + } + } +} + +/** + * @private + * Step 6 of the checks before inserting and before replacing a child are different. + * + * @param {Document} parent the parent node to insert `node` into + * @param {Node} node the node to insert + * @param {Node | undefined} child the node that should become the `nextSibling` of `node` + * @returns {Node} + * @throws DOMException for several node combinations that would create a DOM that is not well-formed. + * @throws DOMException if `child` is provided but is not a child of `parent`. + * @see https://dom.spec.whatwg.org/#concept-node-ensure-pre-insertion-validity + * @see https://dom.spec.whatwg.org/#concept-node-replace + */ +function assertPreReplacementValidityInDocument(parent, node, child) { + var parentChildNodes = parent.childNodes || []; + var nodeChildNodes = node.childNodes || []; + + // DocumentFragment + if (node.nodeType === Node.DOCUMENT_FRAGMENT_NODE) { + var nodeChildElements = nodeChildNodes.filter(isElementNode); + // If `node` has more than one element child or has a Text node child. + if (nodeChildElements.length > 1 || find(nodeChildNodes, isTextNode)) { + throw new DOMException(HIERARCHY_REQUEST_ERR, 'More than one element or text in fragment'); + } + // Otherwise, if `node` has one element child and either `parent` has an element child that is not `child` or a doctype is following `child`. + if (nodeChildElements.length === 1 && !isElementReplacementPossible(parent, child)) { + throw new DOMException(HIERARCHY_REQUEST_ERR, 'Element in fragment can not be inserted before doctype'); + } + } + // Element + if (isElementNode(node)) { + // `parent` has an element child that is not `child` or a doctype is following `child`. + if (!isElementReplacementPossible(parent, child)) { + throw new DOMException(HIERARCHY_REQUEST_ERR, 'Only one element can be added and only after doctype'); + } + } + // DocumentType + if (isDocTypeNode(node)) { + function hasDoctypeChildThatIsNotChild(node) { + return isDocTypeNode(node) && node !== child; + } + + // `parent` has a doctype child that is not `child`, + if (find(parentChildNodes, hasDoctypeChildThatIsNotChild)) { + throw new DOMException(HIERARCHY_REQUEST_ERR, 'Only one doctype is allowed'); + } + var parentElementChild = find(parentChildNodes, isElementNode); + // or an element is preceding `child`. + if (child && parentChildNodes.indexOf(parentElementChild) < parentChildNodes.indexOf(child)) { + throw new DOMException(HIERARCHY_REQUEST_ERR, 'Doctype can only be inserted before an element'); } } +} + +/** + * @private + * @param {Node} parent the parent node to insert `node` into + * @param {Node} node the node to insert + * @param {Node=} child the node that should become the `nextSibling` of `node` + * @returns {Node} + * @throws DOMException for several node combinations that would create a DOM that is not well-formed. + * @throws DOMException if `child` is provided but is not a child of `parent`. + * @see https://dom.spec.whatwg.org/#concept-node-ensure-pre-insertion-validity + */ +function _insertBefore(parent, node, child, _inDocumentAssertion) { + // To ensure pre-insertion validity of a node into a parent before a child, run these steps: + assertPreInsertionValidity1to5(parent, node, child); + + // If parent is a document, and any of the statements below, switched on the interface node implements, + // are true, then throw a "HierarchyRequestError" DOMException. + if (parent.nodeType === Node.DOCUMENT_NODE) { + (_inDocumentAssertion || assertPreInsertionValidityInDocument)(parent, node, child); + } var cp = node.parentNode; if(cp){ @@ -829,25 +970,20 @@ function _insertBefore(parent, node, child) { return node; } function _appendSingleChild(parentNode,newChild){ - var cp = newChild.parentNode; - if(cp){ - var pre = parentNode.lastChild; - cp.removeChild(newChild);//remove and update - var pre = parentNode.lastChild; + if (newChild.parentNode) { + newChild.parentNode.removeChild(newChild); } - var pre = parentNode.lastChild; newChild.parentNode = parentNode; - newChild.previousSibling = pre; + newChild.previousSibling = parentNode.lastChild; newChild.nextSibling = null; - if(pre){ - pre.nextSibling = newChild; + if (newChild.previousSibling) { + newChild.previousSibling.nextSibling = newChild; }else{ parentNode.firstChild = newChild; } parentNode.lastChild = newChild; _onUpdateChild(parentNode.ownerDocument,parentNode,newChild); return newChild; - //console.log("__aa",parentNode.lastChild.nextSibling == null) } Document.prototype = { @@ -888,6 +1024,17 @@ Document.prototype = { } return _removeChild(this,oldChild); }, + replaceChild: function (newChild, oldChild) { + //raises + _insertBefore(this, newChild, oldChild, assertPreReplacementValidityInDocument); + newChild.ownerDocument = this; + if (oldChild) { + this.removeChild(oldChild); + } + if (isElementNode(newChild)) { + this.documentElement = newChild; + } + }, // Introduced in DOM Level 2: importNode : function(importedNode,deep){ return importNode(this,importedNode,deep); diff --git a/test/dom/document.test.js b/test/dom/document.test.js index efa132109..89043de8b 100644 --- a/test/dom/document.test.js +++ b/test/dom/document.test.js @@ -142,9 +142,38 @@ describe('Document.prototype', () => { expect(() => doc.insertBefore(root, withoutParent)).toThrow(DOMException); - expect(doc.documentElement).toBeNull(); - expect(doc.childNodes).toHaveLength(0); - expect(root.parentNode).toBeNull(); + expect(doc.documentElement).toBeNull() + expect(doc.childNodes).toHaveLength(0) + expect(root.parentNode).toBeNull() + }) + }) + describe('replaceChild', () => { + it('should remove the only element and add the new one', () => { + const doc = new DOMImplementation().createDocument('', 'xml'); + const initialFirstChild = doc.firstChild; + const replacement = doc.createElement('replaced'); + + doc.replaceChild(replacement, doc.firstChild); + + expect(doc.childNodes).toHaveLength(1); + expect(initialFirstChild.parentNode).toBeNull(); + expect(doc.documentElement.name).toBe(replacement.name); + }); + }); + describe('removeChild', () => { + it('should remove all connections to node', () => { + const doc = new DOMImplementation().createDocument('', 'xml'); + doc.insertBefore(doc.createComment('just a comment'), doc.firstChild); + expect(doc.childNodes).toHaveLength(2); + const initialElement = doc.firstChild; + + doc.removeChild(initialElement); + + // expect(doc.documentElement).toBeNull(); + expect(initialElement.parentNode).toBeNull(); + expect(initialElement.nextSibling).toBeNull(); + // expect(initialElement.previousSibling).toBeNull(); + expect(doc.childNodes).toHaveLength(1); }); }); })