Skip to content

Commit

Permalink
fix(dom): Remove all links as part of removeChild
Browse files Browse the repository at this point in the history
which fixes weird side effects when trying to `appendChild` the same node later
  • Loading branch information
karfau committed Dec 22, 2021
1 parent 2761639 commit c73f8f5
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 29 deletions.
70 changes: 41 additions & 29 deletions lib/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -591,48 +591,65 @@ function _onRemoveAttribute(doc,el,newAttr,remove){
}
}

/**
* Updates `el.childNodes` by either appending `newChild`
* or updating the indexed items, by iterating from `el.firstChild` over all `nextSibling`s
* and updating the `length` accordingly.
*
* @param {Document} doc
* @param {Node} el
* @param {Node} [newChild]
* @private
*/
function _onUpdateChild(doc,el,newChild){
if(doc && doc._inc){
doc._inc++;
//update childNodes
var cs = el.childNodes;
if(newChild){
if (newChild) {
cs[cs.length++] = newChild;
}else{
//console.log(1)
} else {
var child = el.firstChild;
var i = 0;
while(child){
while (child) {
cs[i++] = child;
child =child.nextSibling;
child = child.nextSibling;
}
cs.length = i;
delete cs[cs.length];
}
}
}

/**
* attributes;
* children;
*
* writeable properties:
* nodeValue,Attr:value,CharacterData:data
* prefix
* Removes the connections between `parentNode` and `child`
* and any existing `child.previousSibling` or `child.nextSibling`.
*
* @see https://github.com/xmldom/xmldom/issues/135
* @see https://github.com/xmldom/xmldom/issues/145
*
* @param {Node} parentNode
* @param {Node} child
* @returns {Node} the child that was removed.
* @private
*/
function _removeChild(parentNode,child){
var previous = child.previousSibling;
var next = child.nextSibling;
if(previous){
if (previous) {
previous.nextSibling = next;
}else{
parentNode.firstChild = next
} else {
parentNode.firstChild = next;
}
if(next){
if (next) {
next.previousSibling = previous;
}else{
} else {
parentNode.lastChild = previous;
}
_onUpdateChild(parentNode.ownerDocument,parentNode);
child.parentNode = null;
child.previousSibling = null;
child.nextSibling = null;
_onUpdateChild(parentNode.ownerDocument, parentNode);
return child;
}
/**
Expand Down Expand Up @@ -679,25 +696,20 @@ function _insertBefore(parentNode,newChild,nextChild){
return newChild;
}
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;
}else{
if (newChild.previousSibling) {
newChild.previousSibling.nextSibling = newChild;
} else {
parentNode.firstChild = newChild;
}
parentNode.lastChild = newChild;
_onUpdateChild(parentNode.ownerDocument,parentNode,newChild);
_onUpdateChild(parentNode.ownerDocument, parentNode, newChild);
return newChild;
//console.log("__aa",parentNode.lastChild.nextSibling == null)
}
Document.prototype = {
//implementation : null,
Expand Down
33 changes: 33 additions & 0 deletions test/dom/element.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict'

const { DOMParser, DOMImplementation, XMLSerializer } = require('../../lib')
const { Node } = require('../../lib/dom')

describe('Document', () => {
// See: http://jsfiddle.net/bigeasy/ShcXP/1/
Expand Down Expand Up @@ -175,6 +176,38 @@ describe('Document', () => {
})
})

it('appendElement and removeElement', () => {
const dom = new DOMParser().parseFromString(`<root><A/><B/><C/></root>`)
const doc = dom.documentElement
const arr = []
while (doc.firstChild) {
const node = doc.removeChild(doc.firstChild)
arr.push(node)
expect(node.parentNode).toBeNull()
expect(node.previousSibling).toBeNull()
expect(node.nextSibling).toBeNull()
expect(node.ownerDocument).toBe(dom)
expect(doc.firstChild).not.toBe(node)
const expectedLength = 3 - arr.length
expect(doc.childNodes).toHaveLength(expectedLength)
expect(doc.childNodes.item(expectedLength)).toBeNull()
}
expect(arr).toHaveLength(3)
while (arr.length) {
const node = arr.shift()
expect(doc.appendChild(node)).toBe(node)
expect(node.parentNode).toBe(doc)
const expectedLength = 3 - arr.length
expect(doc.childNodes).toHaveLength(expectedLength)
expect(doc.childNodes.item(expectedLength - 1)).toBe(node)
if (expectedLength > 1) {
expect(node.previousSibling).not.toBeInstanceOf(Node)
expect(node.previousSibling.nextSibling).toBe(node)
}
}
expect(doc.childNodes.toString()).toBe(`<A/><B/><C/>`)
})

xit('nested append failed', () => {})

xit('self append failed', () => {})
Expand Down

0 comments on commit c73f8f5

Please sign in to comment.