Reviewed by John
authorkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Feb 2005 17:01:57 +0000 (17:01 +0000)
committerkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Feb 2005 17:01:57 +0000 (17:01 +0000)
        Fix for this bug:

        <rdar://problem/3951178> REGRESSION (Mail): blank line lost after pasting as quotation

        Problem was that the blank line after the selection was getting deleted incorrectly since the
        paste code thought this was an unneeded placeholder rather than a placeholder outside of the
        selection.

        * khtml/editing/htmlediting.cpp:
        (khtml::CompositeEditCommand::removeBlockPlaceholderIfNeeded): Now calls findBlockPlaceholder.
        (khtml::CompositeEditCommand::findBlockPlaceholder): Moved finding code formerly in
        removeBlockPlaceholderIfNeeded to this new helper.
        (khtml::ReplaceSelectionCommand::doApply): Do not delete placeholder up front. Call
        findBlockPlaceholder, and delete it later if needed in the already-existing cleanup step.
        * khtml/editing/htmlediting.h: Add new function.

        New layout test.

        * layout-tests/editing/pasteboard/paste-text-017-expected.txt: Added.
        * layout-tests/editing/pasteboard/paste-text-017.html: Added.

        Result changed for the better.

        * layout-tests/editing/pasteboard/paste-text-011-expected.txt

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

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

index 5ddbcc4..944ffaf 100644 (file)
@@ -14,16 +14,16 @@ layer at (0,0) size 800x600
             RenderText {TEXT} at (0,0) size 39x18
               text run at (0,0) width 39: "there"
       RenderBlock {P} at (0,68) size 784x18
-        RenderInline {FONT} at (0,0) size 37x18
-          RenderInline {B} at (0,0) size 37x18
-            RenderText {TEXT} at (0,0) size 37x18
-              text run at (0,0) width 37: "hello"
+        RenderInline {B} at (0,0) size 37x18
+          RenderText {TEXT} at (0,0) size 37x18
+            text run at (0,0) width 37: "hello"
       RenderBlock {P} at (0,102) size 784x18
-        RenderInline {FONT} at (0,0) size 39x18
-          RenderInline {B} at (0,0) size 39x18
-            RenderText {TEXT} at (0,0) size 39x18
-              text run at (0,0) width 39: "there"
+        RenderInline {B} at (0,0) size 39x18
+          RenderText {TEXT} at (0,0) size 39x18
+            text run at (0,0) width 39: "there"
+          RenderInline {SPAN} at (0,0) size 0x18
+            RenderInline {B} at (0,0) size 0x18
 selection is CARET:
