LayoutTests:
authorjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Apr 2007 20:27:58 +0000 (20:27 +0000)
committerjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Apr 2007 20:27:58 +0000 (20:27 +0000)
        Reviewed by darin

        <rdar://problem/5136770>
        Gmail Editor: Hang when turning a particular multi-line selection into a list

        Demonstrates the bug:
        * editing/execCommand/5136770-expected.checksum: Added.
        * editing/execCommand/5136770-expected.png: Added.
        * editing/execCommand/5136770-expected.txt: Added.
        * editing/execCommand/5136770.html: Added.

        Added placeholders in empty list items:
        * editing/execCommand/4747450-expected.txt:
        * editing/execCommand/insert-list-empty-div-expected.txt:
        * editing/execCommand/4917055-expected.txt:

        A horizontal rule pushed into a list item appears *before*
        the list marker in the render tree, although, the new results
        look more correct (13376):
        * editing/execCommand/create-list-with-hr-expected.checksum:
        * editing/execCommand/create-list-with-hr-expected.png:
        * editing/execCommand/create-list-with-hr-expected.txt:
        * editing/execCommand/create-list-with-hr.html:

WebCore:

        Reviewed by darin

        <rdar://problem/5136770>
        Gmail Editor: Hang when turning a particular multi-line selection into a list

        When InsertListCommand pushes content into list items,
        it creates an empty list item and then calls moveParagraphs.
        But moveParagraphs' selection preservation code fails when
        it encounters empty list items (list items w/o placeholders).
        This causes InsertListCommand to lose track of where it has
        already been performed, which causes the hang.

        * editing/InsertListCommand.cpp:
        (WebCore::InsertListCommand::doApply): Use brs to hold open
        empty list items.

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

14 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/execCommand/4747450-expected.txt
LayoutTests/editing/execCommand/4917055-expected.txt
LayoutTests/editing/execCommand/5136770-expected.checksum [new file with mode: 0644]
LayoutTests/editing/execCommand/5136770-expected.png [new file with mode: 0644]
LayoutTests/editing/execCommand/5136770-expected.txt [new file with mode: 0644]
LayoutTests/editing/execCommand/5136770.html [new file with mode: 0644]
LayoutTests/editing/execCommand/create-list-with-hr-expected.checksum
LayoutTests/editing/execCommand/create-list-with-hr-expected.png
LayoutTests/editing/execCommand/create-list-with-hr-expected.txt
LayoutTests/editing/execCommand/create-list-with-hr.html
LayoutTests/editing/execCommand/insert-list-empty-div-expected.txt
WebCore/ChangeLog
WebCore/editing/InsertListCommand.cpp

index d09cdab..6cce4ed 100644 (file)
@@ -1,3 +1,30 @@
+2007-04-17  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by darin
+        
+        <rdar://problem/5136770> 
+        Gmail Editor: Hang when turning a particular multi-line selection into a list
+
+        Demonstrates the bug:
+        * editing/execCommand/5136770-expected.checksum: Added.
+        * editing/execCommand/5136770-expected.png: Added.
+        * editing/execCommand/5136770-expected.txt: Added.
+        * editing/execCommand/5136770.html: Added.
+        
+        Added placeholders in empty list items:
+        * editing/execCommand/4747450-expected.txt:
+        * editing/execCommand/insert-list-empty-div-expected.txt:
+        * editing/execCommand/4917055-expected.txt:
+        
+        A horizontal rule pushed into a list item appears *before*
+        the list marker in the render tree, although, the new results 
+        look more correct (13376):
+        * editing/execCommand/create-list-with-hr-expected.checksum:
+        * editing/execCommand/create-list-with-hr-expected.png:
+        * editing/execCommand/create-list-with-hr-expected.txt:
+        * editing/execCommand/create-list-with-hr.html:
+
+
 2007-04-16  Justin Garcia  <justin.garcia@apple.com>
 
         Reviewed by darin
index 5c13a00..9e6e8f5 100644 (file)
@@ -21,5 +21,6 @@ layer at (0,0) size 800x600
           RenderBlock {UL} at (0,0) size 784x18
             RenderListItem {LI} at (40,0) size 744x18
               RenderListMarker at (-17,0) size 7x18: bullet
+              RenderBR {BR} at (0,0) size 0x18
           RenderBlock (anonymous) at (0,34) size 784x0
