2011-05-20 Ryosuke Niwa <rniwa@webkit.org>
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 May 2011 21:23:23 +0000 (21:23 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 May 2011 21:23:23 +0000 (21:23 +0000)
        Reviewed by Enrica Casucci.

        Wrap copied contents by one style span instead of two
        https://bugs.webkit.org/show_bug.cgi?id=60988

        Rebaselined tests due to the change in how WebKit preserves style in copy and paste.

        * editing/pasteboard/4930986-2-expected.txt: Whitespace change.
        * editing/pasteboard/5065605-expected.txt: No longer adds redundant inline style declaration.
        * editing/pasteboard/paste-4039777-fix-expected.txt: Progression; Now we hit the list merging logic
        in ReplaceSelectionCommand: isStyleSpan(refNode.get()) && isListElement(refNode->firstChild()).
        * editing/pasteboard/paste-list-001-expected.txt: Ditto.
        * editing/pasteboard/paste-text-011-expected.txt: An extra style span was added.
        * editing/pasteboard/paste-text-012-expected.txt: Ditto.
        * editing/pasteboard/smart-paste-003-trailing-whitespace-expected.txt: No longer adds redundant style span.
        * platform/chromium-win/editing/pasteboard/paste-text-003-expected.txt: No longer adds empty anonymous nodes.
        * platform/chromium-win/editing/pasteboard/paste-text-011-expected.txt: Ditto.
        * platform/gtk/editing/pasteboard/paste-text-003-expected.txt: Ditto.
        * platform/mac/editing/pasteboard/paste-text-003-expected.txt: Ditto.
        * platform/qt/editing/pasteboard/paste-text-003-expected.txt: Ditto.
2011-05-20  Ryosuke Niwa  <rniwa@webkit.org>

        Reviewed by Enrica Casucci.

        Wrap copied contents by one style span instead of two
        https://bugs.webkit.org/show_bug.cgi?id=60988

        Replaced sourceDocumentStyleSpan and copiedRangeStyleSpan by one wrapping style span. Instead
        of wrapping the copied contents by user-applied style and document default style in serialization,
        take the difference with the document default's style in paste code.

        This will dramatically simplify our copy and paste code and pave a way to fix the bug 60914.

        No new tests because copy & paste is tested by existing layout tests.

        * editing/EditingStyle.cpp:
        (WebCore::EditingStyle::prepareToApplyAt): Remove the color property if RGBA values of color
        matches that of the computed style at the specified position.
        * editing/ReplaceSelectionCommand.cpp:
        (WebCore::ReplaceSelectionCommand::handleStyleSpans): Replaced sourceDocumentStyleSpan and
        copiedRangeStyleSpan by wrappingStyleSpan. When pasting as a quotation, compare style against
        the document's default style to avoid keeping the document default style (tested by
        editing/pasteboard/4930986-3.html).
        * editing/ReplaceSelectionCommand.h:
        * editing/markup.cpp:
        (WebCore::createMarkup): Only use one style span to wrap the serialized contents.

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

18 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/pasteboard/4930986-2-expected.txt
LayoutTests/editing/pasteboard/5065605-expected.txt
LayoutTests/editing/pasteboard/paste-4039777-fix-expected.txt
LayoutTests/editing/pasteboard/paste-list-001-expected.txt
LayoutTests/editing/pasteboard/paste-text-011-expected.txt
LayoutTests/editing/pasteboard/paste-text-012-expected.txt
LayoutTests/editing/pasteboard/smart-paste-003-trailing-whitespace-expected.txt
LayoutTests/platform/chromium-win/editing/pasteboard/paste-text-003-expected.txt
LayoutTests/platform/chromium-win/editing/pasteboard/paste-text-011-expected.txt
LayoutTests/platform/gtk/editing/pasteboard/paste-text-003-expected.txt
LayoutTests/platform/mac/editing/pasteboard/paste-text-003-expected.txt
LayoutTests/platform/qt/editing/pasteboard/paste-text-003-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/editing/EditingStyle.cpp
Source/WebCore/editing/ReplaceSelectionCommand.cpp
Source/WebCore/editing/ReplaceSelectionCommand.h
Source/WebCore/editing/markup.cpp

