WebCore:
authorjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Aug 2007 18:11:04 +0000 (18:11 +0000)
committerjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Aug 2007 18:11:04 +0000 (18:11 +0000)
        Reviewed by Adele.

        <rdar://problem/5156801> REGRESSION: Crash at DeleteSelectionCommand::doApply() when deleting table content

        * editing/DeleteSelectionCommand.cpp:
        (WebCore::DeleteSelectionCommand::handleGeneralDelete): Use a RefPtr
        for node.  If the node to be removed contains the selection, and if
        the next node to be removed (nextNode) is inside the deletion UI,
        removing node will remove nextNode from the document.  nextNode is
        a RefPtr, but node isn't and when nextNode falls out of scope the node
        that node points to will be destroyed and we'll end up using a stale pointer.
        Long term we should probably just disable the deletion UI before editing
        operations because the undo of the removal of node in the situation
        described above relies on the presence of the deletion UI, but it isn't
        present because its added and removed in a non-undoable way.

LayoutTests:

        Reviewed by Adele.

        <rdar://problem/5156801> REGRESSION: Crash at DeleteSelectionCommand::doApply() when deleting table content

        * editing/deleting/5156801-2.html: Added.
        * platform/mac/editing/deleting: Added.
        * platform/mac/editing/deleting/5156801-2-expected.checksum: Added.
        * platform/mac/editing/deleting/5156801-2-expected.png: Added.
        * platform/mac/editing/deleting/5156801-2-expected.txt: Added.

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

LayoutTests/ChangeLog
LayoutTests/editing/deleting/5156801-2.html [new file with mode: 0644]
LayoutTests/platform/mac/editing/deleting/5156801-2-expected.checksum [new file with mode: 0644]
LayoutTests/platform/mac/editing/deleting/5156801-2-expected.png [new file with mode: 0644]
LayoutTests/platform/mac/editing/deleting/5156801-2-expected.txt [new file with mode: 0644]
WebCore/ChangeLog
WebCore/editing/DeleteSelectionCommand.cpp

index 8909c3f46e3ec34cf2df8bf9dbddd25a05dfc008..e540ea4398bc8d716f4b7812c3e39755eb70255f 100644 (file)
@@ -1,3 +1,15 @@
+2007-08-23  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Adele.
+        
+        <rdar://problem/5156801> REGRESSION: Crash at DeleteSelectionCommand::doApply() when deleting table content
+
+        * editing/deleting/5156801-2.html: Added.
+        * platform/mac/editing/deleting: Added.
+        * platform/mac/editing/deleting/5156801-2-expected.checksum: Added.
+        * platform/mac/editing/deleting/5156801-2-expected.png: Added.
+        * platform/mac/editing/deleting/5156801-2-expected.txt: Added.
+
 2007-08-22  Justin Garcia  <justin.garcia@apple.com>
 
         Reviewed by Adam.
