WebCore:
authorjustin.garcia@apple.com <justin.garcia@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Dec 2007 06:56:28 +0000 (06:56 +0000)
committerjustin.garcia@apple.com <justin.garcia@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Dec 2007 06:56:28 +0000 (06:56 +0000)
        Reviewed by Darin Adler.

        <rdar://problem/5433862> Mail crashes at WebCore::highestAncestor() when deleting a particular selection

        * editing/DeleteSelectionCommand.cpp:
        (WebCore::DeleteSelectionCommand::removePreviouslySelectedEmptyTableRows):
        Don't remove the table row that contained the end of the selection if it is where we are
        about to place the ending selection.
        Don't remove all empty rows after the row that contained the start of the selection,
        they might come after the row that contained the end of the selection.

LayoutTests:

        Reviewed by Darin Adler.

        <rdar://problem/5433862> Mail crashes at WebCore::highestAncestor() when deleting a particular selection

        * editing/deleting/5433862-1-expected.txt: Added.
        * editing/deleting/5433862-1.html: Added.
        * editing/deleting/5433862-2.html: Added.
        * platform/mac/editing/deleting/5433862-2-expected.checksum: Added.
        * platform/mac/editing/deleting/5433862-2-expected.txt: Added.

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

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

index 60e9108..24a9799 100644 (file)
@@ -1,3 +1,15 @@
+2007-12-12  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Darin Adler.
+
+        <rdar://problem/5433862> Mail crashes at WebCore::highestAncestor() when deleting a particular selection
+
+        * editing/deleting/5433862-1-expected.txt: Added.
+        * editing/deleting/5433862-1.html: Added.
+        * editing/deleting/5433862-2.html: Added.
+        * platform/mac/editing/deleting/5433862-2-expected.checksum: Added.
+        * platform/mac/editing/deleting/5433862-2-expected.txt: Added.
+
 2007-12-12  Dan Bernstein  <mitz@apple.com>
 
         Reviewed by John Sullivan.
