Skip to content

Commit

Permalink
fix: Throw DOMException when calling removeChild with invalid paramet…
Browse files Browse the repository at this point in the history
…er (#494)

BREAKING CHANGE: Previously it was possible (but not documented) to call
`Node.removeChild` with any node in the tree,
and with certain exceptions, it would work. This is no longer the case:
calling `Node.removeChild` with an argument that is not a direct child
of the node that it is called from, will throw a NotFoundError
DOMException, as it is described by the specs.

https://dom.spec.whatwg.org/#concept-node-pre-remove 
Fixes #135
  • Loading branch information
karfau committed Jun 9, 2023
1 parent 169aa4e commit 8a9542d
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 12 deletions.
27 changes: 16 additions & 11 deletions lib/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -952,22 +952,26 @@ function _onUpdateChild(doc, el, newChild) {
* @private
*/
function _removeChild(parentNode, child) {
var previous = child.previousSibling;
var next = child.nextSibling;
if (previous) {
previous.nextSibling = next;
if (parentNode !== child.parentNode) {
throw new DOMException(NOT_FOUND_ERR, "child's parent is not parent");
}
//var index = parentNode.childNodes.
var oldPreviousSibling = child.previousSibling;
var oldNextSibling = child.nextSibling;
if (oldPreviousSibling) {
oldPreviousSibling.nextSibling = oldNextSibling;
} else {
parentNode.firstChild = next;
parentNode.firstChild = oldNextSibling;
}
if (next) {
next.previousSibling = previous;
if (oldNextSibling) {
oldNextSibling.previousSibling = oldPreviousSibling;
} else {
parentNode.lastChild = previous;
parentNode.lastChild = oldPreviousSibling;
}
_onUpdateChild(parentNode.ownerDocument, parentNode);
child.parentNode = null;
child.previousSibling = null;
child.nextSibling = null;
_onUpdateChild(parentNode.ownerDocument, parentNode);
return child;
}

Expand Down Expand Up @@ -1345,10 +1349,11 @@ Document.prototype = {
return newChild;
},
removeChild: function (oldChild) {
if (this.documentElement == oldChild) {
var removed = _removeChild(this, oldChild);
if (removed === this.documentElement) {
this.documentElement = null;
}
return _removeChild(this, oldChild);
return removed;
},
replaceChild: function (newChild, oldChild) {
//raises
Expand Down
15 changes: 14 additions & 1 deletion test/dom/document.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,21 @@ describe('Document.prototype', () => {
// expect(doc.documentElement).toBeNull();
expect(initialElement.parentNode).toBeNull();
expect(initialElement.nextSibling).toBeNull();
// expect(initialElement.previousSibling).toBeNull();
expect(initialElement.previousSibling).toBeNull();
expect(doc.childNodes).toHaveLength(1);
});

it('Remove child from non-parent node throws', async () => {
const ISSUE_CHECK = `<xml>
<a><x/></a>
<b><y/></b>
</xml>`;
const dom = new DOMParser().parseFromString(ISSUE_CHECK);
const ys = dom.getElementsByTagName('y');
const as = dom.getElementsByTagName('a');

expect(() => as[0].removeChild(ys[0])).toThrow(DOMException);
expect(dom.toString()).toBe(ISSUE_CHECK);
});
});
});

0 comments on commit 8a9542d

Please sign in to comment.