index 7b0a480..837aa4f 100644 (file)
@@ -1,3 +1,26 @@
+2011-05-20  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Enrica Casucci.
+
+        Wrap copied contents by one style span instead of two
+        https://bugs.webkit.org/show_bug.cgi?id=60988
+
+        Rebaselined tests due to the change in how WebKit preserves style in copy and paste.
+
+        * editing/pasteboard/4930986-2-expected.txt: Whitespace change.
+        * editing/pasteboard/5065605-expected.txt: No longer adds redundant inline style declaration.
+        * editing/pasteboard/paste-4039777-fix-expected.txt: Progression; Now we hit the list merging logic
+        in ReplaceSelectionCommand: isStyleSpan(refNode.get()) && isListElement(refNode->firstChild()).
+        * editing/pasteboard/paste-list-001-expected.txt: Ditto.
+        * editing/pasteboard/paste-text-011-expected.txt: An extra style span was added.
+        * editing/pasteboard/paste-text-012-expected.txt: Ditto.
+        * editing/pasteboard/smart-paste-003-trailing-whitespace-expected.txt: No longer adds redundant style span.
+        * platform/chromium-win/editing/pasteboard/paste-text-003-expected.txt: No longer adds empty anonymous nodes.
+        * platform/chromium-win/editing/pasteboard/paste-text-011-expected.txt: Ditto.
+        * platform/gtk/editing/pasteboard/paste-text-003-expected.txt: Ditto.
+        * platform/mac/editing/pasteboard/paste-text-003-expected.txt: Ditto.
+        * platform/qt/editing/pasteboard/paste-text-003-expected.txt: Ditto.
+
 2011-05-20  Justin Schuh  <jschuh@chromium.org>
 
         Unreviewed.
index c0bd830..8853db1 100644 (file)
@@ -1,2 +1,2 @@
 This tests to make sure that content that is colored by the user is pasted with that color during a Paste as Quotation.
-<blockquote><span class="Apple-style-span" style="color: red; ">This text should be red (it should be wrapped in a style span).</span></blockquote>
+<blockquote><span class="Apple-style-span" style="color: red;">This text should be red (it should be wrapped in a style span).</span></blockquote>
index f81b204..51ccd08 100644 (file)
@@ -21,7 +21,7 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 24 of #text > FONT > DIV > FONT > DIV > DIV > BODY > HTML > #document to 24 of #text > FONT > DIV > FONT > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 24 of #text > FONT > DIV > SPAN > FONT > DIV > DIV > BODY > HTML > #document to 24 of #text > FONT > DIV > SPAN > FONT > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
@@ -47,9 +47,8 @@ This tests for a bug where text copied with Select All + Copy would lose its col
 |         class="Apple-style-span"
 |         color="#ff0000"
 |         "This text should be red."
-|     <div>
-|       style="color: rgb(0, 0, 0); "
-|       <font>
-|         class="Apple-style-span"
-|         color="#ff0000"
-|         "This text should be red.<#selection-caret>"
+|       <div>
+|         <font>
+|           class="Apple-style-span"
+|           color="#ff0000"
+|           "This text should be red.<#selection-caret>"
index 9c0ee00..78dc4c6 100644 (file)
@@ -5,7 +5,7 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of #text > LI > UL > DIV > DIV > BODY > HTML > #document to 14 of #text > LI > UL > DIV > DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 1 of #text > LI > UL > DIV > LI > UL > DIV > DIV > BODY > HTML > #document to 1 of #text > LI > UL > DIV > LI > UL > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 1 of #text > LI > UL > DIV > UL > DIV > DIV > BODY > HTML > #document to 1 of #text > LI > UL > DIV > UL > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
@@ -51,20 +51,19 @@ Actual result:
 "
 | <ul>
 |   style="text-align:right;"