diff --git a/LayoutTests/editing/deleting/5433862-1-expected.txt b/LayoutTests/editing/deleting/5433862-1-expected.txt
new file mode 100644 (file)
index 0000000..953c81d
--- /dev/null
@@ -0,0 +1,8 @@
+This tests for a crash when removing a selection ends with a fully selected table row.
+
+foo
+
+
+
+
+foo    bar     baz
diff --git a/LayoutTests/editing/deleting/5433862-1.html b/LayoutTests/editing/deleting/5433862-1.html
new file mode 100644 (file)
index 0000000..4c4f0e6
--- /dev/null
@@ -0,0 +1,24 @@
+<body>
+    
+<p>This tests for a crash when removing a selection ends with a fully selected table row.</p>
+
+<div contenteditable="true">
+<div id="start">foo<br><br></div>
+<table border="1">
+<tr><td>foo</td><td>bar</td><td id="end"></td></tr>
+<tr><td>foo</td><td>bar</td><td>baz</td></tr>
+</table>
+</div>
+
+<script>
+if (window.layoutTestController)
+    window.layoutTestController.dumpAsText();
+    
+start = document.getElementById("start");
+end = document.getElementById("end");
+
+window.getSelection().setBaseAndExtent(start, start.childNodes.length, end, end.childNodes.length);
+document.execCommand("Delete");
+</script>
+
+</body>
diff --git a/LayoutTests/editing/deleting/5433862-2.html b/LayoutTests/editing/deleting/5433862-2.html
new file mode 100644 (file)
index 0000000..948c2ad
--- /dev/null
@@ -0,0 +1,20 @@
+<body>
+<p>This tests for a bug where empty table rows well after the selection to delete were removed.  There should be five rows in the table below, before and after the deletion.</p>
+<div contenteditable="true">
+<table border="1" cellpadding="5">
+<tr><td id="start">foo</td><td>bar</td><td id="end">baz</td></tr>
+<tr><td>foo</td><td>bar</td><td>baz</td></tr>
+<tr><td><br></td><td><br></td><td><br></td></tr>
+<tr><td><br></td><td><br></td><td><br></td></tr>
+</table>
+</div>
+
+<script>
+start = document.getElementById("start");
+end = document.getElementById("end");
+
+window.getSelection().setBaseAndExtent(start, 0, end, end.childNodes.length);
+document.execCommand("Delete");
+</script>
+
+</body>
diff --git a/LayoutTests/platform/mac/editing/deleting/5433862-2-expected.checksum b/LayoutTests/platform/mac/editing/deleting/5433862-2-expected.checksum
new file mode 100644 (file)
index 0000000..2a00259
--- /dev/null
@@ -0,0 +1 @@
+163cbd7f6fb0458b94933e848d765d4f
\ No newline at end of file
diff --git a/LayoutTests/platform/mac/editing/deleting/5433862-2-expected.png b/LayoutTests/platform/mac/editing/deleting/5433862-2-expected.png
new file mode 100644 (file)
index 0000000..fd9d165
Binary files /dev/null and b/LayoutTests/platform/mac/editing/deleting/5433862-2-expected.png differ
diff --git a/LayoutTests/platform/mac/editing/deleting/5433862-2-expected.txt b/LayoutTests/platform/mac/editing/deleting/5433862-2-expected.txt
new file mode 100644 (file)
index 0000000..b1fda83
--- /dev/null
@@ -0,0 +1,45 @@
+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 783x36
+          text run at (0,0) width 579: "This tests for a bug where empty table rows well after the selection to delete were removed. "
+          text run at (579,0) width 204: "There should be five rows in the"
+          text run at (0,18) width 261: "table below, before and after the deletion."
+      RenderBlock {DIV} at (0,52) size 784x132
+        RenderTable {TABLE} at (0,0) size 109x132 [border: (1px outset #808080)]
+          RenderTableSection {TBODY} at (1,1) size 107x130
+            RenderTableRow {TR} at (0,2) size 107x30
+              RenderTableCell {TD} at (2,2) size 33x30 [border: (1px inset #808080)] [r=0 c=0 rs=1 cs=1]
+                RenderBR {BR} at (6,6) size 0x18
+              RenderTableCell {TD} at (37,2) size 32x30 [border: (1px inset #808080)] [r=0 c=1 rs=1 cs=1]
+                RenderBR {BR} at (6,6) size 0x18
+              RenderTableCell {TD} at (71,2) size 34x30 [border: (1px inset #808080)] [r=0 c=2 rs=1 cs=1]
+                RenderBR {BR} at (6,6) size 0x18
+            RenderTableRow {TR} at (0,34) size 107x30
+              RenderTableCell {TD} at (2,34) size 33x30 [border: (1px inset #808080)] [r=1 c=0 rs=1 cs=1]
+                RenderText {#text} at (6,6) size 21x18
+                  text run at (6,6) width 21: "foo"
+              RenderTableCell {TD} at (37,34) size 32x30 [border: (1px inset #808080)] [r=1 c=1 rs=1 cs=1]
+                RenderText {#text} at (6,6) size 20x18
+                  text run at (6,6) width 20: "bar"
+              RenderTableCell {TD} at (71,34) size 34x30 [border: (1px inset #808080)] [r=1 c=2 rs=1 cs=1]
+                RenderText {#text} at (6,6) size 22x18
+                  text run at (6,6) width 22: "baz"
+            RenderTableRow {TR} at (0,66) size 107x30
+              RenderTableCell {TD} at (2,66) size 33x30 [border: (1px inset #808080)] [r=2 c=0 rs=1 cs=1]
+                RenderBR {BR} at (6,6) size 0x18
+              RenderTableCell {TD} at (37,66) size 32x30 [border: (1px inset #808080)] [r=2 c=1 rs=1 cs=1]
+                RenderBR {BR} at (6,6) size 0x18
+              RenderTableCell {TD} at (71,66) size 34x30 [border: (1px inset #808080)] [r=2 c=2 rs=1 cs=1]
+                RenderBR {BR} at (6,6) size 0x18
+            RenderTableRow {TR} at (0,98) size 107x30
+              RenderTableCell {TD} at (2,98) size 33x30 [border: (1px inset #808080)] [r=3 c=0 rs=1 cs=1]
+                RenderBR {BR} at (6,6) size 0x18
+              RenderTableCell {TD} at (37,98) size 32x30 [border: (1px inset #808080)] [r=3 c=1 rs=1 cs=1]
+                RenderBR {BR} at (6,6) size 0x18
+              RenderTableCell {TD} at (71,98) size 34x30 [border: (1px inset #808080)] [r=3 c=2 rs=1 cs=1]
+                RenderBR {BR} at (6,6) size 0x18
+caret: position 0 of child 0 {BR} of child 0 {TD} of child 0 {TR} of child 1 {TBODY} of child 1 {TABLE} of child 3 {DIV} of child 0 {BODY} of child 0 {HTML} of document
index 26825f0..373845a 100644 (file)
@@ -1,3 +1,16 @@
+2007-12-12  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Darin Adler.
+
+        <rdar://problem/5433862> Mail crashes at WebCore::highestAncestor() when deleting a particular selection
+
+        * editing/DeleteSelectionCommand.cpp:
+        (WebCore::DeleteSelectionCommand::removePreviouslySelectedEmptyTableRows):
+        Don't remove the table row that contained the end of the selection if it is where we are
+        about to place the ending selection.
+        Don't remove all empty rows after the row that contained the start of the selection,
+        they might come after the row that contained the end of the selection.
+
 2007-12-12  Sam Weinig  <sam@webkit.org>
 
         Reviewed by Anders Carlsson.
index 1a2f4b9..463d80c 100644 (file)
@@ -568,11 +568,9 @@ void DeleteSelectionCommand::mergeParagraphs()
 
 void DeleteSelectionCommand::removePreviouslySelectedEmptyTableRows()
 {
-    if (m_endTableRow && m_endTableRow->inDocument()) {
-        Node* row = m_endTableRow.get();
-        // Do not remove the row that contained the start of the selection,
-        // since it now contains the selection.
-        while (row && row != m_startTableRow.get()) {
+    if (m_endTableRow && m_endTableRow->inDocument() && m_endTableRow != m_startTableRow) {
+        Node* row = m_endTableRow->previousSibling();
+        while (row && row != m_startTableRow) {
             RefPtr<Node> previousRow = row->previousSibling();
             if (isTableRowEmpty(row))
                 // Use a raw removeNode, instead of DeleteSelectionCommand's, because
@@ -584,16 +582,25 @@ void DeleteSelectionCommand::removePreviouslySelectedEmptyTableRows()
     
     // Remove empty rows after the start row.
     if (m_startTableRow && m_startTableRow->inDocument() && m_startTableRow != m_endTableRow) {
-        // Do not remove the row that contained the start of the selection,
-        // since it now contains the selection.
         Node* row = m_startTableRow->nextSibling();
-        while (row) {
+        while (row && row != m_endTableRow) {
             RefPtr<Node> nextRow = row->nextSibling();
             if (isTableRowEmpty(row))
                 CompositeEditCommand::removeNode(row);
             row = nextRow.get();
         }
     }
+    
+    if (m_endTableRow && m_endTableRow->inDocument() && m_endTableRow != m_startTableRow)
+        if (isTableRowEmpty(m_endTableRow.get())) {
+            // Don't remove m_endTableRow if it's where we're putting the ending selection.
+            if (!m_endingPosition.node()->isDescendantOf(m_endTableRow.get())) {
+                // FIXME: We probably shouldn't remove m_endTableRow unless it's fully selected, even if it is empty.
+                // We'll need to start adjusting the selection endpoints during deletion to know whether or not m_endTableRow
+                // was fully selected here.
+                CompositeEditCommand::removeNode(m_endTableRow.get());
+            }
+        }
 }
 
 void DeleteSelectionCommand::calculateTypingStyleAfterDelete(Node *insertedPlaceholder)