LayoutTests:
authorjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 Apr 2007 00:08:50 +0000 (00:08 +0000)
committerjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 Apr 2007 00:08:50 +0000 (00:08 +0000)
        Reviewed by harrison

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

WebCore:

        Reviewed by harrison

        Fixes some issues found while investigating:
        <rdar://problem/5144139> On delete, <BR> inserted into non-editable ToDo <TABLE> element

        Move the code to handle inserting content before/after
        tables for [table, 0/max] to insertNodeAt, so that
        all insertions get it, not just some.
        Changed insertNodeAt to take in a position instead of a
        node and an offset.

        * editing/CompositeEditCommand.cpp:
        (WebCore::CompositeEditCommand::insertNodeAt):
        (WebCore::CompositeEditCommand::insertNodeAtTabSpanPosition):
        (WebCore::CompositeEditCommand::insertBlockPlaceholder):
        (WebCore::CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary):
        If paragrahStart.node() is an atomic node, insertNodeAt can handle
        insertion, we don't need to special case it.
        (WebCore::CompositeEditCommand::moveParagraphs):
        (WebCore::CompositeEditCommand::positionAvoidingSpecialElementBoundary):
        * editing/CompositeEditCommand.h:
        * editing/DeleteSelectionCommand.cpp:
        (WebCore::DeleteSelectionCommand::mergeParagraphs):
        (WebCore::DeleteSelectionCommand::doApply):
        * editing/FormatBlockCommand.cpp:
        (WebCore::FormatBlockCommand::doApply):
        * editing/IndentOutdentCommand.cpp:
        (WebCore::IndentOutdentCommand::indentRegion):
        (WebCore::IndentOutdentCommand::outdentParagraph):
        * editing/InsertLineBreakCommand.cpp:
        (WebCore::InsertLineBreakCommand::doApply):
        * editing/InsertListCommand.cpp:
        (WebCore::InsertListCommand::doApply):
        * editing/InsertParagraphSeparatorCommand.cpp:
        (WebCore::InsertParagraphSeparatorCommand::doApply):
        * editing/InsertTextCommand.cpp:
        (WebCore::InsertTextCommand::prepareForTextInsertion): Removed some
        dead code that handled insertion at non-editable positions.
        (WebCore::InsertTextCommand::insertTab):
        * editing/ReplaceSelectionCommand.cpp:
        (WebCore::ReplaceSelectionCommand::doApply):
        (WebCore::ReplaceSelectionCommand::insertNodeAtAndUpdateNodesInserted):
        * editing/ReplaceSelectionCommand.h:

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

17 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/execCommand/5144139-1-expected.checksum [new file with mode: 0644]
LayoutTests/editing/execCommand/5144139-1-expected.png [new file with mode: 0644]
LayoutTests/editing/execCommand/5144139-1-expected.txt [new file with mode: 0644]
LayoutTests/editing/execCommand/5144139-1.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/editing/CompositeEditCommand.cpp
WebCore/editing/CompositeEditCommand.h
WebCore/editing/DeleteSelectionCommand.cpp
WebCore/editing/FormatBlockCommand.cpp
WebCore/editing/IndentOutdentCommand.cpp
WebCore/editing/InsertLineBreakCommand.cpp
WebCore/editing/InsertListCommand.cpp
WebCore/editing/InsertParagraphSeparatorCommand.cpp
WebCore/editing/InsertTextCommand.cpp
WebCore/editing/ReplaceSelectionCommand.cpp
WebCore/editing/ReplaceSelectionCommand.h

index 5799a72..64f6a55 100644 (file)
@@ -1,5 +1,14 @@
 2007-04-19  Justin Garcia  <justin.garcia@apple.com>
 
+        Reviewed by harrison
+
+        * editing/execCommand/5144139-1-expected.checksum: Added.
+        * editing/execCommand/5144139-1-expected.png: Added.
+        * editing/execCommand/5144139-1-expected.txt: Added.
+        * editing/execCommand/5144139-1.html: Added.
+
+2007-04-19  Justin Garcia  <justin.garcia@apple.com>
+
         Reviewed by darin
         
         <rdar://problem/5142012> 
