diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 58fb488dca7851b33007f9bb446dfe2e6aaefb4a..a455bb28d29348e55fe727d8973382b53f032d76 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,12 @@ +2010-12-10 Emil Eklund + + Reviewed by Adam Barth. + + Add testcase for ReplaceSelectionCommand crash. + https://bugs.webkit.org/show_bug.cgi?id=50840 + + * editing/execCommand/insertHTML-mutation-crash.html: Added. + 2010-12-10 Peter Kasting Unreviewed Chromium test expectations update. diff --git a/LayoutTests/editing/execCommand/insertHTML-mutation-crash-expected.txt b/LayoutTests/editing/execCommand/insertHTML-mutation-crash-expected.txt new file mode 100644 index 0000000000000000000000000000000000000000..9ceb737f58bb9ee79d53e4bb31935b7fe3e40990 --- /dev/null +++ b/LayoutTests/editing/execCommand/insertHTML-mutation-crash-expected.txt @@ -0,0 +1 @@ +PASS: No crash. diff --git a/LayoutTests/editing/execCommand/insertHTML-mutation-crash.html b/LayoutTests/editing/execCommand/insertHTML-mutation-crash.html new file mode 100644 index 0000000000000000000000000000000000000000..d46e6b36a1a8d3531c54b95d2aca48329fc46404 --- /dev/null +++ b/LayoutTests/editing/execCommand/insertHTML-mutation-crash.html @@ -0,0 +1,45 @@ + + + + + + +
+ This tests + that we don't crash when mutating the dom + during execution of an InsertHTML command. +
+ + diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog index b275e7a24609f9dfa8a045e4c7239f67044879ba..03e65aeaccd9b37818477bbaa95bf703df06b523 100644 --- a/WebCore/ChangeLog +++ b/WebCore/ChangeLog @@ -1,3 +1,20 @@ +2010-12-10 Emil Eklund + + Reviewed by Adam Barth. + + Fix crash in ReplaceSelectionCommand::doApply when selection is modified + during execution. + https://bugs.webkit.org/show_bug.cgi?id=50840 + + Test: editing/execCommand/insertHTML-mutation-crash.html + + * editing/ReplaceSelectionCommand.cpp: + (WebCore::ReplaceSelectionCommand::copyStyleToChildren): + Replaced raw node pointer with RefPtr. + + (WebCore::ReplaceSelectionCommand::doApply): + Replaced raw node pointer with RefPtr and added null check. + 2010-12-10 Emil Eklund Reviewed by Adam Barth. diff --git a/WebCore/editing/ReplaceSelectionCommand.cpp b/WebCore/editing/ReplaceSelectionCommand.cpp index 54a7fdeefc606c45786410da8db2325f59e92025..044ce634126947d3cbb8ed5a2dcc9c60dc837c7f 100644 --- a/WebCore/editing/ReplaceSelectionCommand.cpp +++ b/WebCore/editing/ReplaceSelectionCommand.cpp @@ -50,9 +50,12 @@ #include "markup.h" #include "visible_units.h" #include +#include namespace WebCore { +typedef Vector > NodeVector; + using namespace HTMLNames; enum EFragmentType { EmptyFragment, SingleTextNodeFragment, TreeFragment }; @@ -682,7 +685,12 @@ void ReplaceSelectionCommand::handleStyleSpans() void ReplaceSelectionCommand::copyStyleToChildren(Node* parentNode, const CSSMutableStyleDeclaration* parentStyle) { ASSERT(parentNode->hasTagName(spanTag)); - for (Node* childNode = parentNode->firstChild(); childNode; childNode = childNode->nextSibling()) { + NodeVector childNodes; + for (RefPtr childNode = parentNode->firstChild(); childNode; childNode = childNode->nextSibling()) + childNodes.append(childNode); + + for (NodeVector::const_iterator it = childNodes.begin(); it != childNodes.end(); it++) { + Node* childNode = it->get(); if (childNode->isTextNode() || !isBlock(childNode) || childNode->hasTagName(preTag)) { // In this case, put a span tag around the child node. RefPtr newNode = parentNode->cloneNode(false); @@ -864,6 +872,10 @@ void ReplaceSelectionCommand::doApply() // Inserting content could cause whitespace to collapse, e.g. inserting
foo
into hello^ world. prepareWhitespaceAtPositionForSplit(insertionPos); + + // If the downstream node has been removed there's no point in continuing. + if (!insertionPos.downstream().node()) + return; // NOTE: This would be an incorrect usage of downstream() if downstream() were changed to mean the last position after // p that maps to the same visible position as p (since in the case where a br is at the end of a block and collapsed @@ -942,8 +954,8 @@ void ReplaceSelectionCommand::doApply() bool plainTextFragment = isPlainTextMarkup(refNode.get()); while (node) { - Node* next = node->nextSibling(); - fragment.removeNode(node); + RefPtr next = node->nextSibling(); + fragment.removeNode(node.get()); insertNodeAfterAndUpdateNodesInserted(node, refNode.get()); // Mutation events (bug 22634) may have already removed the inserted content