LayoutTests:
authorjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Oct 2006 19:27:19 +0000 (19:27 +0000)
committerjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 18 Oct 2006 19:27:19 +0000 (19:27 +0000)
        Reviewed by harrison

        <rdar://problem/4784823>
        GMail Editor: Hang occurs when removing list styling on text in a rich text message

        * editing/execCommand/remove-list-1-expected.checksum: Added.
        * editing/execCommand/remove-list-1-expected.png: Added.
        * editing/execCommand/remove-list-1-expected.txt: Added.
        * editing/execCommand/remove-list-1.html: Added.

WebCore:

        Reviewed by harrison

        <rdar://problem/4784823>
        GMail Editor: Hang occurs when removing list styling on text in a rich text message

        List removal moves the contents of every list item out of the list it's
        in. When the code tried to move the contents of an empty list item (an li
        with no child nodes, not even a placeholder br), moveParagraph didn't prune
        the li, like it would if the li had a placeholder inside it.  So the list
        removal code went into an infinite loop, continually attempting to de-list
        an empty list item.

        * editing/CompositeEditCommand.cpp:
        (WebCore::CompositeEditCommand::moveParagraphs): If the selection
        to move was empty and in an empty block that doesn't require a
        placeholder to prop itself open (like a bordered div or an li),
        remove it during the move.

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

LayoutTests/ChangeLog
LayoutTests/editing/execCommand/remove-list-1-expected.checksum [new file with mode: 0644]
LayoutTests/editing/execCommand/remove-list-1-expected.png [new file with mode: 0644]
LayoutTests/editing/execCommand/remove-list-1-expected.txt [new file with mode: 0644]
LayoutTests/editing/execCommand/remove-list-1.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/editing/CompositeEditCommand.cpp

index 9862b9f..453f53d 100644 (file)
@@ -1,3 +1,15 @@
+2006-10-18  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by harrison
+
+        <rdar://problem/4784823>
+        GMail Editor: Hang occurs when removing list styling on text in a rich text message
+
+        * editing/execCommand/remove-list-1-expected.checksum: Added.
+        * editing/execCommand/remove-list-1-expected.png: Added.
+        * editing/execCommand/remove-list-1-expected.txt: Added.
+        * editing/execCommand/remove-list-1.html: Added.
+
 2006-10-17  Justin Garcia  <justin.garcia@apple.com>
 
         Reviewed by harrison
diff --git a/LayoutTests/editing/execCommand/remove-list-1-expected.checksum b/LayoutTests/editing/execCommand/remove-list-1-expected.checksum
new file mode 100644 (file)
index 0000000..9aebb0e
--- /dev/null
@@ -0,0 +1 @@
+958726dee6c5c5c73089d158c46f17c5
\ No newline at end of file
diff --git a/LayoutTests/editing/execCommand/remove-list-1-expected.png b/LayoutTests/editing/execCommand/remove-list-1-expected.png
new file mode 100644 (file)
index 0000000..0cf1c88
Binary files /dev/null and b/LayoutTests/editing/execCommand/remove-list-1-expected.png differ
diff --git a/LayoutTests/editing/execCommand/remove-list-1-expected.txt b/LayoutTests/editing/execCommand/remove-list-1-expected.txt
new file mode 100644 (file)
index 0000000..520d167
--- /dev/null
@@ -0,0 +1,26 @@
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 1 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of LI > OL > DIV > BODY > HTML > #document to 49 of #text > LI > OL > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of LI > OL > DIV > BODY > HTML > #document to 49 of #text > LI > OL > DIV > BODY > HTML > #document toDOMRange:range from 0 of LI > OL > DIV > BODY > HTML > #document to 49 of #text > LI > OL > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+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 179x18
+          text run at (0,0) width 179: "This tests de-listing content. "
+        RenderInline {B} at (0,0) size 768x36
+          RenderText {#text} at (179,0) size 768x36
+            text run at (179,0) width 589: "This demonstrates a bug: the entire editable region should be selected (selections should"
+            text run at (0,18) width 324: "be preserved when performing a list command)."
+      RenderBlock {DIV} at (0,52) size 784x36
+        RenderBR {BR} at (0,0) size 0x18
+        RenderText {#text} at (0,18) size 322x18
+          text run at (0,18) width 322: "There should be a empty paragraph above this one."
+caret: position 0 of child 0 {BR} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/execCommand/remove-list-1.html b/LayoutTests/editing/execCommand/remove-list-1.html
new file mode 100644 (file)
index 0000000..f8ea20d
--- /dev/null
@@ -0,0 +1,11 @@
+<p>This tests de-listing content. <b>This demonstrates a bug: the entire editable region should be selected (selections should be preserved when performing a list command).</b></p>
+<div id="div" contenteditable="true"><ol><li></li><li>There should be a empty paragraph above this one.</li></ol></div>
+
+<script>
+var div = document.getElementById("div");
+var sel = window.getSelection();
+
+div.focus();
+document.execCommand("SelectAll");
+document.execCommand("InsertOrderedList");
+</script>
index 04d5d03..728b4b2 100644 (file)
@@ -1,3 +1,23 @@
+2006-10-18  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by harrison
+        
+        <rdar://problem/4784823>
+        GMail Editor: Hang occurs when removing list styling on text in a rich text message
+
+        List removal moves the contents of every list item out of the list it's 
+        in. When the code tried to move the contents of an empty list item (an li 
+        with no child nodes, not even a placeholder br), moveParagraph didn't prune 
+        the li, like it would if the li had a placeholder inside it.  So the list 
+        removal code went into an infinite loop, continually attempting to de-list 
+        an empty list item.
+
+        * editing/CompositeEditCommand.cpp:
+        (WebCore::CompositeEditCommand::moveParagraphs): If the selection
+        to move was empty and in an empty block that doesn't require a 
+        placeholder to prop itself open (like a bordered div or an li), 
+        remove it during the move.
+
 2006-10-18  Adele Peterson  <adele@apple.com>
 
         Reviewed by Beth.
index ebf5725..1a3195e 100644 (file)
@@ -661,15 +661,26 @@ void CompositeEditCommand::moveParagraphs(const VisiblePosition& startOfParagrap
 
     ASSERT(destination.deepEquivalent().node()->inDocument());
     
-    // There are bugs in deletion when it removes a fully selected table/list.  It expands and removes the entire table/list, but will let content
+    // There are bugs in deletion when it removes a fully selected table/list.  
+    // It expands and removes the entire table/list, but will let content
     // before and after the table/list collapse onto one line.
+    
+    // Deleting a paragraph will leave a placeholder.  Remove it (and prune
+    // empty or unrendered parents).
     VisiblePosition caretAfterDelete = endingSelection().visibleStart();
     if (isStartOfParagraph(caretAfterDelete) && isEndOfParagraph(caretAfterDelete)) {
         // Note: We want the rightmost candidate.
         Position position = caretAfterDelete.deepEquivalent().downstream();
         Node* node = position.node();
+        // Normally deletion will leave a br as a placeholder.
         if (node->hasTagName(brTag))
             removeNodeAndPruneAncestors(node);
+        // If the selection to move was empty and in an empty block that 
+        // doesn't require a placeholder to prop itself open (like a bordered 
+        // div or an li), remove it during the move (the list removal code 
+        // expects this behavior).
+        else if (isBlock(node))
+            removeNodeAndPruneAncestors(node);
         else if (node->renderer() && node->renderer()->style()->preserveNewline() && caretAfterDelete.characterAfter() == '\n')
             deleteTextFromNode(static_cast<Text*>(node), position.offset(), 1);
     }