WebCore:
authorjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 25 Aug 2007 01:20:33 +0000 (01:20 +0000)
committerjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 25 Aug 2007 01:20:33 +0000 (01:20 +0000)
        Reviewed by Darin.

        <rdar://problem/5432254> GoogleDocs: A hang occurs when applying list style to selected table

        * editing/DeleteSelectionCommand.cpp:
        (WebCore::DeleteSelectionCommand::handleGeneralDelete): If the position
        that marked the start of the range to delete has been removed from the
        document, and it was inside the node that holds the position that marks
        the end of the range to delete, don't remove any children of that node,
        because we don't know how many to remove.  For example, if the end is
        [a, 5] and the start was in some descendant of a and was removed, don't
        remove any of the children of a.  We will now refuse to remove some content
        incorrectly, but that's less dangerous than removing content incorrectly.
        Long term we need to update these positions as we remove content from the
        document, but that seems like a more risky change.  Added a testcase.
        * editing/InsertListCommand.cpp:
        (WebCore::InsertListCommand::modifyRange): If the end of the selection to
        modify is just after a table, and if the start of the selection is inside
        that table, the last paragraph that we'll want modify is the last one inside
        the table, not the paragraph that contains the table itself. Adjust
        startOfLastParagraph here to avoid infinite recursion.

LayoutTests:

        Reviewed by Darin.

        <rdar://problem/5432254> GoogleDocs: A hang occurs when applying list style to selected table

        * editing/execCommand/5432254-1.html: Added.
        * editing/execCommand/5432254-2.html: Added.
        * platform/mac/editing/execCommand: Added.
        * platform/mac/editing/execCommand/5432254-1-expected.checksum: Added.
        * platform/mac/editing/execCommand/5432254-1-expected.png: Added.
        * platform/mac/editing/execCommand/5432254-1-expected.txt: Added.
        * platform/mac/editing/execCommand/5432254-2-expected.checksum: Added.
        * platform/mac/editing/execCommand/5432254-2-expected.png: Added.
        * platform/mac/editing/execCommand/5432254-2-expected.txt: Added.

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

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

index af1b4f4f0ed095b7f3eb3cd78671f1de8703d76d..65fb80e0f839815c2f1e8e3ed41e50ee4e7009e7 100644 (file)
@@ -1,3 +1,19 @@
+2007-08-24  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Darin.
+        
+        <rdar://problem/5432254> GoogleDocs: A hang occurs when applying list style to selected table
+
+        * editing/execCommand/5432254-1.html: Added.
+        * editing/execCommand/5432254-2.html: Added.
+        * platform/mac/editing/execCommand: Added.
+        * platform/mac/editing/execCommand/5432254-1-expected.checksum: Added.
+        * platform/mac/editing/execCommand/5432254-1-expected.png: Added.
+        * platform/mac/editing/execCommand/5432254-1-expected.txt: Added.
+        * platform/mac/editing/execCommand/5432254-2-expected.checksum: Added.
+        * platform/mac/editing/execCommand/5432254-2-expected.png: Added.
+        * platform/mac/editing/execCommand/5432254-2-expected.txt: Added.
+
 2007-08-24  Anders Carlsson  <andersca@apple.com>
 
         Reviewed by Geoff.
