2010-12-10 Emil Eklund <eae@chromium.org>
authorinferno@chromium.org <inferno@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Dec 2010 22:09:37 +0000 (22:09 +0000)
committerinferno@chromium.org <inferno@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Dec 2010 22:09:37 +0000 (22:09 +0000)
        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  <eae@chromium.org>

        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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@73801 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/editing/execCommand/insertHTML-mutation-crash-expected.txt [new file with mode: 0644]
LayoutTests/editing/execCommand/insertHTML-mutation-crash.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/editing/ReplaceSelectionCommand.cpp

index 58fb488..a455bb2 100644 (file)
@@ -1,3 +1,12 @@
+2010-12-10  Emil Eklund  <eae@chromium.org>
+
+        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  <pkasting@google.com>
 
         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 (file)
index 0000000..9ceb737
--- /dev/null
@@ -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 (file)
index 0000000..d46e6b3
--- /dev/null
@@ -0,0 +1,45 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <script type="text/javascript">
+        function log(msg)
+        {
+            document.body.appendChild(document.createTextNode(msg + '\n'));
+        }
+
+        function runTests()
+        {
+            if (window.layoutTestController)
+                layoutTestController.dumpAsText();
+
+            var listener = function(e) {
+                var el = document.getElementById('cont');
+                if (el.firstElementChild && el.lastElementChild != el.firstElementChild) {
+                    el.lastElementChild.appendChild(el.firstElementChild);
+                    el.lastElementChild && el.removeChild(el.lastElementChild);
+                }
+                if (e.target.firstChild && e.target.firstChild.className == 'Apple-style-span')
+                    e.target.firstChild.innerHTML = e.target.firstChild.innerHTML.split(' ')[0];
+            };
+            document.addEventListener("DOMSubtreeModified", listener);
+
+            var el = document.getElementById('cont');
+            window.getSelection().setBaseAndExtent(document.getElementById('start'), 0, document.getElementById('end'), 0);
+            var str = '<span class="Apple-style-span" style="color: red;"><span>styled</span> <span>content</span></span>';
+            document.execCommand("InsertHTML", false, str);
+
+            document.removeEventListener("DOMSubtreeModified", listener);
+
+            log('PASS: No crash.');
+        }
+
+    </script>
+</head>
+<body onload="runTests();">
+    <div id="cont" contenteditable="true">
+        <span>This <span id="start">tests</span></span>
+        <span>that we don't crash when <code id="end">mutating</code> the dom</span>
+        <span>during execution of an InsertHTML command.</span>
+    </div>
+</body>
+</html>
index b275e7a..03e65ae 100644 (file)
@@ -2,6 +2,23 @@
 
         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  <eae@chromium.org>
+
+        Reviewed by Adam Barth.
+
         Fix crash in Range::processContents when modified during mutation event.
         https://bugs.webkit.org/show_bug.cgi?id=50710
 
index 54a7fde..044ce63 100644 (file)
 #include "markup.h"
 #include "visible_units.h"
 #include <wtf/StdLibExtras.h>
+#include <wtf/Vector.h>
 
 namespace WebCore {
 
+typedef Vector<RefPtr<Node> > 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<Node> 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<Node> newNode = parentNode->cloneNode(false);
@@ -864,6 +872,10 @@ void ReplaceSelectionCommand::doApply()
     
     // Inserting content could cause whitespace to collapse, e.g. inserting <div>foo</div> 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<Node> next = node->nextSibling();
+        fragment.removeNode(node.get());
         insertNodeAfterAndUpdateNodesInserted(node, refNode.get());
 
         // Mutation events (bug 22634) may have already removed the inserted content