Diff
Modified: trunk/Source/WebCore/ChangeLog (119869 => 119870)
--- trunk/Source/WebCore/ChangeLog 2012-06-08 22:24:55 UTC (rev 119869)
+++ trunk/Source/WebCore/ChangeLog 2012-06-08 22:35:35 UTC (rev 119870)
@@ -1,3 +1,24 @@
+2012-06-08 Ryosuke Niwa <[email protected]>
+
+ Crash in WebCore::InsertParagraphSeparatorCommand::doApply
+ https://bugs.webkit.org/show_bug.cgi?id=88108
+
+ Reviewed by Levi Weintraub.
+
+ Use NodeVector instead of walking through siblings as we mutate the DOM.
+
+ No new tests are added since there is no reliable reduction.
+
+ * editing/BreakBlockquoteCommand.cpp:
+ (WebCore::BreakBlockquoteCommand::doApply):
+ * editing/CompositeEditCommand.cpp:
+ (WebCore::CompositeEditCommand::moveRemainingSiblingsToNewParent):
+ (WebCore):
+ * editing/CompositeEditCommand.h:
+ (CompositeEditCommand):
+ * editing/InsertParagraphSeparatorCommand.cpp:
+ (WebCore::InsertParagraphSeparatorCommand::doApply):
+
2012-06-08 David Grogan <[email protected]>
IndexedDB: rename some instances of open to registerFrontendCallbacks
Modified: trunk/Source/WebCore/editing/BreakBlockquoteCommand.cpp (119869 => 119870)
--- trunk/Source/WebCore/editing/BreakBlockquoteCommand.cpp 2012-06-08 22:24:55 UTC (rev 119869)
+++ trunk/Source/WebCore/editing/BreakBlockquoteCommand.cpp 2012-06-08 22:35:35 UTC (rev 119870)
@@ -126,7 +126,7 @@
}
// Build up list of ancestors in between the start node and the top blockquote.
- Vector<Element*> ancestors;
+ Vector<RefPtr<Element> > ancestors;
for (Element* node = startNode->parentElement(); node && node != topBlockquote; node = node->parentElement())
ancestors.append(node);
@@ -143,7 +143,7 @@
RefPtr<Element> clonedChild = ancestors[i - 1]->cloneElementWithoutChildren();
// Preserve list item numbering in cloned lists.
if (clonedChild->isElementNode() && clonedChild->hasTagName(olTag)) {
- Node* listChildNode = i > 1 ? ancestors[i - 2] : startNode;
+ Node* listChildNode = i > 1 ? ancestors[i - 2].get() : startNode;
// The first child of the cloned list might not be a list item element,
// find the first one so that we know where to start numbering.
while (listChildNode && !listChildNode->hasTagName(liTag))
@@ -155,37 +155,23 @@
appendNode(clonedChild.get(), clonedAncestor.get());
clonedAncestor = clonedChild;
}
-
- // Move the startNode and its siblings.
- Node *moveNode = startNode;
- while (moveNode) {
- Node *next = moveNode->nextSibling();
- removeNode(moveNode);
- appendNode(moveNode, clonedAncestor.get());
- moveNode = next;
- }
+ moveRemainingSiblingsToNewParent(startNode, 0, clonedAncestor);
+
if (!ancestors.isEmpty()) {
// Split the tree up the ancestor chain until the topBlockquote
// Throughout this loop, clonedParent is the clone of ancestor's parent.
// This is so we can clone ancestor's siblings and place the clones
// into the clone corresponding to the ancestor's parent.
- Element* ancestor;
- Element* clonedParent;
+ RefPtr<Element> ancestor;
+ RefPtr<Element> clonedParent;
for (ancestor = ancestors.first(), clonedParent = clonedAncestor->parentElement();
ancestor && ancestor != topBlockquote;
- ancestor = ancestor->parentElement(), clonedParent = clonedParent->parentElement()) {
- moveNode = ancestor->nextSibling();
- while (moveNode) {
- Node *next = moveNode->nextSibling();
- removeNode(moveNode);
- appendNode(moveNode, clonedParent);
- moveNode = next;
- }
- }
-
+ ancestor = ancestor->parentElement(), clonedParent = clonedParent->parentElement())
+ moveRemainingSiblingsToNewParent(ancestor->nextSibling(), 0, clonedParent);
+
// If the startNode's original parent is now empty, remove it
- Node* originalParent = ancestors.first();
+ Node* originalParent = ancestors.first().get();
if (!originalParent->hasChildNodes())
removeNode(originalParent);
}
Modified: trunk/Source/WebCore/editing/CompositeEditCommand.cpp (119869 => 119870)
--- trunk/Source/WebCore/editing/CompositeEditCommand.cpp 2012-06-08 22:24:55 UTC (rev 119869)
+++ trunk/Source/WebCore/editing/CompositeEditCommand.cpp 2012-06-08 22:35:35 UTC (rev 119870)
@@ -406,6 +406,20 @@
prune(parent.release());
}
+void CompositeEditCommand::moveRemainingSiblingsToNewParent(Node* node, Node* pastLastNodeToMove, PassRefPtr<Element> prpNewParent)
+{
+ NodeVector nodesToRemove;
+ RefPtr<Element> newParent = prpNewParent;
+
+ for (; node && node != pastLastNodeToMove; node = node->nextSibling())
+ nodesToRemove.append(node);
+
+ for (unsigned i = 0; i < nodesToRemove.size(); i++) {
+ removeNode(nodesToRemove[i]);
+ appendNode(nodesToRemove[i], newParent);
+ }
+}
+
void CompositeEditCommand::updatePositionForNodeRemovalPreservingChildren(Position& position, Node* node)
{
int offset = (position.anchorType() == Position::PositionIsOffsetInAnchor) ? position.offsetInContainerNode() : 0;
Modified: trunk/Source/WebCore/editing/CompositeEditCommand.h (119869 => 119870)
--- trunk/Source/WebCore/editing/CompositeEditCommand.h 2012-06-08 22:24:55 UTC (rev 119869)
+++ trunk/Source/WebCore/editing/CompositeEditCommand.h 2012-06-08 22:35:35 UTC (rev 119870)
@@ -126,6 +126,7 @@
HTMLElement* replaceElementWithSpanPreservingChildrenAndAttributes(PassRefPtr<HTMLElement>);
void removeNodePreservingChildren(PassRefPtr<Node>);
void removeNodeAndPruneAncestors(PassRefPtr<Node>);
+ void moveRemainingSiblingsToNewParent(Node*, Node* pastLastNodeToMove, PassRefPtr<Element> prpNewParent);
void updatePositionForNodeRemovalPreservingChildren(Position&, Node*);
void prune(PassRefPtr<Node>);
void replaceTextInNode(PassRefPtr<Text>, unsigned offset, unsigned count, const String& replacementText);
Modified: trunk/Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp (119869 => 119870)
--- trunk/Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp 2012-06-08 22:24:55 UTC (rev 119869)
+++ trunk/Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp 2012-06-08 22:35:35 UTC (rev 119870)
@@ -376,12 +376,7 @@
}
}
- while (n && n != blockToInsert) {
- Node *next = n->nextSibling();
- removeNode(n);
- appendNode(n, blockToInsert);
- n = next;
- }
+ moveRemainingSiblingsToNewParent(n, blockToInsert.get(), blockToInsert);
}
// Handle whitespace that occurs after the split
Modified: trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp (119869 => 119870)
--- trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp 2012-06-08 22:24:55 UTC (rev 119869)
+++ trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp 2012-06-08 22:35:35 UTC (rev 119870)
@@ -81,7 +81,7 @@
bool hasInterchangeNewlineAtEnd() const { return m_hasInterchangeNewlineAtEnd; }
void removeNode(PassRefPtr<Node>);
- void removeNodePreservingChildren(Node*);
+ void removeNodePreservingChildren(PassRefPtr<Node>);
private:
PassRefPtr<StyledElement> insertFragmentForTestRendering(Node* rootEditableNode);
@@ -211,14 +211,14 @@
return m_fragment ? m_fragment->lastChild() : 0;
}
-void ReplacementFragment::removeNodePreservingChildren(Node *node)
+void ReplacementFragment::removeNodePreservingChildren(PassRefPtr<Node> node)
{
if (!node)
return;
while (RefPtr<Node> n = node->firstChild()) {
removeNode(n);
- insertNodeBefore(n.release(), node);
+ insertNodeBefore(n.release(), node.get());
}
removeNode(node);
}
@@ -329,18 +329,12 @@
node = container->firstChild();
while (node) {
- Node *next = node->traverseNextNode();
+ RefPtr<Node> next = node->traverseNextNode();
if (isInterchangeConvertedSpaceSpan(node)) {
- RefPtr<Node> n = 0;
- while ((n = node->firstChild())) {
- removeNode(n);
- insertNodeBefore(n, node);
- }
- removeNode(node);
- if (n)
- next = n->traverseNextNode();
+ next = node->traverseNextSibling();
+ removeNodePreservingChildren(node);
}
- node = next;
+ node = next.get();
}
}