-caret: position 0 of child 0 {LI} of child 0 {UL} of child 1 {DIV} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
+caret: position 0 of child 0 {BR} of child 0 {LI} of child 0 {UL} of child 1 {DIV} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/execCommand/5136770-expected.checksum b/LayoutTests/editing/execCommand/5136770-expected.checksum
new file mode 100644 (file)
index 0000000..7e35317
--- /dev/null
@@ -0,0 +1 @@
+0b0b499114c6e2e7387c98c53e2aeee2
\ No newline at end of file
diff --git a/LayoutTests/editing/execCommand/5136770-expected.png b/LayoutTests/editing/execCommand/5136770-expected.png
new file mode 100644 (file)
index 0000000..5bd1667
Binary files /dev/null and b/LayoutTests/editing/execCommand/5136770-expected.png differ
diff --git a/LayoutTests/editing/execCommand/5136770-expected.txt b/LayoutTests/editing/execCommand/5136770-expected.txt
new file mode 100644 (file)
index 0000000..6438473
--- /dev/null
@@ -0,0 +1,27 @@
+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 784x576
+      RenderBlock {P} at (0,0) size 784x18
+        RenderText {#text} at (0,0) size 375x18
+          text run at (0,0) width 375: "This tests for a hang when performing InsertUnorderedList."
+      RenderBlock {DIV} at (0,34) size 784x88
+        RenderBlock {DIV} at (0,0) size 784x18
+          RenderText {#text} at (0,0) size 347x18
+            text run at (0,0) width 347: "There should be two empty unordered list items below."
+        RenderBlock {DIV} at (0,34) size 784x54
+          RenderBlock {UL} at (0,0) size 784x54
+            RenderListItem {LI} at (40,0) size 744x18
+              RenderListMarker at (-17,0) size 7x18: bullet
+              RenderBR {BR} at (0,0) size 0x18
+            RenderListItem {LI} at (40,18) size 744x18
+              RenderListMarker at (-17,0) size 7x18: bullet
+              RenderBR {BR} at (0,0) size 0x18
+            RenderListItem {LI} at (40,36) size 744x18
+              RenderListMarker at (-17,0) size 7x18: bullet
+              RenderText {#text} at (0,0) size 237x18
+                text run at (0,0) width 237: "This should be an unordered list item."
+          RenderBlock (anonymous) at (0,70) size 784x0
+selection start: position 0 of child 0 {BR} of child 0 {LI} of child 0 {UL} of child 3 {DIV} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
+selection end:   position 38 of child 0 {#text} of child 2 {LI} of child 0 {UL} of child 3 {DIV} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/execCommand/5136770.html b/LayoutTests/editing/execCommand/5136770.html
new file mode 100644 (file)
index 0000000..69fd3c8
--- /dev/null
@@ -0,0 +1,15 @@
+<p>This tests for a hang when performing InsertUnorderedList.</p>
+<div contenteditable="true">
+<div>There should be two empty unordered list items below.</div>
+<div id="start"><br></div>
+<div><br></div>
+<div id="end">This should be an unordered list item.</div>
+</div>
+
+<script>
+var selection = window.getSelection();
+var div = document.getElementById("div");
+selection.setBaseAndExtent(start, 0, end, end.childNodes.length);
+
+document.execCommand("InsertUnorderedList");
+</script>
index 5799aa2..24cba8a 100644 (file)
Binary files a/LayoutTests/editing/execCommand/create-list-with-hr-expected.png and b/LayoutTests/editing/execCommand/create-list-with-hr-expected.png differ
index 388f1dc..d320d21 100644 (file)
@@ -9,14 +9,21 @@ layer at (0,0) size 800x600
 layer at (0,0) size 800x600
   RenderBlock {HTML} at (0,0) size 800x600
     RenderBody {BODY} at (8,8) size 784x576
-      RenderBlock {P} at (0,0) size 784x18
-        RenderText {#text} at (0,0) size 510x18
-          text run at (0,0) width 510: "This test pushes a horizontal rule into an unordered list with InsertUnorderedList."
-      RenderBlock {DIV} at (0,34) size 784x28
+      RenderBlock {P} at (0,0) size 784x36
+        RenderText {#text} at (0,0) size 514x18
+          text run at (0,0) width 514: "This test pushes a horizontal rule into an unordered list with InsertUnorderedList. "
+        RenderInline {B} at (0,0) size 751x36
+          RenderText {#text} at (514,0) size 751x36
+            text run at (514,0) width 237: "This demonstrates what might be a"
+            text run at (0,18) width 31: "bug:"
+        RenderText {#text} at (31,18) size 421x18
+          text run at (31,18) width 421: " the horizontal rule appears before the list marker in the render tree."
+      RenderBlock {DIV} at (0,52) size 784x28
         RenderBlock {UL} at (0,0) size 784x28
           RenderListItem {LI} at (40,0) size 744x28
-            RenderBlock (anonymous) at (0,0) size 744x18
+            RenderBlock (anonymous) at (0,0) size 744x0
+            RenderBlock {HR} at (0,0) size 744x2 [border: (1px inset #000000)]
+            RenderBlock (anonymous) at (0,10) size 744x18
               RenderListMarker at (-17,0) size 7x18: bullet
-            RenderBlock {HR} at (0,26) size 744x2 [border: (1px inset #000000)]
         RenderBlock (anonymous) at (0,44) size 784x0
 caret: position 0 of child 0 {HR} of child 0 {LI} of child 0 {UL} of child 2 {DIV} of child 1 {BODY} of child 0 {HTML} of document
index c42f27a..40678bc 100644 (file)
@@ -2,7 +2,7 @@
 if (window.layoutTestController)
      layoutTestController.dumpEditingCallbacks();
 </script>
-<p>This test pushes a horizontal rule into an unordered list with InsertUnorderedList.</p>
+<p>This test pushes a horizontal rule into an unordered list with InsertUnorderedList. <b>This demonstrates what might be a bug:</b> the horizontal rule appears before the list marker in the render tree.</p>
 <div contenteditable="true" id="div"><hr></div>
 
 <script>
@@ -11,4 +11,4 @@ var sel = window.getSelection();
 
 sel.setPosition(div, 0);
 document.execCommand("InsertUnorderedList");
-</script>
\ No newline at end of file
+</script>
index 6b67f4b..346f919 100644 (file)
@@ -11,5 +11,6 @@ layer at (0,0) size 800x600
         RenderBlock {UL} at (0,0) size 784x18
           RenderListItem {LI} at (40,0) size 744x18
             RenderListMarker at (-17,0) size 7x18: bullet
+            RenderBR {BR} at (0,0) size 0x18
         RenderBlock (anonymous) at (0,34) size 784x0
-caret: position 0 of child 0 {LI} of child 0 {UL} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
+caret: position 0 of child 0 {BR} of child 0 {LI} of child 0 {UL} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
index 81c3180..89799f5 100644 (file)
@@ -1,3 +1,21 @@
+2007-04-17  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by darin
+
+        <rdar://problem/5136770> 
+        Gmail Editor: Hang when turning a particular multi-line selection into a list
+        
+        When InsertListCommand pushes content into list items, 
+        it creates an empty list item and then calls moveParagraphs.  
+        But moveParagraphs' selection preservation code fails when 
+        it encounters empty list items (list items w/o placeholders).
+        This causes InsertListCommand to lose track of where it has 
+        already been performed, which causes the hang.
+
+        * editing/InsertListCommand.cpp:
+        (WebCore::InsertListCommand::doApply): Use brs to hold open 
+        empty list items.
+
 2007-04-17  Darin Adler  <darin@apple.com>
 
         Oops, rolled that last change out. I'll redo it again later after making
index 4da953b..78b9ec1 100644 (file)
@@ -170,6 +170,8 @@ void InsertListCommand::doApply()
         VisiblePosition previousPosition = start.previous(true);
         VisiblePosition nextPosition = end.next(true);
         RefPtr<Element> listItemElement = createListItemElement(document());
+        RefPtr<Element> placeholder = createBreakElement(document());
+        appendNode(placeholder.get(), listItemElement.get());
         Node* previousList = outermostEnclosingList(previousPosition.deepEquivalent().node());
         Node* nextList = outermostEnclosingList(nextPosition.deepEquivalent().node());
         if (previousList && !previousList->hasTagName(listTag))
@@ -198,7 +200,7 @@ void InsertListCommand::doApply()
             
             insertNodeAt(listElement.get(), start.deepEquivalent().node(), start.deepEquivalent().offset());
         }
-        moveParagraph(start, end, VisiblePosition(Position(listItemElement.get(), 0)), true);
+        moveParagraph(start, end, VisiblePosition(Position(placeholder.get(), 0)), true);
         if (nextList && previousList)
             mergeIdenticalElements(static_cast<Element*>(previousList), static_cast<Element*>(nextList));
     }