-start:      position 5 of child 1 {TEXT} of child 1 {B} of child 1 {FONT} of child 5 {P} of root {BODY}
-upstream:   position 5 of child 1 {TEXT} of child 1 {B} of child 1 {FONT} of child 5 {P} of root {BODY}
-downstream: position 5 of child 1 {TEXT} of child 1 {B} of child 1 {FONT} of child 5 {P} of root {BODY}
+start:      position 5 of child 1 {TEXT} of child 1 {B} of child 5 {P} of root {BODY}
+upstream:   position 5 of child 1 {TEXT} of child 1 {B} of child 5 {P} of root {BODY}
+downstream: position 1 of child 1 {B} of child 2 {SPAN} of child 1 {B} of child 5 {P} of root {BODY}
diff --git a/LayoutTests/editing/pasteboard/paste-text-017-expected.txt b/LayoutTests/editing/pasteboard/paste-text-017-expected.txt
new file mode 100644 (file)
index 0000000..6105998
--- /dev/null
@@ -0,0 +1,43 @@
+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 {DIV} at (0,0) size 784x212 [border: (2px solid #0000FF)]
+        RenderBlock {DIV} at (14,14) size 756x112
+          RenderText {TEXT} at (0,0) size 67x28
+            text run at (0,0) width 67: "Tests: "
+          RenderBR {BR} at (0,0) size 0x0
+          RenderText {TEXT} at (0,28) size 742x56
+            text run at (0,28) width 742: "Copying and pasting a whole line followed by a blank line could remove the"
+            text run at (0,56) width 480: "blank line incorrectly, as in the case described in "
+          RenderInline {A} at (0,0) size 260x28 [color=#0000EE]
+            RenderText {TEXT} at (480,56) size 260x28
+              text run at (480,56) width 260: "<rdar://problem/3951178>"
+          RenderText {TEXT} at (0,84) size 623x28
+            text run at (0,84) width 623: "REGRESSION (Mail): blank line lost after pasting as quotation."
+        RenderBlock {DIV} at (14,142) size 756x56
+          RenderText {TEXT} at (0,0) size 189x28
+            text run at (0,0) width 189: "Expected Results: "
+          RenderBR {BR} at (0,0) size 0x0
+          RenderText {TEXT} at (0,28) size 490x28
+            text run at (0,28) width 490: "Should see a blank line between \"two\" and \"three\""
+      RenderBlock {DIV} at (0,236) size 784x144
+        RenderBlock {DIV} at (0,0) size 784x144 [border: (2px solid #FF0000)]
+          RenderBlock {P} at (2,2) size 780x28
+            RenderText {TEXT} at (0,0) size 35x28
+              text run at (0,0) width 35: "one"
+          RenderBlock {P} at (2,30) size 780x28
+            RenderBR {BR} at (0,0) size 0x28
+          RenderBlock {P} at (2,58) size 780x28
+            RenderText {TEXT} at (0,0) size 36x28
+              text run at (0,0) width 36: "two"
+          RenderBlock {P} at (2,86) size 780x28
+            RenderBR {BR} at (0,0) size 0x28
+          RenderBlock {P} at (2,114) size 780x28
+            RenderText {TEXT} at (0,0) size 49x28
+              text run at (0,0) width 49: "three"
+selection is CARET:
+start:      position 0 of child 1 {BR} of child 7 {P} of child 1 {DIV} of root {DIV}
+upstream:   position 0 of child 7 {P} of child 1 {DIV} of root {DIV}
+downstream: position 0 of child 1 {BR} of child 7 {P} of child 1 {DIV} of root {DIV}
diff --git a/LayoutTests/editing/pasteboard/paste-text-017.html b/LayoutTests/editing/pasteboard/paste-text-017.html
new file mode 100644 (file)
index 0000000..13c3d21
--- /dev/null
@@ -0,0 +1,76 @@
+<html> 
+<head>
+
+<style>
+.editing { 
+    border: 2px solid red; 
+    font-size: 24px; 
+}
+.explanation { 
+    border: 2px solid blue; 
+    padding: 12px; 
+    font-size: 24px; 
+    margin-bottom: 24px;
+}
+.scenario { margin-bottom: 16px;}
+.scenario:first-line { font-weight: bold; margin-bottom: 16px;}
+.expected-results:first-line { font-weight: bold }
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    moveSelectionForwardByLineCommand();
+    moveSelectionForwardByLineCommand();
+    extendSelectionForwardByLineCommand();
+    copyCommand();
+    pasteCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body>
+
+<div class="explanation">
+<div class="scenario">
+Tests: 
+<br>
+Copying and pasting a whole line followed by a blank line could remove the blank line incorrectly, as in the case described in 
+<a href="rdar://problem/3951178">&lt;rdar://problem/3951178&gt;</a> REGRESSION (Mail): blank line lost after pasting as quotation.
+</div>
+<div class="expected-results">
+Expected Results:
+<br>
+Should see a blank line between "two" and "three"
+</div>
+</div>
+
+<div contenteditable id="root" style="word-wrap: break-word; -khtml-nbsp-mode: space; -khtml-line-break: after-white-space;">
+<div id="test" class="editing">
+<p style="margin-top: 0px; margin-bottom: 0px; ">
+one
+</p>
+<p style="margin-top: 0px; margin-bottom: 0px; ">
+    <br class="khtml-block-placeholder">
+</p>
+<p style="margin-top: 0px; margin-bottom: 0px; ">
+    two
+</p>
+<p style="margin-top: 0px; margin-bottom: 0px; ">
+    <br class="khtml-block-placeholder">
+</p>
+<p style="margin-top: 0px; margin-bottom: 0px; ">
+    three
+</p>
+</div>
+</div>
+
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
index 47aa148..06fba26 100644 (file)
@@ -1,9 +1,42 @@
+2005-02-15  Ken Kocienda  <kocienda@apple.com>
+
+        Reviewed by John
+        
+        Fix for this bug:
+        
+        <rdar://problem/3951178> REGRESSION (Mail): blank line lost after pasting as quotation
+        
+        Problem was that the blank line after the selection was getting deleted incorrectly since the
+        paste code thought this was an unneeded placeholder rather than a placeholder outside of the
+        selection.
+
+        * khtml/editing/htmlediting.cpp:
+        (khtml::CompositeEditCommand::removeBlockPlaceholderIfNeeded): Now calls findBlockPlaceholder.
+        (khtml::CompositeEditCommand::findBlockPlaceholder): Moved finding code formerly in 
+        removeBlockPlaceholderIfNeeded to this new helper.
+        (khtml::ReplaceSelectionCommand::doApply): Do not delete placeholder up front. Call 
+        findBlockPlaceholder, and delete it later if needed in the already-existing cleanup step.
+        * khtml/editing/htmlediting.h: Add new function.
+
+        New layout test.
+
+        * layout-tests/editing/pasteboard/paste-text-017-expected.txt: Added.
+        * layout-tests/editing/pasteboard/paste-text-017.html: Added.
+
+        Result changed for the better.
+
+        * layout-tests/editing/pasteboard/paste-text-011-expected.txt
+
 2005-02-14  David Harrison  <harrison@apple.com>
 
         Reviewed by Darin.
 
         <rdar://problem/4004305> REGRESSION (Mail): Command-right-arrow on wrapped text goes to end of previous line
 
+        Reviewed by Darin.
+
+        <rdar://problem/4004305> REGRESSION (Mail): Command-right-arrow on wrapped text goes to end of previous line
+
         * khtml/editing/visible_text.cpp:
         (khtml::SimplifiedBackwardsTextIterator::advance):
         Add BR in for <rdar://problem/3917929> fix only if leaving a visible text node.
index 1aaa46a..ca5b817 100644 (file)
@@ -1032,27 +1032,36 @@ bool CompositeEditCommand::insertBlockPlaceholderIfNeeded(NodeImpl *node)
 
 bool CompositeEditCommand::removeBlockPlaceholderIfNeeded(NodeImpl *node)
 {
+    NodeImpl *placeholder = findBlockPlaceholder(node);
+    if (placeholder) {
+        removeNode(placeholder);
+        return true;
+    }
+    return false;
+}
+
+NodeImpl *CompositeEditCommand::findBlockPlaceholder(NodeImpl *node)
+{
     if (!node)
-        return false;
+        return 0;
 
     document()->updateLayout();
 
     RenderObject *renderer = node->renderer();
     if (!renderer || !renderer->isBlockFlow())
-        return false;
+        return 0;
 
     for (NodeImpl *checkMe = node; checkMe; checkMe = checkMe->traverseNextNode(node)) {
         if (checkMe->isElementNode()) {
             ElementImpl *element = static_cast<ElementImpl *>(checkMe);
             if (element->enclosingBlockFlowElement() == node && 
                 element->getAttribute(ATTR_CLASS) == blockPlaceholderClassString()) {
-                removeNode(element);
-                return true;
+                return element;
             }
         }
     }
     
-    return false;
+    return 0;
 }
 
 void CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary(const Position &pos)
@@ -4308,15 +4317,7 @@ void ReplaceSelectionCommand::doApply()
     // now that we are about to add content, check whether a placeholder element can be removed
     // if so, do it and update startPos and endPos
     NodeImpl *block = startPos.node()->enclosingBlockFlowElement();
-    NodeImpl *placeholderBlock = 0;
-    if (removeBlockPlaceholderIfNeeded(block)) {
-        placeholderBlock = block;
-        Position pos = Position(block, 0);
-        // endPos might have been in the placeholder just removed
-        if (!endPos.node()->inDocument())
-            endPos = pos;
-        startPos = pos;
-    }
+    NodeImpl *placeholderBlock = findBlockPlaceholder(block);
     
     // check whether to "smart replace" needs to add leading and/or trailing space
     bool addLeadingSpace = false;
index ea21ab1..c9c164c 100644 (file)
@@ -232,6 +232,7 @@ protected:
     void insertBlockPlaceholder(DOM::NodeImpl *);
     bool insertBlockPlaceholderIfNeeded(DOM::NodeImpl *);
     bool removeBlockPlaceholderIfNeeded(DOM::NodeImpl *);
+    DOM::NodeImpl *findBlockPlaceholder(DOM::NodeImpl *);
 
     void moveParagraphContentsToNewBlockIfNecessary(const DOM::Position &);