diff --git a/LayoutTests/platform/mac/editing/execCommand/5432254-1-expected.checksum b/LayoutTests/platform/mac/editing/execCommand/5432254-1-expected.checksum
new file mode 100644 (file)
index 0000000..ce3a6d2
--- /dev/null
@@ -0,0 +1 @@
+d4c893a03c062eb91ff0a04f0628f65f
\ No newline at end of file
diff --git a/LayoutTests/platform/mac/editing/execCommand/5432254-1-expected.png b/LayoutTests/platform/mac/editing/execCommand/5432254-1-expected.png
new file mode 100644 (file)
index 0000000..87079ce
Binary files /dev/null and b/LayoutTests/platform/mac/editing/execCommand/5432254-1-expected.png differ
diff --git a/LayoutTests/platform/mac/editing/execCommand/5432254-1-expected.txt b/LayoutTests/platform/mac/editing/execCommand/5432254-1-expected.txt
new file mode 100644 (file)
index 0000000..71a70ec
--- /dev/null
@@ -0,0 +1,22 @@
+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 778x36
+          text run at (0,0) width 182: "This tests for a deletion bug. "
+          text run at (182,0) width 390: "Only the second paragraph inside the table should be deleted. "
+          text run at (572,0) width 206: "You should see a table with 'foo'"
+          text run at (0,18) width 71: "in it below."
+      RenderBlock {DIV} at (0,52) size 784x46
+        RenderTable {TABLE} at (0,0) size 31x46 [border: (1px outset #808080)]
+          RenderTableSection {TBODY} at (1,1) size 29x44
+            RenderTableRow {TR} at (0,2) size 29x40
+              RenderTableCell {TD} at (2,2) size 25x40 [border: (1px inset #808080)] [r=0 c=0 rs=1 cs=1]
+                RenderBlock {DIV} at (2,2) size 21x18
+                  RenderText {#text} at (0,0) size 21x18
+                    text run at (0,0) width 21: "foo"
+                RenderBlock (anonymous) at (2,20) size 21x18
+                  RenderBR {BR} at (0,0) size 0x18
+caret: position 0 of child 1 {BR} of child 0 {TD} of child 0 {TR} of child 0 {TBODY} of child 0 {TABLE} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/platform/mac/editing/execCommand/5432254-2-expected.checksum b/LayoutTests/platform/mac/editing/execCommand/5432254-2-expected.checksum
new file mode 100644 (file)
index 0000000..2052744
--- /dev/null
@@ -0,0 +1 @@
+ab5114af2304436556dc70e0f32ad275
\ No newline at end of file
diff --git a/LayoutTests/platform/mac/editing/execCommand/5432254-2-expected.png b/LayoutTests/platform/mac/editing/execCommand/5432254-2-expected.png
new file mode 100644 (file)
index 0000000..65c6888
Binary files /dev/null and b/LayoutTests/platform/mac/editing/execCommand/5432254-2-expected.png differ
diff --git a/LayoutTests/platform/mac/editing/execCommand/5432254-2-expected.txt b/LayoutTests/platform/mac/editing/execCommand/5432254-2-expected.txt
new file mode 100644 (file)
index 0000000..0641428
--- /dev/null
@@ -0,0 +1,25 @@
+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 665x18
+          text run at (0,0) width 182: "This tests for a deletion bug. "
+          text run at (182,0) width 483: "The paragraph inside the table should be in a list, and the test shouldn't hang."
+      RenderBlock {DIV} at (0,34) size 784x62
+        RenderBlock {DIV} at (0,0) size 784x44
+          RenderTable {TABLE} at (0,0) size 71x44 [border: (1px outset #808080)]
+            RenderTableSection {TBODY} at (1,1) size 69x42
+              RenderTableRow {TR} at (0,2) size 69x38
+                RenderTableCell {TD} at (2,2) size 65x38 [border: (1px inset #808080)] [r=0 c=0 rs=1 cs=1]
+                  RenderBlock {UL} at (2,2) size 61x18
+                    RenderListItem {LI} at (40,0) size 21x18
+                      RenderListMarker at (-17,0) size 7x18: bullet
+                      RenderText {#text} at (0,0) size 21x18
+                        text run at (0,0) width 21: "foo"
+                  RenderBlock (anonymous) at (2,36) size 61x0
+        RenderBlock (anonymous) at (0,44) size 784x18
+          RenderBR {BR} at (0,0) size 0x18
+selection start: position 0 of child 0 {TABLE} of child 0 {DIV} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
+selection end:   position 1 of child 0 {TABLE} of child 0 {DIV} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
index a2328f681f9a265b8cdb38d7a40dc2fbc4b0d7a0..304a36ba8858207702ff9b09e0670fc0789af34f 100644 (file)
@@ -1,3 +1,27 @@
+2007-08-23  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Darin.
+        
+        <rdar://problem/5432254> GoogleDocs: A hang occurs when applying list style to selected table
+        
+        * editing/DeleteSelectionCommand.cpp:
+        (WebCore::DeleteSelectionCommand::handleGeneralDelete): If the position
+        that marked the start of the range to delete has been removed from the
+        document, and it was inside the node that holds the position that marks
+        the end of the range to delete, don't remove any children of that node,
+        because we don't know how many to remove.  For example, if the end is
+        [a, 5] and the start was in some descendant of a and was removed, don't
+        remove any of the children of a.  We will now refuse to remove some content
+        incorrectly, but that's less dangerous than removing content incorrectly.
+        Long term we need to update these positions as we remove content from the 
+        document, but that seems like a more risky change.  Added a testcase.
+        * editing/InsertListCommand.cpp:
+        (WebCore::InsertListCommand::modifyRange): If the end of the selection to 
+        modify is just after a table, and if the start of the selection is inside 
+        that table, the last paragraph that we'll want modify is the last one inside 
+        the table, not the paragraph that contains the table itself. Adjust 
+        startOfLastParagraph here to avoid infinite recursion.
+
 2007-08-24  Anders Carlsson  <andersca@apple.com>
 
         Reviewed by Geoff.
index a6bf480a17b6850ba2ba4793d4c0a1260239daee..faf8562e3802bdce875c9071ece46142e7e59a73 100644 (file)
@@ -404,8 +404,9 @@ void DeleteSelectionCommand::handleGeneralDelete()
         }
     }
     else {
+        bool startNodeWasDescendantOfEndNode = m_upstreamStart.node()->isDescendantOf(m_downstreamEnd.node());
         // The selection to delete spans more than one node.
-        RefPtr<Node> node = startNode;
+        RefPtr<Node> node(startNode);
         
         if (startOffset > 0) {
             if (startNode->isTextNode()) {
@@ -456,7 +457,13 @@ void DeleteSelectionCommand::handleGeneralDelete()
                         deleteTextFromNode(text, 0, m_downstreamEnd.offset());
                         m_downstreamEnd = Position(text, 0);
                     }
-                } else {
+                // Remove children of m_downstreamEnd.node() that come after m_upstreamStart.
+                // Don't try to remove children if m_upstreamStart was inside m_downstreamEnd.node()
+                // and m_upstreamStart has been removed from the document, because then we don't 
+                // know how many children to remove.
+                // FIXME: Make m_upstreamStart a position we update as we remove content, then we can
+                // always know which children to remove.
+                } else if (!(startNodeWasDescendantOfEndNode && !m_upstreamStart.node()->inDocument())) {
                     int offset = 0;
                     if (m_upstreamStart.node()->isDescendantOf(m_downstreamEnd.node())) {
                         Node *n = m_upstreamStart.node();
index 063580bcd0195115cdde28aa610327532bf4f3ac..093b637fa1fd12e8f583ac94b1eeccad1d7de809 100644 (file)
@@ -65,6 +65,14 @@ bool InsertListCommand::modifyRange()
     VisiblePosition visibleEnd = endingSelection().visibleEnd();
     VisiblePosition startOfLastParagraph = startOfParagraph(visibleEnd);
     
+    // If the end of the selection to modify is just after a table, and
+    // if the start of the selection is inside that table, the last paragraph
+    // that we'll want modify is the last one inside the table, not the table itself.
+    // Adjust startOfLastParagraph here to avoid infinite recursion.
+    if (Node* table = isFirstPositionAfterTable(visibleEnd))
+        if (visibleStart.deepEquivalent().node()->isDescendantOf(table))
+            startOfLastParagraph = startOfParagraph(visibleEnd.previous(true));
+        
     if (startOfParagraph(visibleStart) == startOfLastParagraph)
         return false;