LayoutTests:

        Reviewed by John.

        <rdar://problem/5156801> REGRESSION: Crash when deleting table content in Leopard Mail

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

WebCore:

        Reviewed by John.

        REGRESSION: Crash when deleting table content in Leopard Mail

        * editing/DeleteSelectionCommand.cpp:
        (WebCore::DeleteSelectionCommand::handleGeneralDelete): Use a RefPtr
        for the next node to delete, in case it is destroyed when the
        current node is deleted.  In the crash reported in the radar, we
        crash because the next node is the div that holds the delete button
        and it's destroyed when the node that contains the selection is removed.

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 97b3f6c12aac8e7603af86f7d18bfe5bba779d5f..5cf193811101b1b7bcb0c85f889b6ffd8f55c080 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2007-07-09  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by John.
+        
+        <rdar://problem/5156801> REGRESSION: Crash when deleting table content in Leopard Mail
+
+        * editing/deleting/5156801-expected.checksum: Added.
+        * editing/deleting/5156801-expected.png: Added.
+        * editing/deleting/5156801-expected.txt: Added.
+        * editing/deleting/5156801.html: Added.
+
 2007-07-09  Anders Carlsson  <andersca@apple.com>
 
         Reviewed by Oliver.

diff --git a/LayoutTests/editing/deleting/5156801-expected.checksum b/LayoutTests/editing/deleting/5156801-expected.checksum
new file mode 100644 (file)
index 0000000..60d687a
--- /dev/null
+++ b/LayoutTests/editing/deleting/5156801-expected.checksum
@@ -0,0 +1 @@
+c003f0a32c771dbd0cced1f565e43caf
\ No newline at end of file

diff --git a/LayoutTests/editing/deleting/5156801-expected.png b/LayoutTests/editing/deleting/5156801-expected.png
new file mode 100644 (file)
index 0000000..f6932a6
Binary files /dev/null and b/LayoutTests/editing/deleting/5156801-expected.png differ

diff --git a/LayoutTests/editing/deleting/5156801-expected.txt b/LayoutTests/editing/deleting/5156801-expected.txt
new file mode 100644 (file)
index 0000000..1bb98d6
--- /dev/null
+++ b/LayoutTests/editing/deleting/5156801-expected.txt
@@ -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 784x36
+        RenderText {#text} at (0,0) size 779x36
+          text run at (0,0) width 779: "This tests for a bug where delete would crash if a node just after the selection to delete is removed in a DOMNodeRemoved"
+          text run at (0,18) width 349: "event handler for the last node in the selection to delete."
+      RenderBlock {DIV} at (0,52) size 784x18
+        RenderText {#text} at (0,0) size 21x18
+          text run at (0,0) width 21: "foo"
+        RenderInline {SPAN} at (0,0) size 8x18
+          RenderText {#text} at (21,0) size 8x18
+            text run at (21,0) width 8: "x"
+        RenderInline {SPAN} at (0,0) size 8x18
+          RenderText {#text} at (29,0) size 8x18
+            text run at (29,0) width 8: "y"

diff --git a/LayoutTests/editing/deleting/5156801.html b/LayoutTests/editing/deleting/5156801.html
new file mode 100644 (file)
index 0000000..cc4c8fe
--- /dev/null
+++ b/LayoutTests/editing/deleting/5156801.html
@@ -0,0 +1,11 @@
+<p>This tests for a bug where delete would crash if a node just after the selection to delete is removed in a DOMNodeRemoved event handler for the last node in the selection to delete.</p>
+<div contenteditable="true">foo<span id="foo">x</span><span id="removeme">y</span></div>
+
+<script>
+function removeHandler() {
+    var removeme = document.getElementById("removeme");
+    removeme.parentNode.removeChild(removeme);
+}
+
+document.getElementById("foo").addEventListener("DOMNodeRemoved", removeHandler);
+</script>

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index fea8f64d20669d6b838edacb45aa7cc7f0c09e0a..a221340ed195deeca094fc8be5685b0e05335983 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,16 @@
+2007-07-09  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by John.
+        
+        REGRESSION: Crash when deleting table content in Leopard Mail
+
+        * editing/DeleteSelectionCommand.cpp:
+        (WebCore::DeleteSelectionCommand::handleGeneralDelete): Use a RefPtr
+        for the next node to delete, in case it is destroyed when the
+        current node is deleted.  In the crash reported in the radar, we
+        crash because the next node is the div that holds the delete button
+        and it's destroyed when the node that contains the selection is removed.
+
 2007-07-09  Adam Treat  <adam@staikos.net>
 
         Reviewed by George Staikos.

diff --git a/WebCore/editing/DeleteSelectionCommand.cpp b/WebCore/editing/DeleteSelectionCommand.cpp
index 1c2da7cec3dfc12646f0ffde4f8c1d33655752a1..327818f42e89f95b212890286920db38a9b33fee 100644 (file)
--- a/WebCore/editing/DeleteSelectionCommand.cpp
+++ b/WebCore/editing/DeleteSelectionCommand.cpp
@@ -390,7 +390,7 @@ void DeleteSelectionCommand::handleGeneralDelete()
                 // traverseNextSibling just blew past the end position, so stop deleting
                 node = 0;
             } else if (!m_downstreamEnd.node()->isDescendantOf(node)) {
-                Node *nextNode = node->traverseNextSibling();
+                RefPtr<Node> nextNode = node->traverseNextSibling();
                 // if we just removed a node from the end container, update end position so the
                 // check above will work
                 if (node->parentNode() == m_downstreamEnd.node()) {
@@ -398,7 +398,7 @@ void DeleteSelectionCommand::handleGeneralDelete()
                     m_downstreamEnd = Position(m_downstreamEnd.node(), m_downstreamEnd.offset() - 1);
                 }
                 removeNode(node);
-                node = nextNode;
+                node = nextNode.get();
             } else {
                 Node* n = node->lastDescendant();
                 if (m_downstreamEnd.node() == n && m_downstreamEnd.offset() >= n->caretMaxOffset()) {