Reviewed by Chris
authorkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Feb 2005 19:55:57 +0000 (19:55 +0000)
committerkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Feb 2005 19:55:57 +0000 (19:55 +0000)
        Fix for these bugs:

        <rdar://problem/4013025> Copy/Paste of quoted word results in removal of any following <cr>
        <rdar://problem/4013100> Copy/Paste quoted text and then decrease quote level does not change text color

        For the most part, these bugs were caused by errors and lack of foresight on my part when
        I added the better paste code. Chalk these fixes up to the result of bake time.

        * khtml/editing/htmlediting.cpp:
        (khtml::ReplacementFragment::ReplacementFragment): Need to move count of number of blocks in
        fragment after the call to remove unrendered nodes. Meant to do this before, but forgot to.
        (khtml::ReplacementFragment::removeStyleNodes): Need to remove inline styles from elements!
        Terrible omission now fixed.
        (khtml::ReplacementFragment::removeBlockquoteColorsIfNeeded): Remove blockquote colors for now.
        Code has a more extensive comment in it now to explain the difficulty, and the need for more
        study and changes.
        (khtml::ReplaceSelectionCommand::doApply): Need to call applyStyleToInsertedNodes() in the
        m_fragment.hasInterchangeNewline() case. This was just missed before.
        * layout-tests/editing/pasteboard/paste-text-011-expected.txt: Updated results, subtly different, but OK.
        * layout-tests/editing/pasteboard/paste-text-017-expected.txt: Updated for <p> to <div> change in test content.
        * layout-tests/editing/pasteboard/paste-text-017.html: Needed to change <p> to <div> to
        make this test go with the new design of using <div> tags for default paragraphs.

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

LayoutTests/editing/pasteboard/paste-text-011-expected.txt
LayoutTests/editing/pasteboard/paste-text-017-expected.txt
LayoutTests/editing/pasteboard/paste-text-017.html
WebCore/ChangeLog-2005-08-23
WebCore/khtml/editing/htmlediting.cpp

index 944ffaf847653f42a5e4061454af98169a94c411..b08a1af58904c108db27f2ca9311afcd64a436cb 100644 (file)
@@ -15,15 +15,17 @@ layer at (0,0) size 800x600
               text run at (0,0) width 39: "there"
       RenderBlock {P} at (0,68) size 784x18
         RenderInline {B} at (0,0) size 37x18
-          RenderText {TEXT} at (0,0) size 37x18
-            text run at (0,0) width 37: "hello"
+          RenderInline {SPAN} 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 {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 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 5 {P} of root {BODY}
-upstream:   position 5 of child 1 {TEXT} of child 1 {B} of child 5 {P} of root {BODY}
+start:      position 5 of child 1 {TEXT} of child 1 {SPAN} of child 1 {B} of child 5 {P} of root {BODY}
+upstream:   position 5 of child 1 {TEXT} of child 1 {SPAN} 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}
index 61059985c3a8180bd496352781d6a9a33a055f24..ff51801b2855884b906ba8518acd99189be86156 100644 (file)
@@ -24,20 +24,20 @@ layer at (0,0) size 800x600
             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
+          RenderBlock {DIV} 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
+          RenderBlock {DIV} at (2,30) size 780x28
             RenderBR {BR} at (0,0) size 0x28
-          RenderBlock {P} at (2,58) size 780x28
+          RenderBlock {DIV} 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
+          RenderBlock {DIV} at (2,86) size 780x28
             RenderBR {BR} at (0,0) size 0x28
-          RenderBlock {P} at (2,114) size 780x28
+          RenderBlock {DIV} 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}
+start:      position 0 of child 1 {BR} of child 7 {DIV} of child 1 {DIV} of root {DIV}
+upstream:   position 0 of child 7 {DIV} of child 1 {DIV} of root {DIV}
+downstream: position 0 of child 1 {BR} of child 7 {DIV} of child 1 {DIV} of root {DIV}
index 13c3d21afea1ae308c0142a621e9de39475ca4f8..b7ab7a31b8d4c8ea3500fc384c67c6a98a9c654a 100644 (file)
@@ -50,21 +50,21 @@ Should see a blank line between "two" and "three"
 
 <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; ">
+<div>
 one
-</p>
-<p style="margin-top: 0px; margin-bottom: 0px; ">
+</div>
+<div>
     <br class="khtml-block-placeholder">
-</p>
-<p style="margin-top: 0px; margin-bottom: 0px; ">
+</div>
+<div>
     two
-</p>
-<p style="margin-top: 0px; margin-bottom: 0px; ">
+</div>
+<div>
     <br class="khtml-block-placeholder">
-</p>
-<p style="margin-top: 0px; margin-bottom: 0px; ">
+</div>
+<div>
     three
-</p>
+</div>
 </div>
 </div>
 
