WebCore:
authorjustin.garcia@apple.com <justin.garcia@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Jun 2008 21:22:12 +0000 (21:22 +0000)
committerjustin.garcia@apple.com <justin.garcia@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Jun 2008 21:22:12 +0000 (21:22 +0000)
2008-06-25  Justin Garcia  <justin.garcia@apple.com>

        Reviewed by John.

        <rdar://problem/5994480> Line break lost on some pastes

        Merging the the first paragraph of inserted content with the content that came
        before the selection that was pasted into would also move content after
        the selection that was pasted into if:

        1) Only one paragraph was being pasted, and it was not wrapped in a block
        2) The selection that was pasted into ended at the end of a block
        3) The next paragraph didn't start at the start of a block.

        Insert a line break just after the inserted content to separate it from what
        comes after and prevent that from happening.

        Doing this exposed a bug in deletion where it would insert an unnecessary placeholder
        when deleting a paragraph that started or ended with an input element.  This was
        because its m_startBlock and m_endBlock were still computed with the old deprecated
        enclosingBlockFlowOrTableElement().

        * editing/DeleteSelectionCommand.cpp:
        (WebCore::DeleteSelectionCommand::initializePositionData): Use the new method for
        getting an enclosing block.
        (WebCore::DeleteSelectionCommand::doApply): The new method for getting an enclosing
        block will return 0 if it reaches the root editable element before finding a block,
        so if we're deleting inside an inline editable root, m_start/endBlock will
        be 0.  Removed an early return for this case (we already have test coverage for it).
        * editing/ReplaceSelectionCommand.cpp:
        (WebCore::ReplaceSelectionCommand::doApply): Insert a line break just after the inserted
        content to separate it from what comes after.
        * dom/Node.h: Removed enclosingBlockFlowOrTableElement().
        * dom/Node.cpp: Ditto.

LayoutTests:

2008-06-25  Justin Garcia  <justin.garcia@apple.com>

        Reviewed by John.

        <rdar://problem/5994480> Line break lost on some pastes

        These demonstrate fixes:
        * editing/inserting/5994480.html: Added.
        * editing/inserting/5994480-expected.txt: Added.
        * editing/inserting/5994480-2.html: Added.
        * editing/inserting/5994480-2-expected.txt: Added.

        The changes made in this fix caused more of the unrendered text from the original
        file to be preserved and show up in the test results.  We insert a line break after
        inserted content to separate it from content that comes after and prevent it from
        being merged.  In these what came after was unrendered whitespace that was previously
        clobbered by the merge:
        * platform/mac/editing/pasteboard/paste-match-style-001-expected.txt:
        * platform/mac/editing/pasteboard/paste-text-010-expected.txt:
        * platform/mac/editing/pasteboard/smart-paste-001-expected.txt:
        * platform/mac/editing/style/style-boundary-005-expected.txt:

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

14 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/inserting/5994480-2-expected.txt [new file with mode: 0644]
LayoutTests/editing/inserting/5994480-2.html [new file with mode: 0644]
LayoutTests/editing/inserting/5994480-expected.txt [new file with mode: 0644]
LayoutTests/editing/inserting/5994480.html [new file with mode: 0644]
LayoutTests/platform/mac/editing/pasteboard/paste-match-style-001-expected.txt
LayoutTests/platform/mac/editing/pasteboard/paste-text-010-expected.txt
LayoutTests/platform/mac/editing/pasteboard/smart-paste-001-expected.txt
LayoutTests/platform/mac/editing/style/style-boundary-005-expected.txt
WebCore/ChangeLog
WebCore/dom/Node.cpp
WebCore/dom/Node.h
WebCore/editing/DeleteSelectionCommand.cpp
WebCore/editing/ReplaceSelectionCommand.cpp