diff --git a/LayoutTests/editing/deleting/5156801-2.html b/LayoutTests/editing/deleting/5156801-2.html
new file mode 100644 (file)
index 0000000..f43f9e1
--- /dev/null
@@ -0,0 +1,18 @@
+<body>
+<p>This tests for a crash when deleting the contents of a table cell. You should just see 'Cached' below.</p>
+<div contenteditable="true"><table id="table" border=0 cellpadding=0 cellspacing=0><tr><td><br><a  href="fakelink.html">Cached</a><a  href=""fakelink.html">Similar</a></td></tr></table>
+</div>
+<script>
+s = window.getSelection();
+table = document.getElementById("table");
+s.setPosition(table, table.childNodes.length);
+s.modify("move", "backward", "character");
+document.execCommand("Delete");
+document.execCommand("Delete");
+document.execCommand("Delete");
+document.execCommand("Delete");
+document.execCommand("Delete");
+document.execCommand("Delete");
+document.execCommand("Delete");
+</script>
+</body>
diff --git a/LayoutTests/platform/mac/editing/deleting/5156801-2-expected.checksum b/LayoutTests/platform/mac/editing/deleting/5156801-2-expected.checksum
new file mode 100644 (file)
index 0000000..2528a49
--- /dev/null
@@ -0,0 +1 @@
+e334e5a84a78ed37798ad6b85aac99e2
\ No newline at end of file
diff --git a/LayoutTests/platform/mac/editing/deleting/5156801-2-expected.png b/LayoutTests/platform/mac/editing/deleting/5156801-2-expected.png
new file mode 100644 (file)
index 0000000..998ec72
Binary files /dev/null and b/LayoutTests/platform/mac/editing/deleting/5156801-2-expected.png differ
diff --git a/LayoutTests/platform/mac/editing/deleting/5156801-2-expected.txt b/LayoutTests/platform/mac/editing/deleting/5156801-2-expected.txt
new file mode 100644 (file)
index 0000000..860da5f
--- /dev/null
@@ -0,0 +1,18 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderBlock {P} at (0,0) size 784x18
+        RenderText {#text} at (0,0) size 626x18
+          text run at (0,0) width 626: "This tests for a crash when deleting the contents of a table cell. You should just see 'Cached' below."
+      RenderBlock {DIV} at (0,34) size 784x36
+        RenderTable {TABLE} at (0,0) size 48x36
+          RenderTableSection {TBODY} at (0,0) size 48x36
+            RenderTableRow {TR} at (0,0) size 48x36
+              RenderTableCell {TD} at (0,0) size 48x36 [r=0 c=0 rs=1 cs=1]
+                RenderBR {BR} at (0,0) size 0x18
+                RenderInline {A} at (0,0) size 48x18 [color=#0000EE]
+                  RenderText {#text} at (0,18) size 48x18
+                    text run at (0,18) width 48: "Cached"
+caret: position 6 of child 0 {#text} of child 1 {A} of child 0 {TD} of child 0 {TR} of child 0 {TBODY} of child 0 {TABLE} of child 3 {DIV} of child 0 {BODY} of child 0 {HTML} of document
index f008b71f4f5cb3b3bda95bbbcd5a56663f7d5e72..7129994b9d6adeaa80376d81cf703aeff20776d7 100644 (file)
@@ -1,3 +1,21 @@
+2007-08-23  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Adele.
+
+        <rdar://problem/5156801> REGRESSION: Crash at DeleteSelectionCommand::doApply() when deleting table content
+
+        * editing/DeleteSelectionCommand.cpp:
+        (WebCore::DeleteSelectionCommand::handleGeneralDelete): Use a RefPtr
+        for node.  If the node to be removed contains the selection, and if
+        the next node to be removed (nextNode) is inside the deletion UI,
+        removing node will remove nextNode from the document.  nextNode is
+        a RefPtr, but node isn't and when nextNode falls out of scope the node
+        that node points to will be destroyed and we'll end up using a stale pointer.
+        Long term we should probably just disable the deletion UI before editing 
+        operations because the undo of the removal of node in the situation 
+        described above relies on the presence of the deletion UI, but it isn't 
+        present because its added and removed in a non-undoable way.
+
 2007-08-23  Mitz Pettel  <mitz@webkit.org>
 
         Reviewed by Darin.
index 51650a1125d66edc499a6ad5dd3dfe11f8a1a1f3..a6bf480a17b6850ba2ba4793d4c0a1260239daee 100644 (file)
@@ -405,12 +405,12 @@ void DeleteSelectionCommand::handleGeneralDelete()
     }
     else {
         // The selection to delete spans more than one node.
-        Node *node = startNode;
+        RefPtr<Node> node = startNode;
         
         if (startOffset > 0) {
             if (startNode->isTextNode()) {
                 // in a text node that needs to be trimmed
-                Text *text = static_cast<Text *>(node);
+                Text *text = static_cast<Text *>(node.get());
                 deleteTextFromNode(text, startOffset, text->length() - startOffset);
                 node = node->traverseNextNode();
             } else {
@@ -420,10 +420,10 @@ void DeleteSelectionCommand::handleGeneralDelete()
         
         // handle deleting all nodes that are completely selected
         while (node && node != m_downstreamEnd.node()) {
-            if (Range::compareBoundaryPoints(Position(node, 0), m_downstreamEnd) >= 0) {
+            if (Range::compareBoundaryPoints(Position(node.get(), 0), m_downstreamEnd) >= 0) {
                 // traverseNextSibling just blew past the end position, so stop deleting
                 node = 0;
-            } else if (!m_downstreamEnd.node()->isDescendantOf(node)) {
+            } else if (!m_downstreamEnd.node()->isDescendantOf(node.get())) {
                 RefPtr<Node> nextNode = node->traverseNextSibling();
                 // if we just removed a node from the end container, update end position so the
                 // check above will work
@@ -431,12 +431,12 @@ void DeleteSelectionCommand::handleGeneralDelete()
                     ASSERT(node->nodeIndex() < (unsigned)m_downstreamEnd.offset());
                     m_downstreamEnd = Position(m_downstreamEnd.node(), m_downstreamEnd.offset() - 1);
                 }
-                removeNode(node);
+                removeNode(node.get());
                 node = nextNode.get();
             } else {
                 Node* n = node->lastDescendant();
                 if (m_downstreamEnd.node() == n && m_downstreamEnd.offset() >= n->caretMaxOffset()) {
-                    removeNode(node);
+                    removeNode(node.get());
                     node = 0;
                 } else
                     node = node->traverseNextNode();