LayoutTests:
authorjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Jul 2006 23:38:26 +0000 (23:38 +0000)
committerjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Jul 2006 23:38:26 +0000 (23:38 +0000)
        Reviewed by levi

        <rdar://problem/4548238>
        REGRESSION: Can't remove the first OL/UL list item in a Mail's compose window

        * editing/deleting/delete-first-list-item-expected.checksum: Added.
        * editing/deleting/delete-first-list-item-expected.png: Added.
        * editing/deleting/delete-first-list-item-expected.txt: Added.
        * editing/deleting/delete-first-list-item.html: Added.

WebCore:

        Reviewed by levi

        <rdar://problem/4548238>
        REGRESSION: Can't remove the first OL/UL list item in a Mail's compose window

        * editing/CompositeEditCommand.cpp:
        (WebCore::CompositeEditCommand::breakOutOfEmptyListItem):
        Moved from InsertParagraphSeparator.  Does its own typing style restoration.
        * editing/CompositeEditCommand.h:
        * editing/InsertParagraphSeparatorCommand.cpp:
        (WebCore::InsertParagraphSeparatorCommand::doApply): Call breakOutOfEmptyListItem.
        * editing/TypingCommand.cpp:
        (WebCore::TypingCommand::deleteKeyPressed): Call breakOutOfEmptyListItem if
        the endingSelection is at the start of an editable region.
        * editing/htmlediting.cpp:
        (WebCore::embeddedSublist): Moved from InsertParagraphSeparator.
        (WebCore::appendedSublist): Ditto.
        (WebCore::enclosingEmptyListItem): Ditto.
        * editing/htmlediting.h:

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

14 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/deleting/delete-first-list-item-expected.checksum [new file with mode: 0644]
LayoutTests/editing/deleting/delete-first-list-item-expected.png [new file with mode: 0644]
LayoutTests/editing/deleting/delete-first-list-item-expected.txt [new file with mode: 0644]
LayoutTests/editing/deleting/delete-first-list-item.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/dom/NodeIterator.cpp
WebCore/editing/CompositeEditCommand.cpp
WebCore/editing/CompositeEditCommand.h
WebCore/editing/DeleteSelectionCommand.cpp
WebCore/editing/InsertParagraphSeparatorCommand.cpp
WebCore/editing/TypingCommand.cpp
WebCore/editing/htmlediting.cpp
WebCore/editing/htmlediting.h

index cd8341a7fbb729c20a4dcd4e0f6573870e03da7c..41dd5fb131fcfe41e0b725111532acc041bfc112 100644 (file)
@@ -1,3 +1,15 @@
+2006-07-21  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by levi
+        
+        <rdar://problem/4548238>
+        REGRESSION: Can't remove the first OL/UL list item in a Mail's compose window
+
+        * editing/deleting/delete-first-list-item-expected.checksum: Added.
+        * editing/deleting/delete-first-list-item-expected.png: Added.
+        * editing/deleting/delete-first-list-item-expected.txt: Added.
+        * editing/deleting/delete-first-list-item.html: Added.
+
 2006-07-21  Mitz Pettel  <opendarwin.org@mitzpettel.com>
 
         Reviewed by Darin.