index 41a50e1..22d8c70 100644 (file)
@@ -1,3 +1,25 @@
+2008-06-25  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by John.
+        
+        <rdar://problem/5994480> Line break lost on some pastes
+        
+        These demonstrate fixes:
+        * editing/inserting/5994480.html: Added.
+        * editing/inserting/5994480-expected.txt: Added.
+        * editing/inserting/5994480-2.html: Added.
+        * editing/inserting/5994480-2-expected.txt: Added.
+        
+        The changes made in this fix caused more of the unrendered text from the original 
+        file to be preserved and show up in the test results.  We insert a line break after
+        inserted content to separate it from content that comes after and prevent it from
+        being merged.  In these what came after was unrendered whitespace that was previously
+        clobbered by the merge:
+        * platform/mac/editing/pasteboard/paste-match-style-001-expected.txt:
+        * platform/mac/editing/pasteboard/paste-text-010-expected.txt:
+        * platform/mac/editing/pasteboard/smart-paste-001-expected.txt:
+        * platform/mac/editing/style/style-boundary-005-expected.txt:
+
 2008-06-25  Anders Carlsson  <andersca@apple.com>
 
         Reviewed by Mitz.
diff --git a/LayoutTests/editing/inserting/5994480-2-expected.txt b/LayoutTests/editing/inserting/5994480-2-expected.txt
new file mode 100644 (file)
index 0000000..224592b
--- /dev/null
@@ -0,0 +1 @@
+<font class="Apple-style-span" face="'Lucida Grande'" size="3"><span class="Apple-style-span" style="font-size: 11px;"><br></span></font>
diff --git a/LayoutTests/editing/inserting/5994480-2.html b/LayoutTests/editing/inserting/5994480-2.html
new file mode 100644 (file)
index 0000000..380f50f
--- /dev/null
@@ -0,0 +1,13 @@
+<div id="description">This tests to see if deleting an input element that starts at the start of a block adds an extra placeholder.  You should see a single placeholder in the block below.</div>
+<div id="edit" contentEditable="true"><input type="text"><br></div>
+<script>
+if (window.layoutTestController)
+    window.layoutTestController.dumpAsText();
+edit = document.getElementById("edit");
+edit.focus();
+
+document.execCommand("SelectAll");
+document.execCommand("Delete");
+if (window.layoutTestController)
+    document.body.innerText = edit.innerHTML;
+</script>
diff --git a/LayoutTests/editing/inserting/5994480-expected.txt b/LayoutTests/editing/inserting/5994480-expected.txt
new file mode 100644 (file)
index 0000000..4ab6c68
--- /dev/null
@@ -0,0 +1,2 @@
+This tests to see if pasting a single paragraph not in a block at the end of a block just before a paragraph not in a block removes a line break. You should see two separate paragraphs below.
+<div id="paragraphOne">Paragraph One.</div>Paragraph two.
diff --git a/LayoutTests/editing/inserting/5994480.html b/LayoutTests/editing/inserting/5994480.html
new file mode 100644 (file)
index 0000000..3009882
--- /dev/null
@@ -0,0 +1,14 @@
+<div id="description">This tests to see if pasting a single paragraph not in a block at the end of a block just before a paragraph not in a block removes a line break. You should see two separate paragraphs below.</div>
+<div id="edit" contentEditable="true"><div id="paragraphOne">Paragraph One</div>Paragraph two.</div>
+
+<script>
+if (window.layoutTestController)
+    window.layoutTestController.dumpAsText();
+paragraphOne = document.getElementById("paragraphOne");
+// Place the caret at the end of the block that contains the first paragraph.
+window.getSelection().setPosition(paragraphOne, paragraphOne.childNodes.length);
+
+document.execCommand("InsertHTML", false, ".");
+if (window.layoutTestController)
+    document.body.innerText = document.getElementById("description").innerText + "\n" + document.getElementById("edit").innerHTML;
+</script>
index 24f96c2..61b1048 100644 (file)
@@ -47,4 +47,5 @@ layer at (0,0) size 800x600
             RenderText {#text} at (14,2) size 13x28
               text run at (14,2) width 13: "b"
         RenderBlock (anonymous) at (0,32) size 784x0
+          RenderText {#text} at (0,0) size 0x0
 caret: position 1 of child 1 {#text} of child 0 {B} of child 1 {DIV} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document
index 4380508..d4e312c 100644 (file)
@@ -68,4 +68,8 @@ layer at (0,0) size 800x600
         RenderText {#text} at (88,70) size 74x28
           text run at (88,70) width 74: "of men."
       RenderBlock (anonymous) at (0,112) size 784x0
+        RenderText {#text} at (0,0) size 0x0
+        RenderText {#text} at (0,0) size 0x0
+        RenderText {#text} at (0,0) size 0x0
+        RenderText {#text} at (0,0) size 0x0
 caret: position 7 of child 4 {#text} of child 1 {DIV} of child 1 {BODY} of child 0 {HTML} of document
index 60ade8f..694d7e4 100644 (file)
@@ -36,4 +36,5 @@ layer at (0,0) size 800x600
           RenderText {#text} at (36,2) size 40x28
             text run at (36,2) width 40: " test"
         RenderBlock (anonymous) at (0,32) size 784x0
+          RenderText {#text} at (0,0) size 0x0
 caret: position 5 of child 1 {#text} of child 1 {DIV} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document
index dddfca0..a2e123c 100644 (file)
@@ -66,4 +66,5 @@ layer at (0,0) size 800x600
               RenderText {#text} at (121,2) size 27x18
                 text run at (121,2) width 27: " one"
         RenderBlock (anonymous) at (0,22) size 784x0
+          RenderText {#text} at (0,0) size 0x0
 caret: position 4 of child 0 {#text} of child 1 {SPAN} of child 1 {B} of child 1 {DIV} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document
index c16bc95..3511004 100644 (file)
@@ -1,3 +1,38 @@
+2008-06-25  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by John.
+
+        <rdar://problem/5994480> Line break lost on some pastes
+        
+        Merging the the first paragraph of inserted content with the content that came
+        before the selection that was pasted into would also move content after 
+        the selection that was pasted into if:
+        
+        1) Only one paragraph was being pasted, and it was not wrapped in a block
+        2) The selection that was pasted into ended at the end of a block
+        3) The next paragraph didn't start at the start of a block.
+        
+        Insert a line break just after the inserted content to separate it from what 
+        comes after and prevent that from happening.
+        
+        Doing this exposed a bug in deletion where it would insert an unnecessary placeholder
+        when deleting a paragraph that started or ended with an input element.  This was
+        because its m_startBlock and m_endBlock were still computed with the old deprecated
+        enclosingBlockFlowOrTableElement().
+        
+        * editing/DeleteSelectionCommand.cpp:
+        (WebCore::DeleteSelectionCommand::initializePositionData): Use the new method for
+        getting an enclosing block.
+        (WebCore::DeleteSelectionCommand::doApply): The new method for getting an enclosing
+        block will return 0 if it reaches the root editable element before finding a block,
+        so if we're deleting inside an inline editable root, m_start/endBlock will
+        be 0.  Removed an early return for this case (we already have test coverage for it).
+        * editing/ReplaceSelectionCommand.cpp:
+        (WebCore::ReplaceSelectionCommand::doApply): Insert a line break just after the inserted 
+        content to separate it from what comes after.
+        * dom/Node.h: Removed enclosingBlockFlowOrTableElement().
+        * dom/Node.cpp: Ditto.
+
 2008-06-25  Anders Carlsson  <andersca@apple.com>
 
         Reviewed by Dave Hyatt.
index ea28ab3..119f7dc 100644 (file)
@@ -1118,22 +1118,6 @@ bool Node::isEditableBlock() const
     return isContentEditable() && isBlockFlow();
 }
 
-Element *Node::enclosingBlockFlowOrTableElement() const
-{
-    Node *n = const_cast<Node *>(this);
-    if (isBlockFlowOrBlockTable())
-        return static_cast<Element *>(n);
-
-    while (1) {
-        n = n->parentNode();
-        if (!n)
-            break;
-        if (n->isBlockFlowOrBlockTable() || n->hasTagName(bodyTag))
-            return static_cast<Element *>(n);
-    }
-    return 0;
-}
-
 Element *Node::enclosingBlockFlowElement() const
 {
     Node *n = const_cast<Node *>(this);
index ff19627..7e4e5c0 100644 (file)
@@ -199,8 +199,10 @@ public:
     Node* previousLeafNode() const;
 
     bool isEditableBlock() const;
+    
+    // enclosingBlockFlowElement() is deprecated.  Use enclosingBlock instead.
     Element* enclosingBlockFlowElement() const;
-    Element* enclosingBlockFlowOrTableElement() const;
+    
     Element* enclosingInlineElement() const;
     Element* rootEditableElement() const;
     
index d73310d..0c9d68a 100644 (file)
@@ -226,11 +226,14 @@ void DeleteSelectionCommand::initializePositionData()
         }
     }
     
-    //
-    // Handle setting start and end blocks and the start node.
-    //
-    m_startBlock = m_downstreamStart.node()->enclosingBlockFlowOrTableElement();
-    m_endBlock = m_upstreamEnd.node()->enclosingBlockFlowOrTableElement();
+    // We must pass the positions through rangeCompliantEquivalent, since some editing positions
+    // that appear inside their nodes aren't really inside them.  [hr, 0] is one example.
+    // FIXME: rangeComplaintEquivalent should eventually be moved into enclosing element getters
+    // like the one below, since editing functions should obviously accept editing positions.
+    // FIXME: Passing false to enclosingNodeOfType tells it that it's OK to return a non-editable
+    // node.  This was done to match existing behavior, but it seems wrong.
+    m_startBlock = enclosingNodeOfType(rangeCompliantEquivalent(m_downstreamStart), &isBlock, false);
+    m_endBlock = enclosingNodeOfType(rangeCompliantEquivalent(m_upstreamEnd), &isBlock, false);
 }
 
 void DeleteSelectionCommand::saveTypingStyleState()
@@ -729,15 +732,6 @@ void DeleteSelectionCommand::doApply()
     
     // set up our state
     initializePositionData();
-    if (!m_startBlock || !m_endBlock) {
-        // Can't figure out what blocks we're in. This can happen if
-        // the document structure is not what we are expecting, like if
-        // the document has no body element, or if the editable block
-        // has been changed to display: inline. Some day it might
-        // be nice to be able to deal with this, but for now, bail.
-        clearTransientState();
-        return;
-    }
 
     // Delete any text that may hinder our ability to fixup whitespace after the delete
     deleteInsignificantTextDownstream(m_trailingWhitespace);    
index 4198db6..e319052 100644 (file)
@@ -793,6 +793,17 @@ void ReplaceSelectionCommand::doApply()
         VisiblePosition destination = startOfInsertedContent.previous();
         VisiblePosition startOfParagraphToMove = startOfInsertedContent;
         
+        // Merging the the first paragraph of inserted content with the content that came
+        // before the selection that was pasted into would also move content after 
+        // the selection that was pasted into if: only one paragraph was being pasted, 
+        // and it was not wrapped in a block, the selection that was pasted into ended 
+        // at the end of a block and the next paragraph didn't start at the start of a block.
+        // Insert a line break just after the inserted content to separate it from what 
+        // comes after and prevent that from happening.
+        VisiblePosition endOfInsertedContent = positionAtEndOfInsertedContent();
+        if (startOfParagraph(endOfInsertedContent) == startOfParagraphToMove)
+            insertNodeAt(createBreakElement(document()).get(), endOfInsertedContent.deepEquivalent());
+        
         // FIXME: Maintain positions for the start and end of inserted content instead of keeping nodes.  The nodes are
         // only ever used to create positions where inserted content starts/ends.
         moveParagraph(startOfParagraphToMove, endOfParagraph(startOfParagraphToMove), destination);