diff --git a/LayoutTests/editing/execCommand/5144139-1-expected.checksum b/LayoutTests/editing/execCommand/5144139-1-expected.checksum
new file mode 100644 (file)
index 0000000..b630267
--- /dev/null
@@ -0,0 +1 @@
+5632d98c249cb154b63e3f494d1fd8bc
\ No newline at end of file
diff --git a/LayoutTests/editing/execCommand/5144139-1-expected.png b/LayoutTests/editing/execCommand/5144139-1-expected.png
new file mode 100644 (file)
index 0000000..ee74318
Binary files /dev/null and b/LayoutTests/editing/execCommand/5144139-1-expected.png differ
diff --git a/LayoutTests/editing/execCommand/5144139-1-expected.txt b/LayoutTests/editing/execCommand/5144139-1-expected.txt
new file mode 100644 (file)
index 0000000..9f04aa1
--- /dev/null
@@ -0,0 +1,24 @@
+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 577x18
+          text run at (0,0) width 333: "This tests for a bug when creating a list from a table. "
+          text run at (333,0) width 244: "The table should be inside a a list item."
+      RenderBlock {DIV} at (0,34) size 784x46
+        RenderBlock {UL} at (0,0) size 784x46
+          RenderListItem {LI} at (40,0) size 744x46
+            RenderBlock (anonymous) at (0,0) size 744x18
+              RenderListMarker at (-17,0) size 7x18: bullet
+            RenderTable {TABLE} at (0,18) size 31x28 [border: (1px outset #808080)]
+              RenderTableSection {TBODY} at (1,1) size 29x26
+                RenderTableRow {TR} at (0,2) size 29x22
+                  RenderTableCell {TD} at (2,2) size 25x22 [border: (1px inset #808080)] [r=0 c=0 rs=1 cs=1]
+                    RenderText {#text} at (2,2) size 21x18
+                      text run at (2,2) width 21: "foo"
+            RenderBlock (anonymous) at (0,46) size 744x0
+        RenderBlock (anonymous) at (0,62) size 784x0
+selection start: position 0 of child 0 {TABLE} of child 0 {LI} of child 0 {UL} 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 {LI} of child 0 {UL} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/execCommand/5144139-1.html b/LayoutTests/editing/execCommand/5144139-1.html
new file mode 100644 (file)
index 0000000..9eee1a2
--- /dev/null
@@ -0,0 +1,9 @@
+<p>This tests for a bug when creating a list from a table.  The table should be inside a a list item.</p>
+<div contenteditable="true" id="div"><table border="1"><tr><td>foo</td></tr></table></div>
+
+<script>
+var div = document.getElementById("div");
+div.focus();
+document.execCommand("SelectAll");
+document.execCommand("InsertUnorderedList");
+</script>
index 36e0527..47ed177 100644 (file)
@@ -1,3 +1,49 @@
+2007-04-19  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by harrison
+        
+        Fixes some issues found while investigating:
+        <rdar://problem/5144139> On delete, <BR> inserted into non-editable ToDo <TABLE> element
+        
+        Move the code to handle inserting content before/after
+        tables for [table, 0/max] to insertNodeAt, so that
+        all insertions get it, not just some.
+        Changed insertNodeAt to take in a position instead of a
+        node and an offset.
+
+        * editing/CompositeEditCommand.cpp:
+        (WebCore::CompositeEditCommand::insertNodeAt):
+        (WebCore::CompositeEditCommand::insertNodeAtTabSpanPosition):
+        (WebCore::CompositeEditCommand::insertBlockPlaceholder):
+        (WebCore::CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary):
+        If paragrahStart.node() is an atomic node, insertNodeAt can handle
+        insertion, we don't need to special case it.
+        (WebCore::CompositeEditCommand::moveParagraphs):
+        (WebCore::CompositeEditCommand::positionAvoidingSpecialElementBoundary):
+        * editing/CompositeEditCommand.h:
+        * editing/DeleteSelectionCommand.cpp:
+        (WebCore::DeleteSelectionCommand::mergeParagraphs):
+        (WebCore::DeleteSelectionCommand::doApply):
+        * editing/FormatBlockCommand.cpp:
+        (WebCore::FormatBlockCommand::doApply):
+        * editing/IndentOutdentCommand.cpp:
+        (WebCore::IndentOutdentCommand::indentRegion):
+        (WebCore::IndentOutdentCommand::outdentParagraph):
+        * editing/InsertLineBreakCommand.cpp:
+        (WebCore::InsertLineBreakCommand::doApply):
+        * editing/InsertListCommand.cpp:
+        (WebCore::InsertListCommand::doApply):
+        * editing/InsertParagraphSeparatorCommand.cpp:
+        (WebCore::InsertParagraphSeparatorCommand::doApply):
+        * editing/InsertTextCommand.cpp:
+        (WebCore::InsertTextCommand::prepareForTextInsertion): Removed some
+        dead code that handled insertion at non-editable positions.
+        (WebCore::InsertTextCommand::insertTab):
+        * editing/ReplaceSelectionCommand.cpp:
+        (WebCore::ReplaceSelectionCommand::doApply):
+        (WebCore::ReplaceSelectionCommand::insertNodeAtAndUpdateNodesInserted):
+        * editing/ReplaceSelectionCommand.h:
+
 2007-04-19  Mitz Pettel  <mitz@webkit.org>
 
         Reviewed by Darin.
index 045586f..dd7559c 100644 (file)
@@ -139,8 +139,15 @@ void CompositeEditCommand::insertNodeAfter(Node* insertChild, Node* refChild)
     }
 }
 
-void CompositeEditCommand::insertNodeAt(Node* insertChild, Node* refChild, int offset)
+void CompositeEditCommand::insertNodeAt(Node* insertChild, const Position& editingPosition)
 {
+    ASSERT(isEditablePosition(editingPosition));
+    // For editing positions like [table, 0], insert before the table,
+    // likewise for replaced elements, brs, etc.
+    Position p = rangeCompliantEquivalent(editingPosition);
+    Node* refChild = p.node();
+    int offset = p.offset();
+    
     if (canHaveChildrenForEditing(refChild)) {
         Node* child = refChild->firstChild();
         for (int i = 0; child && i < offset; i++)
@@ -297,7 +304,7 @@ void CompositeEditCommand::insertNodeAtTabSpanPosition(Node* node, const Positio
 {
     // insert node before, after, or at split of tab span
     Position insertPos = positionOutsideTabSpan(pos);
-    insertNodeAt(node, insertPos.node(), insertPos.offset());
+    insertNodeAt(node, insertPos);
 }
 
 void CompositeEditCommand::deleteSelection(bool smartDelete, bool mergeBlocksAfterDelete, bool replace, bool expandForSpecialElements)
@@ -535,7 +542,7 @@ Node* CompositeEditCommand::insertBlockPlaceholder(const Position& pos)
     ASSERT(pos.node()->renderer());
 
     RefPtr<Node> placeholder = createBlockPlaceholderElement(document());
-    insertNodeAt(placeholder.get(), pos.node(), pos.offset());
+    insertNodeAt(placeholder.get(), pos);
     return placeholder.get();
 }
 
@@ -622,14 +629,7 @@ Node* CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary(const Pos
 
     RefPtr<Node> newBlock = createDefaultParagraphElement(document());
     appendNode(createBreakElement(document()).get(), newBlock.get());
-    
-    if (!isAtomicNode(paragraphStart.node()))
-        insertNodeAt(newBlock.get(), paragraphStart.node(), paragraphStart.offset());
-    else {
-        ASSERT(paragraphStart.offset() <= 1);
-        ASSERT(paragraphStart.node()->parentNode());
-        insertNodeAt(newBlock.get(), paragraphStart.node()->parentNode(), paragraphStart.node()->nodeIndex() + paragraphStart.offset());
-    }
+    insertNodeAt(newBlock.get(), paragraphStart);
     
     moveParagraphs(visibleParagraphStart, visibleParagraphEnd, VisiblePosition(Position(newBlock.get(), 0)));
     
@@ -774,7 +774,7 @@ void CompositeEditCommand::moveParagraphs(const VisiblePosition& startOfParagrap
     afterParagraph = VisiblePosition(afterParagraph.deepEquivalent());
     if (beforeParagraph.isNotNull() && (!isEndOfParagraph(beforeParagraph) || beforeParagraph == afterParagraph)) {
         // FIXME: Trim text between beforeParagraph and afterParagraph if they aren't equal.
-        insertNodeAt(createBreakElement(document()).get(), beforeParagraph.deepEquivalent().node(), beforeParagraph.deepEquivalent().offset());
+        insertNodeAt(createBreakElement(document()).get(), beforeParagraph.deepEquivalent());
         // Need an updateLayout here in case inserting the br has split a text node.
         updateLayout();
     }
@@ -871,16 +871,6 @@ Position CompositeEditCommand::positionAvoidingSpecialElementBoundary(const Posi
                 }
             result = positionBeforeNode(enclosingAnchor);
         }
-    // FIXME: If this avoids positions before/after tables, then it should
-    // probably avoid positions before/after replaced elements, before brs,
-    // etc.  Since it doesn't and there are no known cases where we insert
-    // into such elements, avoiding tables is probably done elsewhere
-    // and is not needed here.
-    } else if (isTableElement(p.node())) {
-        if (isFirstPositionAfterTable(visiblePos))
-            result = positionAfterNode(p.node());
-        if (isLastPositionBeforeTable(visiblePos))
-            result = positionBeforeNode(p.node());
     }
         
     if (result.isNull() || !editableRootForPosition(result))
index b8a3491..5dff09e 100644 (file)
@@ -56,7 +56,7 @@ protected:
     virtual void deleteTextFromNode(Text* node, int offset, int count);
     void inputText(const String&, bool selectInsertedText = false);
     void insertNodeAfter(Node* insertChild, Node* refChild);
-    void insertNodeAt(Node* insertChild, Node* refChild, int offset);
+    void insertNodeAt(Node* insertChild, const Position&);
     void insertNodeBefore(Node* insertChild, Node* refChild);
     void insertParagraphSeparator();
     void insertTextIntoNode(Text* node, int offset, const String& text);
index 1b69591..0b6b628 100644 (file)
@@ -479,7 +479,7 @@ void DeleteSelectionCommand::mergeParagraphs()
     
     // We need to merge into m_upstreamStart's block, but it's been emptied out and collapsed by deletion.
     if (!mergeDestination.deepEquivalent().node() || !mergeDestination.deepEquivalent().node()->isDescendantOf(m_upstreamStart.node()->enclosingBlockFlowElement())) {
-        insertNodeAt(createBreakElement(document()).get(), m_upstreamStart.node(), m_upstreamStart.offset());
+        insertNodeAt(createBreakElement(document()).get(), m_upstreamStart);
         mergeDestination = VisiblePosition(m_upstreamStart);
     }
     
@@ -639,7 +639,7 @@ void DeleteSelectionCommand::doApply()
     mergeParagraphs();
     
     if (placeholder)
-        insertNodeAt(placeholder.get(), m_endingPosition.node(), m_endingPosition.offset());
+        insertNodeAt(placeholder.get(), m_endingPosition);
 
     rebalanceWhitespaceAt(m_endingPosition);
 
index 3e52340..85deafa 100644 (file)
@@ -119,7 +119,7 @@ void FormatBlockCommand::doApply()
         // Already in a valid block tag that only contains the current paragraph, so we can swap with the new tag
         insertNodeBefore(blockNode.get(), refNode);
     else {
-        insertNodeAt(blockNode.get(), upstreamStart.node(), upstreamStart.offset());
+        insertNodeAt(blockNode.get(), upstreamStart);
     }
     appendNode(placeholder.get(), blockNode.get());
     moveParagraph(paragraphStart, paragraphEnd, VisiblePosition(Position(placeholder.get(), 0)), true, false);
index cb46243..8d7bb9b 100644 (file)
@@ -144,7 +144,7 @@ void IndentOutdentCommand::indentRegion()
     Position start = startOfSelection.deepEquivalent().downstream();
     if (start.node() == editableRootForPosition(start)) {
         RefPtr<Node> blockquote = createIndentBlockquoteElement(document());
-        insertNodeAt(blockquote.get(), start.node(), 0);
+        insertNodeAt(blockquote.get(), start);
         RefPtr<Node> placeholder = createBreakElement(document());
         appendNode(placeholder.get(), blockquote.get());
         setEndingSelection(Selection(Position(placeholder.get(), 0), DOWNSTREAM));
@@ -229,9 +229,9 @@ void IndentOutdentCommand::outdentParagraph()
         removeNodePreservingChildren(enclosingNode);
         updateLayout();
         if (visibleStartOfParagraph.isNotNull() && !isStartOfParagraph(visibleStartOfParagraph))
-            insertNodeAt(createBreakElement(document()).get(), visibleStartOfParagraph.deepEquivalent().node(), visibleStartOfParagraph.deepEquivalent().offset());
+            insertNodeAt(createBreakElement(document()).get(), visibleStartOfParagraph.deepEquivalent());
         if (visibleEndOfParagraph.isNotNull() && !isEndOfParagraph(visibleEndOfParagraph))
-            insertNodeAt(createBreakElement(document()).get(), visibleEndOfParagraph.deepEquivalent().node(), visibleEndOfParagraph.deepEquivalent().offset());
+            insertNodeAt(createBreakElement(document()).get(), visibleEndOfParagraph.deepEquivalent());
         return;
     }
     Node* enclosingBlockFlow = enclosingBlockFlowElement(visibleStartOfParagraph);
index d2a6720..edf22ca 100644 (file)
@@ -133,7 +133,7 @@ void InsertLineBreakCommand::doApply()
             // There aren't any VisiblePositions like this yet.
             ASSERT_NOT_REACHED();
     } else if (isEndOfParagraph(caret) && !lineBreakExistsAtPosition(caret)) {
-        insertNodeAt(nodeToInsert.get(), pos.node(), pos.offset());
+        insertNodeAt(nodeToInsert.get(), pos);
         insertNodeBefore(nodeToInsert->cloneNode(false).get(), nodeToInsert.get());
         VisiblePosition endingPosition(Position(nodeToInsert.get(), 0));
         setEndingSelection(Selection(endingPosition));
index 78b9ec1..3b09075 100644 (file)
@@ -198,7 +198,7 @@ void InsertListCommand::doApply()
                 end = start;
             }
             
-            insertNodeAt(listElement.get(), start.deepEquivalent().node(), start.deepEquivalent().offset());
+            insertNodeAt(listElement.get(), start.deepEquivalent());
         }
         moveParagraph(start, end, VisiblePosition(Position(placeholder.get(), 0)), true);
         if (nextList && previousList)
index 41d0d0a..3235494 100644 (file)
@@ -192,7 +192,7 @@ void InsertParagraphSeparatorCommand::doApply()
     // content will move down a line.
     if (isStartOfParagraph(visiblePos)) {
         RefPtr<Element> br = createBreakElement(document());
-        insertNodeAt(br.get(), pos.node(), pos.offset());
+        insertNodeAt(br.get(), pos);
         pos = positionAfterNode(br.get());
     }
     
index 8c88b0a..7c59c73 100644 (file)
@@ -57,7 +57,7 @@ Position InsertTextCommand::prepareForTextInsertion(const Position& p)
     // If an anchor was removed and the selection hasn't changed, we restore it.
     RefPtr<Node> anchor = document()->frame()->editor()->removedAnchor();
     if (anchor) {
-        insertNodeAt(anchor.get(), pos.node(), pos.offset());
+        insertNodeAt(anchor.get(), pos);
         document()->frame()->editor()->setRemovedAnchor(0);
         pos = Position(anchor.get(), 0);
     }
@@ -70,15 +70,7 @@ Position InsertTextCommand::prepareForTextInsertion(const Position& p)
     if (!pos.node()->isTextNode()) {
         RefPtr<Node> textNode = document()->createEditingTextNode("");
 
-        // Now insert the node in the right place
-        if (pos.node()->rootEditableElement() != NULL) {
-            insertNodeAt(textNode.get(), pos.node(), pos.offset());
-        } else if (pos.node()->caretMinOffset() == pos.offset()) {
-            insertNodeBefore(textNode.get(), pos.node());
-        } else if (pos.node()->caretMaxOffset() == pos.offset()) {
-            insertNodeAfter(textNode.get(), pos.node());
-        } else
-            ASSERT_NOT_REACHED();
+        insertNodeAt(textNode.get(), pos);
         
         return Position(textNode.get(), 0);
     }
@@ -174,7 +166,7 @@ Position InsertTextCommand::insertTab(const Position& pos)
     
     // place it
     if (!node->isTextNode()) {
-        insertNodeAt(spanNode.get(), node, offset);
+        insertNodeAt(spanNode.get(), insertPos);
     } else {
         Text *textNode = static_cast<Text *>(node);
         if (offset >= textNode->length()) {
index 785f66a..0134732 100644 (file)
@@ -573,7 +573,7 @@ void ReplaceSelectionCommand::doApply()
     RefPtr<Node> node = refNode->nextSibling();
     
     fragment.removeNode(refNode);
-    insertNodeAtAndUpdateNodesInserted(refNode.get(), insertionPos.node(), insertionPos.offset());
+    insertNodeAtAndUpdateNodesInserted(refNode.get(), insertionPos);
     
     while (node) {
         Node* next = node->nextSibling();
@@ -591,7 +591,7 @@ void ReplaceSelectionCommand::doApply()
     // We inserted before the startBlock to prevent nesting, and the content before the startBlock wasn't in its own block and
     // didn't have a br after it, so the inserted content ended up in the same paragraph.
     if (startBlock && insertionPos.node() == startBlock->parentNode() && (unsigned)insertionPos.offset() < startBlock->nodeIndex() && !isStartOfParagraph(startOfInsertedContent))
-        insertNodeAt(createBreakElement(document()).get(), startOfInsertedContent.deepEquivalent().node(), startOfInsertedContent.deepEquivalent().offset());
+        insertNodeAt(createBreakElement(document()).get(), startOfInsertedContent.deepEquivalent());
     
     Position lastPositionToSelect;
     
@@ -777,9 +777,9 @@ void ReplaceSelectionCommand::insertNodeAfterAndUpdateNodesInserted(Node *insert
     updateNodesInserted(insertChild);
 }
 
-void ReplaceSelectionCommand::insertNodeAtAndUpdateNodesInserted(Node *insertChild, Node *refChild, int offset)
+void ReplaceSelectionCommand::insertNodeAtAndUpdateNodesInserted(Node *insertChild, const Position& p)
 {
-    insertNodeAt(insertChild, refChild, offset);
+    insertNodeAt(insertChild, p);
     updateNodesInserted(insertChild);
 }
 
index 670b933..04d5a92 100644 (file)
@@ -79,7 +79,7 @@ private:
     void completeHTMLReplacement(const Position& lastPositionToSelect);
 
     void insertNodeAfterAndUpdateNodesInserted(Node* insertChild, Node* refChild);
-    void insertNodeAtAndUpdateNodesInserted(Node* insertChild, Node* refChild, int offset);
+    void insertNodeAtAndUpdateNodesInserted(Node*, const Position&);
     void insertNodeBeforeAndUpdateNodesInserted(Node* insertChild, Node* refChild);
 
     void updateNodesInserted(Node*);