-|   <li>
+|   <ul>
+|     style="text-align: right; "
+|     <li>
+|       "A"
+|   <div>
 |     <ul>
-|       style="text-align: right; "
 |       <li>
-|         "A"
-|     <div>
-|       <ul>
-|         <li>
-|           <a>
-|             href=""
-|             "B"
-|           " "
-|           <br>
-|           "C<#selection-caret>"
+|         <a>
+|           href=""
+|           "B"
+|         " "
+|         <br>
+|         "C<#selection-caret>"
 | <div>
 |   <ul>
 |     "
index 18f478d..7085ffc 100644 (file)
@@ -7,7 +7,7 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 21 of #text > LI > OL > DIV > BODY > HTML > #document to 21 of #text > LI > OL > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 21 of #text > LI > OL > DIV > BODY > HTML > #document to 21 of #text > LI > OL > DIV > BODY > HTML > #document toDOMRange:range from 21 of #text > LI > OL > LI > OL > DIV > BODY > HTML > #document to 21 of #text > LI > OL > LI > OL > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 21 of #text > LI > OL > DIV > BODY > HTML > #document to 21 of #text > LI > OL > DIV > BODY > HTML > #document toDOMRange:range from 21 of #text > LI > OL > DIV > BODY > HTML > #document to 21 of #text > LI > OL > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
@@ -34,12 +34,11 @@ EDITING DELEGATE: webViewDidEndEditing:WebViewDidEndEditingNotification
     "
 |   <li>
 |     "I should be number 3."
-|     <ol>
-|       <li>
-|         "I should be number 1."
-|       <li>
-|       <li>
-|         "I should be number 3.<#selection-caret>"
+|   <li>
+|     "I should be number 1."
+|   <li>
+|   <li>
+|     "I should be number 3.<#selection-caret>"
 |   "
 "
 | "
index 9873762..6a3bb35 100644 (file)
@@ -6,7 +6,7 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of P > BODY > HTML > #document to 0 of P > BODY > HTML > #document givenAction:WebViewInsertActionPasted
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 5 of #text > B > FONT > P > B > FONT > P > BODY > HTML > #document to 5 of #text > B > FONT > P > B > FONT > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 5 of #text > B > FONT > P > SPAN > B > FONT > P > BODY > HTML > #document to 5 of #text > B > FONT > P > SPAN > B > FONT > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
@@ -30,18 +30,19 @@ The content was inserted at the start of the block instead of the end. This test
 |   <font>
 |     face="Monaco"
 |     <b>
-|       <p>
+|       <span>
+|         class="Apple-style-span"
 |         style="font-family: Times; font-weight: normal; "
-|         <font>
-|           face="Monaco"
-|           <b>
-|             "hello"
-|       <p>
-|         style="font-family: Times; font-weight: normal; "
-|         <font>
-|           face="Monaco"
-|           <b>
-|             "there<#selection-caret>"
+|         <p>
+|           <font>
+|             face="Monaco"
+|             <b>
+|               "hello"
+|         <p>
+|           <font>
+|             face="Monaco"
+|             <b>
+|               "there<#selection-caret>"
 | "
 
 
index 5e8ea51..85fbbf7 100644 (file)
@@ -6,4 +6,4 @@ foo
 foo
 
 execCopyCommand: <div id="test" class="editing"><div><blockquote>foo</blockquote></div></div> <div class="editing"></div>
