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 5ddbcc4950dc8833a28d738b7815389221873833..944ffaf847653f42a5e4061454af98169a94c411 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 47aa1487a093f7e8a0d881b28f448733a0219023..06fba26792c3bb687c74cf0f66dfb0dd8b088281 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 1aaa46a4b7478c20d374749ce5840239c00227d6..ca5b817b8311718c4f10742882977080541931b3 100644 (file)
@@ -1031,28 +1031,37 @@ 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 ea21ab175cb0dce0527ba5e19040ab6a61bc0f75..c9c164c12f5ef51310d7bc820632fc3a507ce11e 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 &);