Reviewed by Hyatt
authorkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Jan 2005 22:43:16 +0000 (22:43 +0000)
committerkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Jan 2005 22:43:16 +0000 (22:43 +0000)
        Fix for this bug:

        <rdar://problem/3941203> REGRESSION (Mail): Paste inserts content in wrong place

        * khtml/editing/htmlediting.cpp:
        (khtml::ReplaceSelectionCommand::doApply): Some cleanup and refinement of the concepts used to make
        this operation work correctly, particularly in the logic to figure out whether to merge content, and
        also performing merges.
        * khtml/editing/visible_position.cpp:
        (khtml::isFirstVisiblePositionInBlock): Simplification of test used to make this determination.
        * khtml/editing/visible_units.cpp:
        (khtml::isStartOfParagraph): New helper, used in khtml::ReplaceSelectionCommand::doApply().
        (khtml::isEndOfParagraph): Ditto.
        * khtml/editing/visible_units.h: Declare new functions.

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

LayoutTests/editing/pasteboard/paste-text-016-expected.txt [new file with mode: 0644]
LayoutTests/editing/pasteboard/paste-text-016.html [new file with mode: 0644]
WebCore/ChangeLog-2005-08-23
WebCore/khtml/editing/htmlediting.cpp
WebCore/khtml/editing/visible_position.cpp
WebCore/khtml/editing/visible_units.cpp
WebCore/khtml/editing/visible_units.h

