Reviewed by John
authorkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 13 Dec 2004 22:41:27 +0000 (22:41 +0000)
committerkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 13 Dec 2004 22:41:27 +0000 (22:41 +0000)
        Fix for this bug:

        <rdar://problem/3917863> REGRESSION (Mail): pasting two lines of plain text copied from an RTF document results in two styles

        Code to figuire out the end node to merge was missing the font tag in the second paragraph
        written out by AppKit convert-to-HTML function. I refined the algorithm to be smarter.

        * khtml/editing/htmlediting.cpp:
        (khtml::ReplacementFragment::mergeEndNode): Refine algorithm used to walk through the fragment being pasted
        looking for the node that is the last inline in the last block of the fragment. The old algorithm was
        insufficiently powerful.
        (khtml::ReplacementFragment::enclosingBlock): New helper function.
        * khtml/editing/htmlediting.h: Add declaration for new helper function.
        * layout-tests/editing/pasteboard/paste-text-011-expected.txt: Added.
        * layout-tests/editing/pasteboard/paste-text-011.html: Added.

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

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

diff --git a/LayoutTests/editing/pasteboard/paste-text-011-expected.txt b/LayoutTests/editing/pasteboard/paste-text-011-expected.txt
new file mode 100644 (file)
index 0000000..5ddbcc4
--- /dev/null
@@ -0,0 +1,29 @@
+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 784x576
+      RenderBlock {P} at (0,0) 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"
+      RenderBlock {P} at (0,34) 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"
+      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"
+      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"
+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}
diff --git a/LayoutTests/editing/pasteboard/paste-text-011.html b/LayoutTests/editing/pasteboard/paste-text-011.html
new file mode 100644 (file)
index 0000000..5e6e9dd
--- /dev/null
@@ -0,0 +1,40 @@
+
+<html> 
+<head>
+
+<style>
+.editing { 
+    border: 2px solid red; 
+    padding: 12px; 
+    font-size: 24px; 
+}
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+// There was a bug when pasting at the end of the block. The content was inserted at the 
+// start of the block instead of the end. This tests the insert-at-end case.
+
+function editingTest() {
+    selectAllCommand();
+    copyCommand();
+    moveSelectionForwardByCharacterCommand();
+    insertParagraphCommand();
+    pasteCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body contenteditable id="root" id="test">
+<p><font face="Monaco"><b>hello</b></font></p>
+<p><font face="Monaco"><b>there</b></font></p>
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
index cc15017038499d88a53c4f143c1b0487b1924539..8719f1c8527fd0ae50644a78a4762d64c4bb6bc0 100644 (file)
@@ -1,3 +1,23 @@
+2004-12-13  Ken Kocienda  <kocienda@apple.com>
+
+        Reviewed by John
+
+        Fix for this bug:
+        
+        <rdar://problem/3917863> REGRESSION (Mail): pasting two lines of plain text copied from an RTF document results in two styles
+
+        Code to figuire out the end node to merge was missing the font tag in the second paragraph
+        written out by AppKit convert-to-HTML function. I refined the algorithm to be smarter.
+
+        * khtml/editing/htmlediting.cpp:
+        (khtml::ReplacementFragment::mergeEndNode): Refine algorithm used to walk through the fragment being pasted
+        looking for the node that is the last inline in the last block of the fragment. The old algorithm was 
+        insufficiently powerful.
+        (khtml::ReplacementFragment::enclosingBlock): New helper function.
+        * khtml/editing/htmlediting.h: Add declaration for new helper function.
+        * layout-tests/editing/pasteboard/paste-text-011-expected.txt: Added.
+        * layout-tests/editing/pasteboard/paste-text-011.html: Added.
+
 2004-12-13  Ken Kocienda  <kocienda@apple.com>
 
         Reviewed by John
index 419737f34faacb9036c6133c22f397dff719df34..2c6c1f0f777d1e2ee382daf09ede555cccf052c6 100644 (file)
@@ -3127,19 +3127,19 @@ NodeImpl *ReplacementFragment::mergeEndNode() const
     NodeImpl *node = m_fragment->lastChild();
     while (node && node->lastChild())
         node = node->lastChild();
+    
+    if (isProbablyBlock(node))
+        return 0;
+        
+    NodeImpl *startingBlock = enclosingBlock(node);
+    ASSERT(startingBlock != node);
     while (node) {
         NodeImpl *prev = node->traversePreviousNode();
-        if (!isProbablyBlock(node)) {
-            NodeImpl *previousSibling = node->previousSibling();
-            while (1) {
-                if (!previousSibling || isProbablyBlock(previousSibling))
-                    return node;
-                node = previousSibling;
-                previousSibling = node->previousSibling();
-            }
-        }
+        if (prev == m_fragment || prev == startingBlock || enclosingBlock(prev) != startingBlock)
+            return node;
         node = prev;
     }
+    
     return 0;
 }
 
@@ -3176,6 +3176,13 @@ bool ReplacementFragment::isInterchangeConvertedSpaceSpan(const NodeImpl *node)
     return node->isHTMLElement() && static_cast<const HTMLElementImpl *>(node)->getAttribute(ATTR_CLASS) == convertedSpaceSpanClassString;
 }
 
+NodeImpl *ReplacementFragment::enclosingBlock(NodeImpl *node) const
+{
+    while (node && !isProbablyBlock(node))
+        node = node->parentNode();    
+    return node ? node : m_fragment;
+}
+
 void ReplacementFragment::removeNode(NodeImpl *node)
 {
     if (!node)
index 9eeb2a8d0e6f5724a8ce20acfe37eb887b12dbc7..c59ae77b1c2aa1e1694f6153d3a6970c37ef3c5d 100644 (file)
@@ -641,6 +641,7 @@ private:
     static bool isInterchangeConvertedSpaceSpan(const DOM::NodeImpl *);
 
     // A couple simple DOM helpers
+    DOM::NodeImpl *enclosingBlock(DOM::NodeImpl *) const;
     void removeNode(DOM::NodeImpl *);
     void insertNodeBefore(DOM::NodeImpl *node, DOM::NodeImpl *refNode);