diff --git a/LayoutTests/editing/deleting/delete-first-list-item-expected.checksum b/LayoutTests/editing/deleting/delete-first-list-item-expected.checksum
new file mode 100644 (file)
index 0000000..fc67fd3
--- /dev/null
@@ -0,0 +1 @@
+f84e35328c9b8b8c25ed9de8be9be9fb
\ No newline at end of file
diff --git a/LayoutTests/editing/deleting/delete-first-list-item-expected.png b/LayoutTests/editing/deleting/delete-first-list-item-expected.png
new file mode 100644 (file)
index 0000000..d06190e
Binary files /dev/null and b/LayoutTests/editing/deleting/delete-first-list-item-expected.png differ
diff --git a/LayoutTests/editing/deleting/delete-first-list-item-expected.txt b/LayoutTests/editing/deleting/delete-first-list-item-expected.txt
new file mode 100644 (file)
index 0000000..47a420b
--- /dev/null
@@ -0,0 +1,35 @@
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 1 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 3 of #text > B > DIV > DIV > BODY > HTML > #document to 3 of #text > B > DIV > 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 784x576
+      RenderBlock {P} at (0,0) size 784x36
+        RenderText {#text} at (0,0) size 768x36
+          text run at (0,0) width 768: "This tests deletion when the caret is in an empty list item at the beginning of the document. It also makes sure that the style"
+          text run at (0,18) width 232: "inside the empty list item is retained. "
+          text run at (232,18) width 105: "You should see '"
+        RenderInline {B} at (0,0) size 21x18
+          RenderText {#text} at (337,18) size 21x18
+            text run at (337,18) width 21: "foo"
+        RenderText {#text} at (358,18) size 251x18
+          text run at (358,18) width 251: "' followed by a list item containing 'bar'."
+      RenderBlock {DIV} at (0,52) size 784x52
+        RenderBlock {DIV} at (0,0) size 784x18
+          RenderInline {B} at (0,0) size 21x18
+            RenderText {#text} at (0,0) size 21x18
+              text run at (0,0) width 21: "foo"
+        RenderBlock {UL} at (0,34) size 784x18
+          RenderListItem {LI} at (40,0) size 744x18
+            RenderListMarker at (-17,0) size 7x18
+            RenderText {#text} at (0,0) size 20x18
+              text run at (0,0) width 20: "bar"
+caret: position 3 of child 0 {#text} of child 0 {B} of child 0 {DIV} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/deleting/delete-first-list-item.html b/LayoutTests/editing/deleting/delete-first-list-item.html
new file mode 100644 (file)
index 0000000..5a894cf
--- /dev/null
@@ -0,0 +1,11 @@
+<p>This tests deletion when the caret is in an empty list item at the beginning of the document. It also makes sure that the style inside the empty list item is retained.  You should see '<b>foo</b>' followed by a list item containing 'bar'.</p>
+<div id="div" contenteditable="true"><ul><li id="start" style="font-weight:bold;"></li><li>bar</li></ul></div>
+
+<script>
+var s = window.getSelection();
+var e = document.getElementById("start");
+
+s.setPosition(e, 0);
+document.execCommand("Delete");
+document.execCommand("InsertText", false, "foo");
+</script>
\ No newline at end of file
index afcaca5e74ed8443ed1c1c7832d78984a7956b62..cfad075d8227ca7ef661a0a87f938873038f7057 100644 (file)
@@ -1,3 +1,25 @@
+2006-07-21  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by levi
+        
+        <rdar://problem/4548238>
+        REGRESSION: Can't remove the first OL/UL list item in a Mail's compose window
+
+        * editing/CompositeEditCommand.cpp:
+        (WebCore::CompositeEditCommand::breakOutOfEmptyListItem): 
+        Moved from InsertParagraphSeparator.  Does its own typing style restoration.
+        * editing/CompositeEditCommand.h:
+        * editing/InsertParagraphSeparatorCommand.cpp:
+        (WebCore::InsertParagraphSeparatorCommand::doApply): Call breakOutOfEmptyListItem.
+        * editing/TypingCommand.cpp:
+        (WebCore::TypingCommand::deleteKeyPressed): Call breakOutOfEmptyListItem if
+        the endingSelection is at the start of an editable region.
+        * editing/htmlediting.cpp:
+        (WebCore::embeddedSublist): Moved from InsertParagraphSeparator.
+        (WebCore::appendedSublist): Ditto.
+        (WebCore::enclosingEmptyListItem): Ditto.
+        * editing/htmlediting.h:
+
 === Safari-521.20 ===
 
 2006-07-21  Tim Omernick  <timo@apple.com>
index 5567b1d6f8cc68d5b2058b97a2220c7db0d71971..960f012698a8ee35637fafacb04d097d35d51234 100644 (file)
@@ -116,7 +116,7 @@ void NodeIterator::notifyBeforeNodeRemoval(Node* removedNode)
 {
     // Iterator is not affected if the removed node is the reference node and is the root.
     // or if removed node is not the reference node, or the ancestor of the reference node.
-    if (!removedNode || removedNode == root())
+    if (!removedNode || removedNode == root() || !removedNode->isAncestor(root()))
         return;
     bool willRemoveReferenceNode = removedNode == referenceNode();
     bool willRemoveReferenceNodeAncestor = referenceNode() && referenceNode()->isAncestor(removedNode);
index 65f16f9acd52a8144a0dd8a89cee4466a619cdf7..06b8fffa04cbf25697dc3163f147a8085f6c1fa1 100644 (file)
@@ -28,6 +28,8 @@
 
 #include "AppendNodeCommand.h"
 #include "ApplyStyleCommand.h"
+#include "CSSComputedStyleDeclaration.h"
+#include "CSSMutableStyleDeclaration.h"
 #include "DeleteFromTextNodeCommand.h"
 #include "DeleteSelectionCommand.h"
 #include "Document.h"
@@ -731,6 +733,39 @@ void CompositeEditCommand::moveParagraphs(const VisiblePosition& startOfParagrap
     }
 }
 
+// FIXME: Send an appropriate shouldDeleteRange call.
+bool CompositeEditCommand::breakOutOfEmptyListItem()
+{
+    Node* emptyListItem = enclosingEmptyListItem(endingSelection().visibleStart());
+    if (!emptyListItem)
+        return false;
+        
+    RefPtr<CSSMutableStyleDeclaration> style = styleAtPosition(endingSelection().start());
+
+    Node *listNode = emptyListItem->parentNode();
+    RefPtr<Node> newBlock = isListElement(listNode->parentNode()) ? createListItemElement(document()) : createDefaultParagraphElement(document());
+    
+    if (emptyListItem->renderer()->nextSibling()) {
+        if (emptyListItem->renderer()->previousSibling())
+            splitElement(static_cast<Element *>(listNode), emptyListItem);
+        insertNodeBefore(newBlock.get(), listNode);
+        removeNode(emptyListItem);
+    } else {
+        insertNodeAfter(newBlock.get(), listNode);
+        removeNode(emptyListItem->renderer()->previousSibling() ? emptyListItem : listNode);
+    }
+    
+    appendBlockPlaceholder(newBlock.get());
+    setEndingSelection(Position(newBlock.get(), 0), DOWNSTREAM);
+    
+    CSSComputedStyleDeclaration endingStyle(endingSelection().start().node());
+    endingStyle.diff(style.get());
+    if (style->length() > 0)
+        applyStyle(style.get());
+    
+    return true;
+}
+
 PassRefPtr<Element> createBlockPlaceholderElement(Document* document)
 {
     ExceptionCode ec = 0;
index 1d7acd71c0aaf6c987f8be26d97ab64af0cb192f..6fc890908b4068f279aecd1f4e6eeb743bdf75af 100644 (file)
@@ -90,9 +90,9 @@ protected:
     void deleteInsignificantText(const Position &start, const Position &end);
     void deleteInsignificantTextDownstream(const Position &);
 
-    Node *appendBlockPlaceholder(Node *);
+    Node *appendBlockPlaceholder(Node*);
     Node *insertBlockPlaceholder(const Position &pos);
-    Node *addBlockPlaceholderIfNeeded(Node *);
+    Node *addBlockPlaceholderIfNeeded(Node*);
     void removeBlockPlaceholder(const VisiblePosition&);
 
     void moveParagraphContentsToNewBlockIfNecessary(const Position &);
@@ -102,6 +102,8 @@ protected:
     
     void moveParagraph(const VisiblePosition&, const VisiblePosition&, const VisiblePosition&, bool preserveSelection = false, bool preserveStyle = true);
     void moveParagraphs(const VisiblePosition&, const VisiblePosition&, const VisiblePosition&, bool preserveSelection = false, bool preserveStyle = true);
+    
+    bool breakOutOfEmptyListItem();
 
     DeprecatedValueList<EditCommandPtr> m_cmds;
 };
index b3b3508235f6b5778f0ef5f80e694a1bbdc6f0b6..900050716d9baac4094ca520346aab7acaa997b2 100644 (file)
@@ -512,7 +512,7 @@ void DeleteSelectionCommand::doApply()
     // use the current ending selection.
     if (!m_hasSelectionToDelete)
         m_selectionToDelete = endingSelection();
-        
+    
     if (!m_selectionToDelete.isRange())
         return;
 
index b5b47516e5930c549975396b4d97ea8f15891bfd..1a6322325778cf676eb7dd670b5da1f85cad75fb 100644 (file)
@@ -77,49 +77,7 @@ void InsertParagraphSeparatorCommand::applyStyleAfterInsertion()
         applyStyle(m_style.get());
 }
 
-static Node* embeddedSublist(Node* listItem)
-{
-    // check for sublist embedded in the list item
-    // NOTE: Must allow for collapsed sublist (i.e. no renderer), so just check DOM
-    for (Node* n = listItem->firstChild(); n; n = n->nextSibling()) {
-        if (isListElement(n))
-            return n;
-    }
-    
-    return 0;
-}
 
-static Node* appendedSublist(Node* listItem)
-{
-    // check for sublist between regular list items
-    // NOTE: Must allow for collapsed sublist (i.e. no renderer), so just check DOM
-    for (Node* n = listItem->nextSibling(); n; n = n->nextSibling()) {
-        if (isListElement(n))
-            return n;
-        if (n->renderer() && n->renderer()->isListItem())
-            return 0;
-    }
-    
-    return 0;
-}
-
-static Node* enclosingEmptyListItem(const VisiblePosition& visiblePos)
-{
-    // check that position is on a line by itself inside a list item
-    Node* listChildNode = enclosingListChild(visiblePos.deepEquivalent().node());
-    if (!listChildNode || !isStartOfParagraph(visiblePos) || !isEndOfParagraph(visiblePos))
-        return 0;
-    
-    // check for sublist embedded in the list item
-    if (embeddedSublist(listChildNode))
-        return 0;
-    
-    // check for sublist between regular list items
-    if (appendedSublist(listChildNode))
-        return 0;
-        
-    return listChildNode;
-}
 
 void InsertParagraphSeparatorCommand::doApply()
 {
@@ -162,26 +120,8 @@ void InsertParagraphSeparatorCommand::doApply()
 
     //---------------------------------------------------------------------
     // Handle special case of typing return on an empty list item
-    Node *emptyListItem = enclosingEmptyListItem(visiblePos);
-    if (emptyListItem) {
-        Node *listNode = emptyListItem->parentNode();
-        RefPtr<Node> newBlock = isListElement(listNode->parentNode()) ? createListItemElement(document()) : createDefaultParagraphElement(document());
-        
-        if (emptyListItem->renderer()->nextSibling()) {
-            if (emptyListItem->renderer()->previousSibling())
-                splitElement(static_cast<Element *>(listNode), emptyListItem);
-            insertNodeBefore(newBlock.get(), listNode);
-            removeNode(emptyListItem);
-        } else {
-            insertNodeAfter(newBlock.get(), listNode);
-            removeNode(emptyListItem->renderer()->previousSibling() ? emptyListItem : listNode);
-        }
-        
-        appendBlockPlaceholder(newBlock.get());
-        setEndingSelection(Position(newBlock.get(), 0), DOWNSTREAM);
-        applyStyleAfterInsertion();
+    if (breakOutOfEmptyListItem())
         return;
-    }
 
     //---------------------------------------------------------------------
     // Prepare for more general cases.
index 278e854c0057b9ed5ecc3e8126665e3cd9c70105..efead0f5f4a1a54544cf6d5cb5a2bb24bd87728a 100644 (file)
@@ -335,11 +335,19 @@ void TypingCommand::deleteKeyPressed(TextGranularity granularity)
             SelectionController sc = SelectionController(endingSelection().start(), endingSelection().end(), SEL_DEFAULT_AFFINITY);
             sc.modify(SelectionController::EXTEND, SelectionController::BACKWARD, granularity);
             
+            // When the caret is at the start of the editable area in an empty list item, break out of the list item.
+            if (endingSelection().visibleStart().previous(true).isNull()) {
+                if (breakOutOfEmptyListItem()) {
+                    typingAddedToOpenCommand();
+                    return;
+                }
+            }
+            
+            // When the caret is a) just after a table, or b) at the beginning of the paragraph after a table, select the table.
             Position upstreamStart = endingSelection().start().upstream();
             VisiblePosition visibleStart = endingSelection().visibleStart();
-            if (visibleStart == startOfParagraph(visibleStart))
+            if (isStartOfParagraph(visibleStart))
                 upstreamStart = visibleStart.previous(true).deepEquivalent().upstream();
-            // When deleting tables: Select the table first, then perform the deletion
             if (upstreamStart.node() && upstreamStart.node()->renderer() && upstreamStart.node()->renderer()->isTable() && upstreamStart.offset() == maxDeepOffset(upstreamStart.node())) {
                 setEndingSelection(Selection(Position(upstreamStart.node(), 0), endingSelection().start(), DOWNSTREAM));
                 typingAddedToOpenCommand();
index b3104fd94a713a9ff23170fd4c2aab8bb98814e1..feac8f3b119fdcd0f6a416285011f1b05bd8301b 100644 (file)
 #include "Document.h"
 #include "EditingText.h"
 #include "HTMLElement.h"
-#include "Text.h"
-#include "VisiblePosition.h"
 #include "HTMLInterchange.h"
 #include "HTMLNames.h"
 #include "RenderObject.h"
 #include "RegularExpression.h"
 #include "Range.h"
+#include "Text.h"
+#include "VisiblePosition.h"
+#include "visible_units.h"
 
 using namespace std;
 
@@ -607,6 +608,43 @@ Node* enclosingListChild (Node *node)
     return 0;
 }
 
+static Node* embeddedSublist(Node* listItem)
+{
+    // Check the DOM so that we'll find collapsed sublists without renderers.
+    for (Node* n = listItem->firstChild(); n; n = n->nextSibling()) {
+        if (isListElement(n))
+            return n;
+    }
+    
+    return 0;
+}
+
+static Node* appendedSublist(Node* listItem)
+{
+    // Check the DOM so that we'll find collapsed sublists without renderers.
+    for (Node* n = listItem->nextSibling(); n; n = n->nextSibling()) {
+        if (isListElement(n))
+            return n;
+        if (n->renderer() && n->renderer()->isListItem())
+            return 0;
+    }
+    
+    return 0;
+}
+
+Node* enclosingEmptyListItem(const VisiblePosition& visiblePos)
+{
+    // Check that position is on a line by itself inside a list item
+    Node* listChildNode = enclosingListChild(visiblePos.deepEquivalent().node());
+    if (!listChildNode || !isStartOfParagraph(visiblePos) || !isEndOfParagraph(visiblePos))
+        return 0;
+    
+    if (embeddedSublist(listChildNode) || appendedSublist(listChildNode))
+        return 0;
+        
+    return listChildNode;
+}
+
 Node* outermostEnclosingListChild(Node* node)
 {
     Node* listNode = 0;
index cb9eef42d21e7c641f87581f93b85bdd446c19c8..45760a1823b62ed9eeee280f77af96504f6b281f 100644 (file)
@@ -104,6 +104,7 @@ Position positionOutsideContainingSpecialElement(const Position&, Node** contain
 
 Node* enclosingNodeWithTag(Node*, const QualifiedName&);
 Node* enclosingTableCell(Node*);
+Node* enclosingEmptyListItem(const VisiblePosition&);
 bool isListElement(Node*);
 Node* enclosingList(Node*);
 Node* outermostEnclosingList(Node*);