-execPasteCommand: <div id="test" class="editing"><div><blockquote>foo</blockquote></div></div> <div class="editing"><div id="test" class="editing" style="border-top-width: 2px; border-right-width: 2px; border-bottom-width: 2px; border-left-width: 2px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: red; border-right-color: red; border-bottom-color: red; border-left-color: red; padding-top: 12px; padding-right: 12px; padding-bottom: 12px; padding-left: 12px; font-size: 24px; "><div><blockquote>foo</blockquote></div><div><br></div></div></div>
+execPasteCommand: <div id="test" class="editing"><div><blockquote>foo</blockquote></div></div> <div class="editing"><span class="Apple-style-span" style="font-size: medium; "><div id="test" class="editing" style="border-top-width: 2px; border-right-width: 2px; border-bottom-width: 2px; border-left-width: 2px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: red; border-right-color: red; border-bottom-color: red; border-left-color: red; padding-top: 12px; padding-right: 12px; padding-bottom: 12px; padding-left: 12px; font-size: 24px; "><div><blockquote>foo</blockquote></div><div><br></div></div></span></div>
index 903c56b..b5148cd 100644 (file)
@@ -9,4 +9,4 @@ A space should be added between the preexisting word and the word that's pasted.
 Actual result
 
 execCopyCommand: <div id="test" class="editing" style="font-size: 24px;"> hello world </div>