diff --git a/LayoutTests/editing/pasteboard/paste-text-016-expected.txt b/LayoutTests/editing/pasteboard/paste-text-016-expected.txt
new file mode 100644 (file)
index 0000000..47d213e
--- /dev/null
@@ -0,0 +1,45 @@
+layer at (0,0) size 800x600
+  RenderCanvas 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 (anonymous) at (0,0) size 784x36
+        RenderText {TEXT} at (0,0) size 96x18
+          text run at (0,0) width 96: "Fixes this bug: "
+        RenderInline {A} at (0,0) size 167x18 [color=#0000EE]
+          RenderText {TEXT} at (96,0) size 167x18
+            text run at (96,0) width 167: "<rdar://problem/3927554>"
+        RenderText {TEXT} at (263,0) size 385x18
+          text run at (263,0) width 385: " REGRESSION (Mail): Paste inserts content in wrong place "
+        RenderBR {BR} at (0,0) size 0x0
+        RenderText {TEXT} at (0,18) size 378x18
+          text run at (0,18) width 378: "***TEST*** line should be second, following the first line."
+      RenderBlock {DIV} at (0,36) size 784x12
+      RenderBlock {DIV} at (0,48) size 784x224
+        RenderBlock {DIV} at (0,0) size 784x224 [border: (2px solid #FF0000)]
+          RenderBlock {P} at (14,14) size 756x112
+            RenderBlock (anonymous) at (0,0) size 756x28
+              RenderText {TEXT} at (0,0) size 319x28
+                text run at (0,0) width 319: "Should be first line of document."
+              RenderBR {BR} at (0,0) size 0x0
+            RenderBlock {P} at (0,28) size 756x28
+              RenderText {TEXT} at (0,0) size 130x28
+                text run at (0,0) width 130: "***TEST***"
+            RenderBlock (anonymous) at (0,56) size 756x56
+              RenderBR {BR} at (0,0) size 0x28
+              RenderText {TEXT} at (0,28) size 128x28
+                text run at (0,28) width 128: "Another line."
+          RenderBlock {P} at (14,126) size 756x0
+          RenderBlock (anonymous) at (14,126) size 756x28
+            RenderText {TEXT} at (0,0) size 6x28
+              text run at (0,0) width 6: " "
+          RenderBlock {P} at (14,154) size 756x0
+          RenderBlock (anonymous) at (14,154) size 756x28
+            RenderText {TEXT} at (0,0) size 6x28
+              text run at (0,0) width 6: " "
+          RenderBlock {P} at (14,182) size 756x28
+            RenderBR {BR} at (0,0) size 0x28
+selection is CARET:
+start:      position 0 of child 4 {BR} of child 1 {P} of child 1 {DIV} of root {DIV}
+upstream:   position 0 of child 4 {BR} of child 1 {P} of child 1 {DIV} of root {DIV}
+downstream: position 0 of child 4 {BR} of child 1 {P} of child 1 {DIV} of root {DIV}
diff --git a/LayoutTests/editing/pasteboard/paste-text-016.html b/LayoutTests/editing/pasteboard/paste-text-016.html
new file mode 100644 (file)
index 0000000..bcafddf
--- /dev/null
@@ -0,0 +1,51 @@
+<html> 
+<head>
+
+<style>
+.editing { 
+    border: 2px solid red; 
+    padding: 12px; 
+    font-size: 24px; 
+}
+p {
+    margin: 0;
+}
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    for (i = 0; i < 5; i++)
+        moveSelectionForwardByLineCommand();
+    extendSelectionForwardByLineCommand();
+    cutCommand();
+    for (i = 0; i < 5; i++)
+        moveSelectionBackwardByLineCommand();
+    moveSelectionForwardByLineCommand();
+    pasteCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body contenteditable style="word-wrap: break-word; -khtml-nbsp-mode: space; -khtml-line-break: after-white-space;">
+
+Fixes this bug:
+<a href="rdar://problem/3941203">&lt;rdar://problem/3927554&gt;</a> REGRESSION (Mail): Paste inserts content in wrong place
+
+<br>***TEST*** line should be second, following the first line.
+
+<div style="height: 12px"></div>
+
+<div id="root">
+<div id="test" class="editing"><p>Should be first line of document.<br><br>Another line.</p><p></p>&nbsp;<p></p>&nbsp;<p>***TEST***</p><p><br></p></div>
+</div>
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
index 73d4888247683a508c93e588701f3c20ced22280..ca04ff60194b73bc4ce0ca5c66c99c2433d0632f 100644 (file)
@@ -1,3 +1,22 @@
+2005-01-05  Ken Kocienda  <kocienda@apple.com>
+
+        Reviewed by Hyatt
+
+        Fix for this bug:
+        
+        <rdar://problem/3941203> REGRESSION (Mail): Paste inserts content in wrong place
+
+        * khtml/editing/htmlediting.cpp:
+        (khtml::ReplaceSelectionCommand::doApply): Some cleanup and refinement of the concepts used to make
+        this operation work correctly, particularly in the logic to figure out whether to merge content, and
+        also performing merges.
+        * khtml/editing/visible_position.cpp:
+        (khtml::isFirstVisiblePositionInBlock): Simplification of test used to make this determination.
+        * khtml/editing/visible_units.cpp:
+        (khtml::isStartOfParagraph): New helper, used in khtml::ReplaceSelectionCommand::doApply().
+        (khtml::isEndOfParagraph): Ditto.
+        * khtml/editing/visible_units.h: Declare new functions.
+
 2005-01-04  Ken Kocienda  <kocienda@apple.com>
 
         Reviewed by John
index 0e199e44e9d125d2c0db97a211e3abf595d494a3..0d876aca1ad7f469e22116f33b36c5a5959502cb 100644 (file)
@@ -3404,7 +3404,6 @@ void ReplaceSelectionCommand::doApply()
     Selection selection = endingSelection();
     VisiblePosition visibleStart(selection.start());
     VisiblePosition visibleEnd(selection.end());
-    bool startAtStartOfLine = isFirstVisiblePositionOnLine(visibleStart);
     bool startAtStartOfBlock = isFirstVisiblePositionInBlock(visibleStart);
     bool startAtEndOfBlock = isLastVisiblePositionInBlock(visibleStart);
     bool startAtBlockBoundary = startAtStartOfBlock || startAtEndOfBlock;
@@ -3416,13 +3415,9 @@ void ReplaceSelectionCommand::doApply()
         // Empty document. Merge neither start nor end.
         mergeStart = mergeEnd = false;
     }
-    else if (startBlock == endBlock && startAtStartOfBlock && startAtEndOfBlock) {
-        // Merge start only if insertion point is in an empty block.
-        mergeStart = true;
-    }
     else {
-        mergeStart = !(startAtStartOfLine && (m_fragment.hasInterchangeNewline() || m_fragment.hasMoreThanOneBlock()));
-        mergeEnd = !m_fragment.hasInterchangeNewline() && m_fragment.hasMoreThanOneBlock();
+        mergeStart = (m_fragment.hasInterchangeNewline() || m_fragment.hasMoreThanOneBlock()) && !isStartOfParagraph(visibleStart);
+        mergeEnd = !m_fragment.hasInterchangeNewline() && m_fragment.hasMoreThanOneBlock() && !isEndOfParagraph(visibleEnd);
     }
     
     Position startPos = Position(selection.start().node()->enclosingBlockFlowElement(), 0);
@@ -3463,7 +3458,9 @@ void ReplaceSelectionCommand::doApply()
     // Now that we are about to add content, check to see if a placeholder element
     // can be removed.
     NodeImpl *block = startPos.node()->enclosingBlockFlowElement();
+    NodeImpl *placeholderBlock = 0;
     if (removeBlockPlaceholderIfNeeded(block)) {
+        placeholderBlock = block;
         Position pos = Position(block, 0);
         if (!endPos.node()->inDocument()) // endPos might have been in the placeholder just removed.
             endPos = pos;
@@ -3491,10 +3488,7 @@ void ReplaceSelectionCommand::doApply()
 
     document()->updateLayout();
 
-    NodeImpl *refBlock = startPos.node()->enclosingBlockFlowElement();
     Position insertionPos = startPos;
-    bool insertBlocksBefore = true;
-
     NodeImpl *firstNodeInserted = 0;
     NodeImpl *lastNodeInserted = 0;
     bool lastNodeInsertedInMergeEnd = false;
@@ -3555,7 +3549,6 @@ void ReplaceSelectionCommand::doApply()
             else
                 insertionPos = Position(insertionNode->parentNode(), insertionNode->nodeIndex() + 1);
         }
-        insertBlocksBefore = false;
     }
 
     // prune empty nodes from fragment
@@ -3566,21 +3559,18 @@ void ReplaceSelectionCommand::doApply()
     if (node) {
         NodeImpl *refNode = node;
         NodeImpl *node = refNode ? refNode->nextSibling() : 0;
-        if (isProbablyBlock(refNode) && (insertBlocksBefore || startAtStartOfBlock) && !mergeStart) {
-            if (refBlock->id() == ID_BODY)
-                insertNodeAt(refNode, refBlock, 0);
-            else
-                insertNodeBefore(refNode, refBlock);
-        }
-        else if (isProbablyBlock(refNode) && startAtEndOfBlock && !mergeEnd) {
-            if (refBlock->id() == ID_BODY)
-                appendNode(refNode, refBlock);
-            else
-                insertNodeAfter(refNode, refBlock);
+        VisiblePosition visiblePos(insertionPos);
+        bool insertionNodeIsBody = insertionPos.node()->id() == ID_BODY;
+        if (!mergeStart && !insertionNodeIsBody && isProbablyBlock(refNode) && isStartOfParagraph(visiblePos)) {
+            Position pos = insertionPos;
+            if (!insertionPos.node()->isTextNode() && !insertionPos.node()->isBlockFlow() && insertionPos.offset() > 0)
+                pos = insertionPos.downstream(StayInBlock);
+            insertNodeBefore(refNode, pos.node());
         }
-        else {
+        else if (!mergeEnd && !insertionNodeIsBody && isProbablyBlock(refNode) && isEndOfParagraph(visiblePos))
+            insertNodeAfter(refNode, insertionPos.node());
+        else
             insertNodeAt(refNode, insertionPos.node(), insertionPos.offset());
-        }
         if (!firstNodeInserted)
             firstNodeInserted = refNode;
         if (!lastNodeInsertedInMergeEnd)
@@ -3652,6 +3642,10 @@ void ReplaceSelectionCommand::doApply()
         }
         completeHTMLReplacement(firstNodeInserted, lastNodeInserted);
     }
+    
+    if (placeholderBlock && placeholderBlock->childNodeCount() == 0) {
+        removeNode(placeholderBlock);
+    }
 }
 
 void ReplaceSelectionCommand::completeHTMLReplacement(const Position &start, const Position &end)