index ca8e7f0ffb191b1161435b8076d03d0a03c60ae1..6ab813400d21d8ea2898ff8ff8a6cd27449e9a4b 100644 (file)
@@ -1,3 +1,30 @@
+2005-02-18  Ken Kocienda  <kocienda@apple.com>
+
+        Reviewed by Chris
+
+        Fix for these bugs:
+        
+        <rdar://problem/4013025> Copy/Paste of quoted word results in removal of any following <cr>
+        <rdar://problem/4013100> Copy/Paste quoted text and then decrease quote level does not change text color
+
+        For the most part, these bugs were caused by errors and lack of foresight on my part when
+        I added the better paste code. Chalk these fixes up to the result of bake time.
+
+        * khtml/editing/htmlediting.cpp:
+        (khtml::ReplacementFragment::ReplacementFragment): Need to move count of number of blocks in
+        fragment after the call to remove unrendered nodes. Meant to do this before, but forgot to.
+        (khtml::ReplacementFragment::removeStyleNodes): Need to remove inline styles from elements!
+        Terrible omission now fixed.
+        (khtml::ReplacementFragment::removeBlockquoteColorsIfNeeded): Remove blockquote colors for now.
+        Code has a more extensive comment in it now to explain the difficulty, and the need for more
+        study and changes.
+        (khtml::ReplaceSelectionCommand::doApply): Need to call applyStyleToInsertedNodes() in the 
+        m_fragment.hasInterchangeNewline() case. This was just missed before.
+        * layout-tests/editing/pasteboard/paste-text-011-expected.txt: Updated results, subtly different, but OK.
+        * layout-tests/editing/pasteboard/paste-text-017-expected.txt: Updated for <p> to <div> change in test content.
+        * layout-tests/editing/pasteboard/paste-text-017.html: Needed to change <p> to <div> to 
+        make this test go with the new design of using <div> tags for default paragraphs.
+
 2005-02-18  David Hyatt  <hyatt@apple.com>
 
        Fix for 3974263 (and possibly others).  Don't let fixed tables use maxint as their maxwidth when some
index 1060dfbd2cdc20f9e5248453d598799355a0ffc3..cb7f4c00229a12da7304c34f433e78d8e5693b7e 100644 (file)
@@ -3984,6 +3984,10 @@ ReplacementFragment::ReplacementFragment(DocumentImpl *document, DocumentFragmen
     if (nodeToDelete)
         removeNode(nodeToDelete);
 
+    // Prepare this fragment to merge styles correctly into the destination.
+    computeStylesAndRemoveUnrendered();
+    removeStyleNodes();
+
     int blockCount = realBlockCount;
     firstChild = m_fragment->firstChild();
     lastChild = m_fragment->lastChild();
@@ -3994,10 +3998,6 @@ ReplacementFragment::ReplacementFragment(DocumentImpl *document, DocumentFragmen
 
      if (blockCount > 1)
         m_hasMoreThanOneBlock = true;
-        
-    // Prepare this fragment to merge styles correctly into the destination.
-    computeStylesAndRemoveUnrendered();
-    removeStyleNodes();
 }
 
 ReplacementFragment::~ReplacementFragment()
@@ -4199,6 +4199,12 @@ void ReplacementFragment::removeStyleNodes()
             isStyleSpan(node)) {
             removeNodePreservingChildren(node);
         }
+        else if (node->isHTMLElement()) {
+            HTMLElementImpl *elem = static_cast<HTMLElementImpl *>(node);
+            CSSMutableStyleDeclarationImpl *inlineStyleDecl = elem->inlineStyleDecl();
+            if (inlineStyleDecl)
+                inlineStyleDecl->clear();
+        }
         node = next;
     }
 }
@@ -4211,10 +4217,12 @@ void ReplacementFragment::removeBlockquoteColorsIfNeeded(NodeImpl *node, CSSMuta
     if (NodeImpl *blockquote = closestMailBlockquote(node)) {
         CSSComputedStyleDeclarationImpl *blockquoteStyle = Position(blockquote, 0).computedStyle();
         if (blockquoteStyle->getPropertyValue(CSS_PROP_COLOR) == style->getPropertyValue(CSS_PROP_COLOR)) {
-            // Don't remove altogether. Set to the document's color.
-            CSSComputedStyleDeclarationImpl *documentStyle = Position(node->getDocument()->documentElement(), 0).computedStyle();
-            DOMString documentColor = documentStyle->getPropertyValue(CSS_PROP_COLOR);
-            style->setProperty(CSS_PROP_COLOR, documentColor, false, false);
+            // This is not quite right.
+            // The speculation now is that we want to set the color here to a pseudo-color that would
+            // make the content take on the color of the nearest-enclosing blockquote (if any) after
+            // being pasted in. However, this is not entirely clear, and requires more study.
+            // For now, just remove the blockquoted color.
+            style->removeProperty(CSS_PROP_COLOR);
         }
     }
 }
@@ -4261,6 +4269,13 @@ void ReplaceSelectionCommand::doApply()
     } else {
         // merge if current selection starts inside a paragraph, or there is only one block and no interchange newline to add
         mergeStart = !isStartOfParagraph(visibleStart) || (!m_fragment.hasInterchangeNewline() && !m_fragment.hasMoreThanOneBlock());
+        
+        // This is a workaround for this bug:
+        // <rdar://problem/4013642> REGRESSION (Mail): Copied quoted word does not paste as a quote if pasted at the start of a line
+        // We need more powerful logic in this whole mergeStart code for this case to come out right without
+        // breaking other cases.
+        if (isStartOfParagraph(visibleStart) && isMailBlockquote(m_fragment.firstChild()))
+            mergeStart = false;
     }
     
     // decide whether to later append nodes to the end
@@ -4425,6 +4440,7 @@ void ReplaceSelectionCommand::doApply()
             insertParagraphSeparator();
             endPos = endingSelection().end().downstream();
         }
+        applyStyleToInsertedNodes();
         completeHTMLReplacement(startPos, endPos);
     } else {
         if (m_lastNodeInserted && m_lastNodeInserted->id() == ID_BR && !document()->inStrictMode()) {