Node.removeChild for Document's direct children doesn't update childNodes

[davidmc24]

Thanks for this library. I believe I encountered an edge case with Node.removeChild support that isn't working quite right.

If you have a Node that is a direct child of the Document (such as a ProcessingInstruction, DocumentType, Comment, Text, the document Element, etc.), calling document.removeChild passing the child node results in the node still being present in the document.childNodes. I ran into this trying to call document.replaceChild to replace a DocumentType, which resulted in the new DocumentType being inserted, but the old DocumentType not being fully removed.

It appears that this behavior is because _removeChild calls _onUpdateChild with parentNode.ownerDocument, and when the parentNode is a document, the ownerDocument is null.

Test that demonstrates the behavior:

'use strict'

var DOMParser = require('../../lib/dom-parser').DOMParser
var assert = require('../assert')

describe('Node operations', () => {
	it('removeChild supports removing nodes within the document element', () => {
		var doc = new DOMParser().parseFromString('<!-- Comment --><a><b/></a>')
		var parent = doc.documentElement
		var initialLength = parent.childNodes.length
		parent.removeChild(parent.firstChild)
		assert(parent.childNodes.length, initialLength-1)
	})

	it('removeChild supports removing nodes outside the document', () => {
		var doc = new DOMParser().parseFromString('<!-- Comment --><a><b/></a>')
		var parent = doc
		var initialLength = parent.childNodes.length
		parent.removeChild(parent.firstChild)
		assert(parent.childNodes.length, initialLength-1)
	})
})

When I run this test, the first test passes while the second fails.

[karfau]

Thank you for reporting this.
Can you check if your case is "the same"/a subset oft what is described in #135 ?

[davidmc24]

@karfau Sure thing. I've updated #135 with my analysis. While they both impact removeChild, they appear unrelated as this ticket deals only with the childNodes state and the other ticket deals only with firstChild/lastChild state.

[karfau]

Thx for helping us to understand the difference.

For later reference, I found this specification (of the current, so called "Living Standard") related to node removal: https://dom.spec.whatwg.org/#concept-node-remove

[karfau]

Versions 0.7.9 and 0.8.6 have been released with a fix for this bug (which was reported again as #456). It is also fixed in the pre-release 0.9.0-beta.6.

The reason was that the removal of the child was based on node.ownerDocument which was previously not set for the Document node itself.