Skip to content

Commit b6ad6e9

Browse files
karfaukarfau
committed
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`. 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
1 parent d7cfa2b commit b6ad6e9

File tree

3 files changed

+212
-26
lines changed

3 files changed

+212
-26
lines changed

‎CHANGELOG.md

+9
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,15 @@ All notable changes to this project will be documented in this file.
44

55
This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
66

7+
## [0.8.6](https://github.com/xmldom/xmldom/compare/0.8.5...0.8.6)
8+
9+
### Fixed
10+
11+
- 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)
12+
13+
Thank you, [@edemaine](https://github.com/edemaine), [@pedro-l9](https://github.com/pedro-l9), for your contributions
14+
15+
716
## [0.8.5](https://github.com/xmldom/xmldom/compare/0.8.4...0.8.5)
817

918
### Fixed

‎lib/dom.js

+174-26
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,7 @@ Node.prototype = {
468468
return _insertBefore(this,newChild,refChild);
469469
},
470470
replaceChild:function(newChild, oldChild){//raises
471-
this.insertBefore(newChild,oldChild);
471+
_insertBefore(this, newChild,oldChild, assertPreReplacementValidityInDocument);
472472
if(oldChild){
473473
this.removeChild(oldChild);
474474
}
@@ -590,6 +590,7 @@ function _visitNode(node,callback){
590590

591591

592592
function Document(){
593+
this.ownerDocument = this;
593594
}
594595

595596
function _onAddAttribute(doc,el,newAttr){
@@ -747,64 +748,200 @@ function isElementInsertionPossible(doc, child) {
747748
var docTypeNode = find(parentChildNodes, isDocTypeNode);
748749
return !(child && docTypeNode && parentChildNodes.indexOf(docTypeNode) > parentChildNodes.indexOf(child));
749750
}
751+
750752
/**
753+
* Check if en element node can be inserted before `child`, or at the end if child is falsy,
754+
* according to the presence and position of a doctype node on the same level.
755+
*
756+
* @param {Node} doc The document node
757+
* @param {Node} child the node that would become the nextSibling if the element would be inserted
758+
* @returns {boolean} `true` if an element can be inserted before child
751759
* @private
760+
* https://dom.spec.whatwg.org/#concept-node-ensure-pre-insertion-validity
761+
*/
762+
function isElementReplacementPossible(doc, child) {
763+
var parentChildNodes = doc.childNodes || [];
764+
765+
function hasElementChildThatIsNotChild(node) {
766+
return isElementNode(node) && node !== child;
767+
}
768+
769+
if (find(parentChildNodes, hasElementChildThatIsNotChild)) {
770+
return false;
771+
}
772+
var docTypeNode = find(parentChildNodes, isDocTypeNode);
773+
return !(child && docTypeNode && parentChildNodes.indexOf(docTypeNode) > parentChildNodes.indexOf(child));
774+
}
775+
776+
/**
777+
* @private
778+
* Steps 1-5 of the checks before inserting and before replacing a child are the same.
779+
*
752780
* @param {Node} parent the parent node to insert `node` into
753781
* @param {Node} node the node to insert
754782
* @param {Node=} child the node that should become the `nextSibling` of `node`
755783
* @returns {Node}
756784
* @throws DOMException for several node combinations that would create a DOM that is not well-formed.
757785
* @throws DOMException if `child` is provided but is not a child of `parent`.
758786
* @see https://dom.spec.whatwg.org/#concept-node-ensure-pre-insertion-validity
787+
* @see https://dom.spec.whatwg.org/#concept-node-replace
759788
*/
760-
function _insertBefore(parent, node, child) {
789+
function assertPreInsertionValidity1to5(parent, node, child) {
790+
// 1. If `parent` is not a Document, DocumentFragment, or Element node, then throw a "HierarchyRequestError" DOMException.
761791
if (!hasValidParentNodeType(parent)) {
762792
throw new DOMException(HIERARCHY_REQUEST_ERR, 'Unexpected parent node type ' + parent.nodeType);
763793
}
794+
// 2. If `node` is a host-including inclusive ancestor of `parent`, then throw a "HierarchyRequestError" DOMException.
795+
// not implemented!
796+
// 3. If `child` is non-null and its parent is not `parent`, then throw a "NotFoundError" DOMException.
764797
if (child && child.parentNode !== parent) {
765798
throw new DOMException(NOT_FOUND_ERR, 'child not in parent');
766799
}
767800
if (
801+
// 4. If `node` is not a DocumentFragment, DocumentType, Element, or CharacterData node, then throw a "HierarchyRequestError" DOMException.
768802
!hasInsertableNodeType(node) ||
803+
// 5. If either `node` is a Text node and `parent` is a document,
769804
// the sax parser currently adds top level text nodes, this will be fixed in 0.9.0
770805
// || (node.nodeType === Node.TEXT_NODE && parent.nodeType === Node.DOCUMENT_NODE)
806+
// or `node` is a doctype and `parent` is not a document, then throw a "HierarchyRequestError" DOMException.
771807
(isDocTypeNode(node) && parent.nodeType !== Node.DOCUMENT_NODE)
772808
) {
773809
throw new DOMException(
774810
HIERARCHY_REQUEST_ERR,
775811
'Unexpected node type ' + node.nodeType + ' for parent node type ' + parent.nodeType
776812
);
777813
}
814+
}
815+
816+
/**
817+
* @private
818+
* Step 6 of the checks before inserting and before replacing a child are different.
819+
*
820+
* @param {Document} parent the parent node to insert `node` into
821+
* @param {Node} node the node to insert
822+
* @param {Node | undefined} child the node that should become the `nextSibling` of `node`
823+
* @returns {Node}
824+
* @throws DOMException for several node combinations that would create a DOM that is not well-formed.
825+
* @throws DOMException if `child` is provided but is not a child of `parent`.
826+
* @see https://dom.spec.whatwg.org/#concept-node-ensure-pre-insertion-validity
827+
* @see https://dom.spec.whatwg.org/#concept-node-replace
828+
*/
829+
function assertPreInsertionValidityInDocument(parent, node, child) {
778830
var parentChildNodes = parent.childNodes || [];
779831
var nodeChildNodes = node.childNodes || [];
780-
if (parent.nodeType === Node.DOCUMENT_NODE) {
781-
if (node.nodeType === Node.DOCUMENT_FRAGMENT_NODE) {
782-
var nodeChildElements = nodeChildNodes.filter(isElementNode);
783-
if (nodeChildElements.length > 1 || find(nodeChildNodes, isTextNode)) {
784-
throw new DOMException(HIERARCHY_REQUEST_ERR, 'More than one element or text in fragment');
785-
}
786-
if (nodeChildElements.length === 1 && !isElementInsertionPossible(parent, child)) {
787-
throw new DOMException(HIERARCHY_REQUEST_ERR, 'Element in fragment can not be inserted before doctype');
788-
}
832+
833+
// DocumentFragment
834+
if (node.nodeType === Node.DOCUMENT_FRAGMENT_NODE) {
835+
var nodeChildElements = nodeChildNodes.filter(isElementNode);
836+
// If node has more than one element child or has a Text node child.
837+
if (nodeChildElements.length > 1 || find(nodeChildNodes, isTextNode)) {
838+
throw new DOMException(HIERARCHY_REQUEST_ERR, 'More than one element or text in fragment');
789839
}
790-
if (isElementNode(node)) {
791-
if (find(parentChildNodes, isElementNode) || !isElementInsertionPossible(parent, child)) {
792-
throw new DOMException(HIERARCHY_REQUEST_ERR, 'Only one element can be added and only after doctype');
793-
}
840+
// Otherwise, if `node` has one element child and either `parent` has an element child,
841+
// `child` is a doctype, or `child` is non-null and a doctype is following `child`.
842+
if (nodeChildElements.length === 1 && !isElementInsertionPossible(parent, child)) {
843+
throw new DOMException(HIERARCHY_REQUEST_ERR, 'Element in fragment can not be inserted before doctype');
794844
}
795-
if (isDocTypeNode(node)) {
796-
if (find(parentChildNodes, isDocTypeNode)) {
797-
throw new DOMException(HIERARCHY_REQUEST_ERR, 'Only one doctype is allowed');
798-
}
799-
var parentElementChild = find(parentChildNodes, isElementNode);
800-
if (child && parentChildNodes.indexOf(parentElementChild) < parentChildNodes.indexOf(child)) {
801-
throw new DOMException(HIERARCHY_REQUEST_ERR, 'Doctype can only be inserted before an element');
802-
}
803-
if (!child && parentElementChild) {
804-
throw new DOMException(HIERARCHY_REQUEST_ERR, 'Doctype can not be appended since element is present');
805-
}
845+
}
846+
// Element
847+
if (isElementNode(node)) {
848+
// `parent` has an element child, `child` is a doctype,
849+
// or `child` is non-null and a doctype is following `child`.
850+
if (!isElementInsertionPossible(parent, child)) {
851+
throw new DOMException(HIERARCHY_REQUEST_ERR, 'Only one element can be added and only after doctype');
852+
}
853+
}
854+
// DocumentType
855+
if (isDocTypeNode(node)) {
856+
// `parent` has a doctype child,
857+
if (find(parentChildNodes, isDocTypeNode)) {
858+
throw new DOMException(HIERARCHY_REQUEST_ERR, 'Only one doctype is allowed');
859+
}
860+
var parentElementChild = find(parentChildNodes, isElementNode);
861+
// `child` is non-null and an element is preceding `child`,
862+
if (child && parentChildNodes.indexOf(parentElementChild) < parentChildNodes.indexOf(child)) {
863+
throw new DOMException(HIERARCHY_REQUEST_ERR, 'Doctype can only be inserted before an element');
864+
}
865+
// or `child` is null and `parent` has an element child.
866+
if (!child && parentElementChild) {
867+
throw new DOMException(HIERARCHY_REQUEST_ERR, 'Doctype can not be appended since element is present');
806868
}
807869
}
870+
}
871+
872+
/**
873+
* @private
874+
* Step 6 of the checks before inserting and before replacing a child are different.
875+
*
876+
* @param {Document} parent the parent node to insert `node` into
877+
* @param {Node} node the node to insert
878+
* @param {Node | undefined} child the node that should become the `nextSibling` of `node`
879+
* @returns {Node}
880+
* @throws DOMException for several node combinations that would create a DOM that is not well-formed.
881+
* @throws DOMException if `child` is provided but is not a child of `parent`.
882+
* @see https://dom.spec.whatwg.org/#concept-node-ensure-pre-insertion-validity
883+
* @see https://dom.spec.whatwg.org/#concept-node-replace
884+
*/
885+
function assertPreReplacementValidityInDocument(parent, node, child) {
886+
var parentChildNodes = parent.childNodes || [];
887+
var nodeChildNodes = node.childNodes || [];
888+
889+
// DocumentFragment
890+
if (node.nodeType === Node.DOCUMENT_FRAGMENT_NODE) {
891+
var nodeChildElements = nodeChildNodes.filter(isElementNode);
892+
// If `node` has more than one element child or has a Text node child.
893+
if (nodeChildElements.length > 1 || find(nodeChildNodes, isTextNode)) {
894+
throw new DOMException(HIERARCHY_REQUEST_ERR, 'More than one element or text in fragment');
895+
}
896+
// Otherwise, if `node` has one element child and either `parent` has an element child that is not `child` or a doctype is following `child`.
897+
if (nodeChildElements.length === 1 && !isElementReplacementPossible(parent, child)) {
898+
throw new DOMException(HIERARCHY_REQUEST_ERR, 'Element in fragment can not be inserted before doctype');
899+
}
900+
}
901+
// Element
902+
if (isElementNode(node)) {
903+
// `parent` has an element child that is not `child` or a doctype is following `child`.
904+
if (!isElementReplacementPossible(parent, child)) {
905+
throw new DOMException(HIERARCHY_REQUEST_ERR, 'Only one element can be added and only after doctype');
906+
}
907+
}
908+
// DocumentType
909+
if (isDocTypeNode(node)) {
910+
function hasDoctypeChildThatIsNotChild(node) {
911+
return isDocTypeNode(node) && node !== child;
912+
}
913+
914+
// `parent` has a doctype child that is not `child`,
915+
if (find(parentChildNodes, hasDoctypeChildThatIsNotChild)) {
916+
throw new DOMException(HIERARCHY_REQUEST_ERR, 'Only one doctype is allowed');
917+
}
918+
var parentElementChild = find(parentChildNodes, isElementNode);
919+
// or an element is preceding `child`.
920+
if (child && parentChildNodes.indexOf(parentElementChild) < parentChildNodes.indexOf(child)) {
921+
throw new DOMException(HIERARCHY_REQUEST_ERR, 'Doctype can only be inserted before an element');
922+
}
923+
}
924+
}
925+
926+
/**
927+
* @private
928+
* @param {Node} parent the parent node to insert `node` into
929+
* @param {Node} node the node to insert
930+
* @param {Node=} child the node that should become the `nextSibling` of `node`
931+
* @returns {Node}
932+
* @throws DOMException for several node combinations that would create a DOM that is not well-formed.
933+
* @throws DOMException if `child` is provided but is not a child of `parent`.
934+
* @see https://dom.spec.whatwg.org/#concept-node-ensure-pre-insertion-validity
935+
*/
936+
function _insertBefore(parent, node, child, _inDocumentAssertion) {
937+
// To ensure pre-insertion validity of a node into a parent before a child, run these steps:
938+
assertPreInsertionValidity1to5(parent, node, child);
939+
940+
// If parent is a document, and any of the statements below, switched on the interface node implements,
941+
// are true, then throw a "HierarchyRequestError" DOMException.
942+
if (parent.nodeType === Node.DOCUMENT_NODE) {
943+
(_inDocumentAssertion || assertPreInsertionValidityInDocument)(parent, node, child);
944+
}
808945

809946
var cp = node.parentNode;
810947
if(cp){
@@ -912,6 +1049,17 @@ Document.prototype = {
9121049
}
9131050
return _removeChild(this,oldChild);
9141051
},
1052+
replaceChild: function (newChild, oldChild) {
1053+
//raises
1054+
_insertBefore(this, newChild, oldChild, assertPreReplacementValidityInDocument);
1055+
newChild.ownerDocument = this;
1056+
if (oldChild) {
1057+
this.removeChild(oldChild);
1058+
}
1059+
if (isElementNode(newChild)) {
1060+
this.documentElement = newChild;
1061+
}
1062+
},
9151063
// Introduced in DOM Level 2:
9161064
importNode : function(importedNode,deep){
9171065
return importNode(this,importedNode,deep);

‎test/dom/document.test.js

+29
Original file line numberDiff line numberDiff line change
@@ -163,4 +163,33 @@ describe('Document.prototype', () => {
163163
expect(root.parentNode).toBeNull()
164164
})
165165
})
166+
describe('replaceChild', () => {
167+
it('should remove the only element and add the new one', () => {
168+
const doc = new DOMImplementation().createDocument('', 'xml');
169+
const initialFirstChild = doc.firstChild;
170+
const replacement = doc.createElement('replaced');
171+
172+
doc.replaceChild(replacement, doc.firstChild);
173+
174+
expect(doc.childNodes).toHaveLength(1);
175+
expect(initialFirstChild.parentNode).toBeNull();
176+
expect(doc.documentElement.name).toBe(replacement.name);
177+
});
178+
});
179+
describe('removeChild', () => {
180+
it('should remove all connections to node', () => {
181+
const doc = new DOMImplementation().createDocument('', 'xml');
182+
doc.insertBefore(doc.createComment('just a comment'), doc.firstChild);
183+
expect(doc.childNodes).toHaveLength(2);
184+
const initialElement = doc.firstChild;
185+
186+
doc.removeChild(initialElement);
187+
188+
// expect(doc.documentElement).toBeNull();
189+
expect(initialElement.parentNode).toBeNull();
190+
expect(initialElement.nextSibling).toBeNull();
191+
// expect(initialElement.previousSibling).toBeNull();
192+
expect(doc.childNodes).toHaveLength(1);
193+
});
194+
});
166195
})

0 commit comments

Comments
 (0)