-execPasteCommand: <div id="test" class="editing" style="font-size: 24px;"> hello&nbsp;hello<span class="Apple-style-span" style="font-size: 24px; ">&nbsp;</span>world</div>
+execPasteCommand: <div id="test" class="editing" style="font-size: 24px;"> hello&nbsp;hello&nbsp;world</div>
index 59a00d3..4e8b8cb 100644 (file)
@@ -118,7 +118,6 @@ layer at (0,0) size 800x600
         RenderBlock {DIV} at (14,42) size 756x56 [border: (2px solid #FF0000)]
           RenderText {#text} at (14,14) size 431x27
             text run at (14,14) width 431: "Which taken at the flood leads on to fortune."
-        RenderBlock (anonymous) at (14,98) size 756x0
         RenderBlock {DIV} at (14,98) size 756x252 [border: (2px solid #FF0000)]
           RenderBlock (anonymous) at (14,14) size 728x0
           RenderBlock {DIV} at (14,14) size 728x56 [border: (2px solid #FF0000)]
@@ -130,7 +129,6 @@ layer at (0,0) size 800x600
           RenderBlock {DIV} at (14,70) size 728x56 [border: (2px solid #FF0000)]
             RenderText {#text} at (14,14) size 431x27
               text run at (14,14) width 431: "Which taken at the flood leads on to fortune."
-          RenderBlock (anonymous) at (14,126) size 728x0
           RenderBlock {DIV} at (14,126) size 728x112 [border: (2px solid #FF0000)]
             RenderBlock (anonymous) at (14,14) size 700x28
               RenderText {#text} at (0,0) size 78x27
index 9e4add4..48328e6 100644 (file)
@@ -6,7 +6,7 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of P > BODY > HTML > #document to 0 of P > BODY > HTML > #document givenAction:WebViewInsertActionPasted
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 5 of #text > B > FONT > P > B > FONT > P > BODY > HTML > #document to 5 of #text > B > FONT > P > B > FONT > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 5 of #text > B > FONT > P > SPAN > B > FONT > P > BODY > HTML > #document to 5 of #text > B > FONT > P > SPAN > B > FONT > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
@@ -30,18 +30,19 @@ The content was inserted at the start of the block instead of the end. This test
 |   <font>
 |     face="Monaco"
 |     <b>
-|       <p>
+|       <span>
+|         class="Apple-style-span"
 |         style="font-family: 'times new roman'; font-weight: normal; "
-|         <font>
-|           face="Monaco"
-|           <b>
-|             "hello"
-|       <p>
-|         style="font-family: 'times new roman'; font-weight: normal; "
-|         <font>
-|           face="Monaco"
-|           <b>
-|             "there<#selection-caret>"
+|         <p>
+|           <font>
+|             face="Monaco"
+|             <b>
+|               "hello"
+|         <p>
+|           <font>
+|             face="Monaco"
+|             <b>
+|               "there<#selection-caret>"
 | "
 
 
index 94db9bd..28d914b 100644 (file)
@@ -118,7 +118,6 @@ layer at (0,0) size 800x600
         RenderBlock {DIV} at (14,42) size 756x56 [border: (2px solid #FF0000)]
           RenderText {#text} at (14,14) size 434x28
             text run at (14,14) width 434: "Which taken at the flood leads on to fortune."
-        RenderBlock (anonymous) at (14,98) size 756x0
         RenderBlock {DIV} at (14,98) size 756x252 [border: (2px solid #FF0000)]
           RenderBlock (anonymous) at (14,14) size 728x0
           RenderBlock {DIV} at (14,14) size 728x56 [border: (2px solid #FF0000)]
@@ -130,7 +129,6 @@ layer at (0,0) size 800x600
           RenderBlock {DIV} at (14,70) size 728x56 [border: (2px solid #FF0000)]
             RenderText {#text} at (14,14) size 434x28
               text run at (14,14) width 434: "Which taken at the flood leads on to fortune."
-          RenderBlock (anonymous) at (14,126) size 728x0
           RenderBlock {DIV} at (14,126) size 728x112 [border: (2px solid #FF0000)]
             RenderBlock (anonymous) at (14,14) size 700x28
               RenderText {#text} at (0,0) size 80x28
index 94db9bd..28d914b 100644 (file)
@@ -118,7 +118,6 @@ layer at (0,0) size 800x600
         RenderBlock {DIV} at (14,42) size 756x56 [border: (2px solid #FF0000)]
           RenderText {#text} at (14,14) size 434x28
             text run at (14,14) width 434: "Which taken at the flood leads on to fortune."
-        RenderBlock (anonymous) at (14,98) size 756x0
         RenderBlock {DIV} at (14,98) size 756x252 [border: (2px solid #FF0000)]
           RenderBlock (anonymous) at (14,14) size 728x0
           RenderBlock {DIV} at (14,14) size 728x56 [border: (2px solid #FF0000)]
@@ -130,7 +129,6 @@ layer at (0,0) size 800x600
           RenderBlock {DIV} at (14,70) size 728x56 [border: (2px solid #FF0000)]
             RenderText {#text} at (14,14) size 434x28
               text run at (14,14) width 434: "Which taken at the flood leads on to fortune."
-          RenderBlock (anonymous) at (14,126) size 728x0
           RenderBlock {DIV} at (14,126) size 728x112 [border: (2px solid #FF0000)]
             RenderBlock (anonymous) at (14,14) size 700x28
               RenderText {#text} at (0,0) size 80x28
index d9146d1..61c7f79 100644 (file)
@@ -118,7 +118,6 @@ layer at (0,0) size 800x600
         RenderBlock {DIV} at (14,47) size 756x61 [border: (2px solid #FF0000)]
           RenderText {#text} at (14,14) size 456x33
             text run at (14,14) width 456: "Which taken at the flood leads on to fortune."
-        RenderBlock (anonymous) at (14,108) size 756x0
         RenderBlock {DIV} at (14,108) size 756x272 [border: (2px solid #FF0000)]
           RenderBlock (anonymous) at (14,14) size 728x0
           RenderBlock {DIV} at (14,14) size 728x61 [border: (2px solid #FF0000)]
@@ -130,7 +129,6 @@ layer at (0,0) size 800x600
           RenderBlock {DIV} at (14,75) size 728x61 [border: (2px solid #FF0000)]
             RenderText {#text} at (14,14) size 456x33
               text run at (14,14) width 456: "Which taken at the flood leads on to fortune."
-          RenderBlock (anonymous) at (14,136) size 728x0
           RenderBlock {DIV} at (14,136) size 728x122 [border: (2px solid #FF0000)]
             RenderBlock (anonymous) at (14,14) size 700x33
               RenderText {#text} at (0,0) size 86x33
index 54bc165..6fe7a00 100644 (file)
@@ -1,3 +1,30 @@
+2011-05-20  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Enrica Casucci.
+
+        Wrap copied contents by one style span instead of two
+        https://bugs.webkit.org/show_bug.cgi?id=60988
+
+        Replaced sourceDocumentStyleSpan and copiedRangeStyleSpan by one wrapping style span. Instead
+        of wrapping the copied contents by user-applied style and document default style in serialization,
+        take the difference with the document default's style in paste code.
+
+        This will dramatically simplify our copy and paste code and pave a way to fix the bug 60914.
+
+        No new tests because copy & paste is tested by existing layout tests.
+
+        * editing/EditingStyle.cpp:
+        (WebCore::EditingStyle::prepareToApplyAt): Remove the color property if RGBA values of color
+        matches that of the computed style at the specified position.
+        * editing/ReplaceSelectionCommand.cpp:
+        (WebCore::ReplaceSelectionCommand::handleStyleSpans): Replaced sourceDocumentStyleSpan and
+        copiedRangeStyleSpan by wrappingStyleSpan. When pasting as a quotation, compare style against
+        the document's default style to avoid keeping the document default style (tested by
+        editing/pasteboard/4930986-3.html).
+        * editing/ReplaceSelectionCommand.h:
+        * editing/markup.cpp:
+        (WebCore::createMarkup): Only use one style span to wrap the serialized contents.
+
 2011-05-20  Simon Fraser  <simon.fraser@apple.com>
 
         Reviewed by Sam Weinig.
index 2b50008..8aa0722 100644 (file)
@@ -90,6 +90,7 @@ static PassRefPtr<CSSMutableStyleDeclaration> editingStyleFromComputedStyle(Pass
 }
 
 static RefPtr<CSSMutableStyleDeclaration> getPropertiesNotIn(CSSStyleDeclaration* styleWithRedundantProperties, CSSStyleDeclaration* baseStyle);
+static RGBA32 getRGBAFontColor(CSSStyleDeclaration*);
 
 class HTMLElementEquivalent {
 public:
@@ -705,6 +706,8 @@ void EditingStyle::prepareToApplyAt(const Position& position, ShouldPreserveWrit
     }
 
     style->m_mutableStyle->diff(m_mutableStyle.get());
+    if (getRGBAFontColor(m_mutableStyle.get()) == getRGBAFontColor(style->m_mutableStyle.get()))
+        m_mutableStyle->removeProperty(CSSPropertyColor);
 
     // if alpha value is zero, we don't add the background color.
     RefPtr<CSSValue> backgroundColor = m_mutableStyle->getPropertyCSSValue(CSSPropertyBackgroundColor);
@@ -1006,7 +1009,6 @@ RefPtr<CSSMutableStyleDeclaration> getPropertiesNotIn(CSSStyleDeclaration* style
     return result;
 }
 
-
 int getIdentifierValue(CSSStyleDeclaration* style, int propertyID)
 {
     if (!style)
index eeef9ac..68ffa5a 100644 (file)
@@ -583,26 +583,17 @@ static bool handleStyleSpansBeforeInsertion(ReplacementFragment& fragment, const
     if (!isStyleSpan(topNode))
         return false;
 
-    Node* sourceDocumentStyleSpan = topNode;
-    RefPtr<Node> copiedRangeStyleSpan = sourceDocumentStyleSpan->firstChild();
-
+    Node* wrappingStyleSpan = topNode;
     RefPtr<EditingStyle> styleAtInsertionPos = EditingStyle::create(insertionPos.parentAnchoredEquivalent());
     String styleText = styleAtInsertionPos->style()->cssText();
 
     // FIXME: This string comparison is a naive way of comparing two styles.
     // We should be taking the diff and check that the diff is empty.
-    if (styleText == static_cast<Element*>(sourceDocumentStyleSpan)->getAttribute(styleAttr)) {
-        fragment.removeNodePreservingChildren(sourceDocumentStyleSpan);
-        if (!isStyleSpan(copiedRangeStyleSpan.get()))
-            return true;
-    }
-
-    if (isStyleSpan(copiedRangeStyleSpan.get()) && styleText == static_cast<Element*>(copiedRangeStyleSpan.get())->getAttribute(styleAttr)) {
-        fragment.removeNodePreservingChildren(copiedRangeStyleSpan.get());
-        return true;
-    }
+    if (styleText != static_cast<Element*>(wrappingStyleSpan)->getAttribute(styleAttr))
+        return false;
 
-    return false;
+    fragment.removeNodePreservingChildren(wrappingStyleSpan);
+    return true;
 }
 
 // At copy time, WebKit wraps copied content in a span that contains the source document's 
@@ -615,86 +606,45 @@ static bool handleStyleSpansBeforeInsertion(ReplacementFragment& fragment, const
 // or at copy time.
 void ReplaceSelectionCommand::handleStyleSpans()
 {
-    Node* sourceDocumentStyleSpan = 0;
-    Node* copiedRangeStyleSpan = 0;
+    HTMLElement* wrappingStyleSpan = 0;
     // The style span that contains the source document's default style should be at
     // the top of the fragment, but Mail sometimes adds a wrapper (for Paste As Quotation),
     // so search for the top level style span instead of assuming it's at the top.
     for (Node* node = m_firstNodeInserted.get(); node; node = node->traverseNextNode()) {
         if (isStyleSpan(node)) {
-            sourceDocumentStyleSpan = node;
-            // If the copied Range's common ancestor had user applied inheritable styles
-            // on it, they'll be on a second style span, just below the one that holds the 
-            // document defaults.
-            if (isStyleSpan(node->firstChild()))
-                copiedRangeStyleSpan = node->firstChild();
+            wrappingStyleSpan = toHTMLElement(node);
             break;
         }
     }
     
     // There might not be any style spans if we're pasting from another application or if 
     // we are here because of a document.execCommand("InsertHTML", ...) call.
-    if (!sourceDocumentStyleSpan)
+    if (!wrappingStyleSpan)
         return;
 
-    RefPtr<EditingStyle> sourceDocumentStyle = EditingStyle::create(toHTMLElement(sourceDocumentStyleSpan)->getInlineStyleDecl());
-    ContainerNode* context = sourceDocumentStyleSpan->parentNode();
+    RefPtr<EditingStyle> style = EditingStyle::create(wrappingStyleSpan->getInlineStyleDecl());
+    ContainerNode* context = wrappingStyleSpan->parentNode();
 
     // If Mail wraps the fragment with a Paste as Quotation blockquote, or if you're pasting into a quoted region,
     // styles from blockquoteNode are allowed to override those from the source document, see <rdar://problem/4930986> and <rdar://problem/5089327>.
     Node* blockquoteNode = isMailPasteAsQuotationNode(context) ? context : enclosingNodeOfType(firstPositionInNode(context), isMailBlockquote, CanCrossEditingBoundary);
-    if (blockquoteNode) {
-        sourceDocumentStyle->removeStyleConflictingWithStyleOfNode(blockquoteNode);
-        context = blockquoteNode->parentNode();
-    }
+    if (blockquoteNode)
+        context = document()->documentElement();
 
     // This operation requires that only editing styles to be removed from sourceDocumentStyle.
-    sourceDocumentStyle->prepareToApplyAt(firstPositionInNode(context));
+    style->prepareToApplyAt(firstPositionInNode(context));
 
     // Remove block properties in the span's style. This prevents properties that probably have no effect 
     // currently from affecting blocks later if the style is cloned for a new block element during a future 
     // editing operation.
     // FIXME: They *can* have an effect currently if blocks beneath the style span aren't individually marked
     // with block styles by the editing engine used to style them.  WebKit doesn't do this, but others might.
-    sourceDocumentStyle->removeBlockProperties();
-
-    // The styles on sourceDocumentStyleSpan are all redundant, and there is no copiedRangeStyleSpan
-    // to consider.  We're finished.
-    if (sourceDocumentStyle->isEmpty() && !copiedRangeStyleSpan) {
-        removeNodePreservingChildren(sourceDocumentStyleSpan);
-        return;
-    }
-
-    // There are non-redundant styles on sourceDocumentStyleSpan, but there is no
-    // copiedRangeStyleSpan.  Remove the span, because it could be surrounding block elements,
-    // and apply the styles to its children.
-    if (!sourceDocumentStyle->isEmpty() && !copiedRangeStyleSpan) {
-        copyStyleToChildren(sourceDocumentStyleSpan, sourceDocumentStyle->style()); 
-        removeNodePreservingChildren(sourceDocumentStyleSpan);
-        return;
-    }
-    
-    RefPtr<EditingStyle> copiedRangeStyle = EditingStyle::create(toHTMLElement(copiedRangeStyleSpan)->getInlineStyleDecl());
-
-    // We're going to put sourceDocumentStyleSpan's non-redundant styles onto copiedRangeStyleSpan,
-    // as long as they aren't overridden by ones on copiedRangeStyleSpan.
-    copiedRangeStyle->style()->merge(sourceDocumentStyle->style(), false);
+    style->removeBlockProperties();
 
-    removeNodePreservingChildren(sourceDocumentStyleSpan);
-
-    // Remove redundant styles.
-    context = copiedRangeStyleSpan->parentNode();
-    copiedRangeStyle->prepareToApplyAt(firstPositionInNode(context));
-    copiedRangeStyle->removeBlockProperties();
-    if (copiedRangeStyle->isEmpty()) {
-        removeNodePreservingChildren(copiedRangeStyleSpan);
-        return;
-    }
-
-    // Clear the redundant styles from the span's style attribute.
-    // FIXME: If font-family:-webkit-monospace is non-redundant, then the font-size should stay, even if it
-    // appears redundant.
-    setNodeAttribute(static_cast<Element*>(copiedRangeStyleSpan), styleAttr, copiedRangeStyle->style()->cssText());
+    if (style->isEmpty() || !wrappingStyleSpan->firstChild())
+        removeNodePreservingChildren(wrappingStyleSpan);
+    else
+        setNodeAttribute(wrappingStyleSpan, styleAttr, style->style()->cssText());
 }
 
 // Take the style attribute of a span and apply it to it's children instead.  This allows us to
index 77559f4..b4e094d 100644 (file)
@@ -70,7 +70,7 @@ private:
     bool shouldRemoveEndBR(Node*, const VisiblePosition&);
     
     bool shouldMergeStart(bool, bool, bool);
-    bool shouldMergeEnd(bool selectEndWasEndOfParagraph);
+    bool shouldMergeEnd(bool selectionEndWasEndOfParagraph);
     bool shouldMerge(const VisiblePosition&, const VisiblePosition&);
     
     void mergeEndIfNeeded();
index de99ed5..8d239bd 100644 (file)
@@ -640,28 +640,9 @@ String createMarkup(const Range* range, Vector<Node*>* nodes, EAnnotateForInterc
         // get the color of content pasted into blockquotes right.
         style->removeStyleAddedByNode(enclosingNodeOfType(firstPositionInNode(parentOfLastClosed), isMailBlockquote, CanCrossEditingBoundary));
 
-        // Document default styles will be added on another wrapper span.
-        if (document && document->documentElement())
-            style->prepareToApplyAt(firstPositionInNode(document->documentElement()));
-
-        // Since we are converting blocks to inlines, remove any inherited block properties that are in the style.
-        // This cuts out meaningless properties and prevents properties from magically affecting blocks later
-        // if the style is cloned for a new block element during a future editing operation.
-        if (convertBlocksToInlines)
-            style->removeBlockProperties();
-
         if (!style->isEmpty())
             accumulator.wrapWithStyleNode(style->style(), document);
     }
-    
-    if (lastClosed && lastClosed != document->documentElement()) {
-        // Add a style span with the document's default styles.  We add these in a separate
-        // span so that at paste time we can differentiate between document defaults and user
-        // applied styles.
-        RefPtr<EditingStyle> defaultStyle = EditingStyle::create(document->documentElement());
-        if (!defaultStyle->isEmpty())
-            accumulator.wrapWithStyleNode(defaultStyle->style(), document);
-    }
 
     // FIXME: The interchange newline should be placed in the block that it's in, not after all of the content, unconditionally.
     if (shouldAnnotate == AnnotateForInterchange && needInterchangeNewlineAfter(visibleEnd.previous()))