index 8a581461f7e75b9ef75d5591feba26405cd115ce..3c56ebb6bf6afb788cce7e6baa82cae62168b338 100644 (file)
@@ -535,23 +535,9 @@ bool isFirstVisiblePositionInBlock(const VisiblePosition &pos)
 {
     if (pos.isNull())
         return false;
-        
-    VisiblePosition previous = pos.previous();
-    if (previous.isNull())
-        return true;
-    
-    switch (blockRelationship(pos, previous)) {
-        case NoBlockRelationship:
-        case SameBlockRelationship:
-        case AncestorBlockRelationship:
-        case OtherBlockRelationship:
-            return false;
-        case PeerBlockRelationship:
-        case DescendantBlockRelationship:
-            return true;
-    }
-    ASSERT_NOT_REACHED();
-    return false;
+
+    Position upstream = pos.deepEquivalent().upstream(StayInBlock);
+    return upstream.node()->isBlockFlow() && upstream.offset() == 0;
 }
 
 bool isFirstVisiblePositionInNode(const VisiblePosition &pos, const NodeImpl *node)
index 25db0143f3050dcc7f5f1c5a917718be4b8e8a6f..18d949814c6b659036ae5fb196b6d52237908ff1 100644 (file)
@@ -343,6 +343,16 @@ bool inSameParagraph(const VisiblePosition &a, const VisiblePosition &b)
     return a == b || startOfParagraph(a) == startOfParagraph(b);
 }
 
+bool isStartOfParagraph(const VisiblePosition &pos)
+{
+    return pos == startOfParagraph(pos);
+}
+
+bool isEndOfParagraph(const VisiblePosition &pos)
+{
+    return pos == endOfParagraph(pos, DoNotIncludeLineBreak);
+}
+
 VisiblePosition previousParagraphPosition(const VisiblePosition &p, EAffinity affinity, int x)
 {
     VisiblePosition pos = p;
index 80c5377939f83baff1ee0807cfbf5cc48ca2e72d..78bab53c45a3facb33ba03ffaf0d5364dbb4658b 100644 (file)
@@ -53,6 +53,8 @@ VisiblePosition endOfParagraph(const VisiblePosition &, EIncludeLineBreak = DoNo
 VisiblePosition previousParagraphPosition(const VisiblePosition &, EAffinity, int x);
 VisiblePosition nextParagraphPosition(const VisiblePosition &, EAffinity, int x);
 bool inSameParagraph(const VisiblePosition &, const VisiblePosition &);
+bool isStartOfParagraph(const VisiblePosition &);
+bool isEndOfParagraph(const VisiblePosition &);
 
 } // namespace DOM