Created attachment 101702 [details] Repro Chromium: https://code.google.com/p/chromium/issues/detail?id=90147 Repro: <body onload="go()"></body> <script> function go() { var oBodyImported = document.importNode(document.body,false); document.body.appendChild(oBodyImported); var oRange1 = document.createRange(); var oRange2 = document.createRange(); document.body=document.activeElement; var oDocumentFragment = document.createDocumentFragment(); oRange1.setStartBefore(oBodyImported); oRange1.insertNode(oDocumentFragment); oRange2.setStart(document.body,0); oRange2.surroundContents(oBodyImported); ''+oRange1; } </script> The range oRange1 ends up containing a NULL node, among valid nodes. This is probably not supposed to happen, as some of the range's methods that access the nodes do not handle this correctly. In the repro, we end up executing this code: String Range::toString(ExceptionCode& ec) const { if (!m_start.container()) { ec = INVALID_STATE_ERR; return String(); } StringBuilder builder; Node* pastLast = pastLastNode(); for (Node* n = firstNode(); n != pastLast; n = n->traverseNextNode()) { if (n->nodeType() == Node::TEXT_NODE || n->nodeType() == Node::CDATA_SECTION_NODE) { String data = static_cast<CharacterData*>(n)->data(); int length = data.length(); int start = (n == m_start.container()) ? min(max(0, m_start.offset()), length) : 0; int end = (n == m_end.container()) ? min(max(start, m_end.offset()), length) : length; builder.append(data.characters() + start, end - start); } } return builder.toString(); } "n" goes through the nodes, which includes the NULL node. This causes a NULL ptr when the code tries to access n->nodeType(). The fact that the code loops through the nodes using n->traverseNextNode() signals that NULL nodes are probably not supposed to be part of a range.
What is a NULL node?
I think I understand. The issue is not with ranges. The issue is that insertNode does the wrong thing when passed an empty document fragment.
I think all that’s needed to fix this bug is code to return from Range::insertNode function and do no further work if newNode->firstChild() is zero inside the DOCUMENT_FRAGMENT_NODE section of the code.
@Darin; yes, with "NULL node" I meant a NULL ptr where a ptr to a node is expected. You analysis sounds correct to me, but I don't know the code, so don't take my word for it.
I think I’ll fix this myself, but if someone else wants to tackle it first, they are welcome to.
Created attachment 104621 [details] Patch
Committed r93481: <http://trac.webkit.org/changeset/93481>
Mass moving XML DOM bugs to the "DOM" Component.