Copying can result in span around block elements on the clipboard
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Aug 2011 06:29:18 +0000 (06:29 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Aug 2011 06:29:18 +0000 (06:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=34564

Reviewed by Tony Chang.

Source/WebCore:

Completely overhauled the way WebKit preserves style in copy and paste. Instead of wrapping the entire
serialized contents by a Apple style span, WebKit now adds inline style to the top level elements,
wrap top level text nodes by a style span.

* editing/EditingStyle.cpp:
(WebCore::EditingStyle::collapseTextDecorationProperties): Remove text-decoration property when the value
of -webkit-text-decorations-in-effect is none.
(WebCore::EditingStyle::removeStyleFromRulesAndContext): Since display: inline and float: none are now
added on copy, remove these properties on paste.
(WebCore::EditingStyle::removePropertiesInElementDefaultStyle): Takes Element* instead of StyledElement*.
(WebCore::EditingStyle::forceInline): Added.
(WebCore::getPropertiesNotIn): Remove properties only when the base style has them.
* editing/EditingStyle.h:
* editing/markup.cpp:
(WebCore::StyledMarkupAccumulator::shouldApplyWrappingStyle): Added.
(WebCore::StyledMarkupAccumulator::StyledMarkupAccumulator): Takes highestNodeToBeSerialized.
(WebCore::StyledMarkupAccumulator::wrapWithStyleNode): Calls appendStyleNodeOpenTag and styleNodeCloseTag.
(WebCore::StyledMarkupAccumulator::appendStyleNodeOpenTag): Extracted from wrapWithStyleNode.
(WebCore::StyledMarkupAccumulator::styleNodeCloseTag): Ditto.
(WebCore::StyledMarkupAccumulator::appendText): Wraps text node with a style span if needed.
Set display: inline and float: none so that it won't be converted to a block on paste side.
(WebCore::StyledMarkupAccumulator::appendElement): Add wrapping style if appropriate; Remove any properties
that are overridden by default style and any style that may conflict with the computed style of node to
avoid modifying the appearance of the serialized nodes.
(WebCore::StyledMarkupAccumulator::serializeNodes): Compute wrapping style; copies of this style are
modified as needed when serializing top-level elements or text nodes. We call traverseNodesForSerialization
with NodeTraversalMode set to DoNotEmitString first to compute the highest node to be serialized. The second
call to the function actually serialize the nodes.
(WebCore::StyledMarkupAccumulator::traverseNodesForSerialization): Extracted from serializeNodes.
Outputs string only if NodeTraversalMode is set to EmitString.
(WebCore::createMarkup): No longer adds wrapping spans.

LayoutTests:

This patch overhauled the way we preserve styles when copying and pasting HTML contents. Many tests progressed
and lost wrapping spans while others are observing different markup on the clipboard as expected.

* editing/deleting/deleting-line-break-preserves-underline-color-expected.txt: span's style attribute now has
a trailing space.
* editing/execCommand/insert-list-with-noneditable-content-expected.txt: LightGray is now rgb(211, 211, 211).
* editing/pasteboard/data-transfer-items-expected.txt: Different serialization.
* editing/pasteboard/onpaste-text-html-expected.txt: Ditto.
* editing/pasteboard/paste-4039777-fix-expected.txt: No longer nests ul erroneously or aligns the inner list
to the right.
* editing/pasteboard/paste-code-in-pre-expected.txt: No longer produces erroneous wrapping span.
* editing/pasteboard/paste-pre-001-expected.txt: Ditto.
* editing/pasteboard/paste-pre-002-expected.txt: Ditto.
* editing/pasteboard/paste-text-012-expected.txt: Ditto.
* editing/pasteboard/paste-text-with-style-4-expected.txt: Ditto.
* fast/events/ondrop-text-html-expected.txt: Different serialization.
* platform/chromium-win/editing/pasteboard/paste-code-in-pre-expected.txt: Removed.
* platform/chromium-win/editing/pasteboard/paste-pre-001-expected.txt: Removed.
* platform/chromium-win/editing/pasteboard/paste-pre-002-expected.txt: Removed.
* platform/qt-mac/editing/pasteboard/paste-code-in-pre-expected.txt: Removed.

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

20 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/deleting/deleting-line-break-preserves-underline-color-expected.txt
LayoutTests/editing/execCommand/insert-list-with-noneditable-content-expected.txt
LayoutTests/editing/pasteboard/data-transfer-items-expected.txt
LayoutTests/editing/pasteboard/onpaste-text-html-expected.txt
LayoutTests/editing/pasteboard/paste-4039777-fix-expected.txt
LayoutTests/editing/pasteboard/paste-code-in-pre-expected.txt
LayoutTests/editing/pasteboard/paste-pre-001-expected.txt
LayoutTests/editing/pasteboard/paste-pre-002-expected.txt
LayoutTests/editing/pasteboard/paste-text-012-expected.txt
LayoutTests/editing/pasteboard/paste-text-with-style-4-expected.txt
LayoutTests/fast/events/ondrop-text-html-expected.txt
LayoutTests/platform/chromium-win/editing/pasteboard/paste-code-in-pre-expected.txt [deleted file]
LayoutTests/platform/chromium-win/editing/pasteboard/paste-pre-001-expected.txt [deleted file]
LayoutTests/platform/chromium-win/editing/pasteboard/paste-pre-002-expected.txt [deleted file]
LayoutTests/platform/qt-mac/editing/pasteboard/paste-code-in-pre-expected.txt [deleted file]
Source/WebCore/ChangeLog
Source/WebCore/editing/EditingStyle.cpp
Source/WebCore/editing/EditingStyle.h
Source/WebCore/editing/markup.cpp

index 37ad724..27ac5c3 100644 (file)
@@ -1,5 +1,33 @@
 2011-08-10  Ryosuke Niwa  <rniwa@webkit.org>
 
+        Copying can result in span around block elements on the clipboard
+        https://bugs.webkit.org/show_bug.cgi?id=34564
+
+        Reviewed by Tony Chang.
+
+        This patch overhauled the way we preserve styles when copying and pasting HTML contents. Many tests progressed
+        and lost wrapping spans while others are observing different markup on the clipboard as expected.
+
+        * editing/deleting/deleting-line-break-preserves-underline-color-expected.txt: span's style attribute now has
+        a trailing space.
+        * editing/execCommand/insert-list-with-noneditable-content-expected.txt: LightGray is now rgb(211, 211, 211).
+        * editing/pasteboard/data-transfer-items-expected.txt: Different serialization.
+        * editing/pasteboard/onpaste-text-html-expected.txt: Ditto.
+        * editing/pasteboard/paste-4039777-fix-expected.txt: No longer nests ul erroneously or aligns the inner list
+        to the right.
+        * editing/pasteboard/paste-code-in-pre-expected.txt: No longer produces erroneous wrapping span.
+        * editing/pasteboard/paste-pre-001-expected.txt: Ditto.
+        * editing/pasteboard/paste-pre-002-expected.txt: Ditto.
+        * editing/pasteboard/paste-text-012-expected.txt: Ditto.
+        * editing/pasteboard/paste-text-with-style-4-expected.txt: Ditto.
+        * fast/events/ondrop-text-html-expected.txt: Different serialization.
+        * platform/chromium-win/editing/pasteboard/paste-code-in-pre-expected.txt: Removed.
+        * platform/chromium-win/editing/pasteboard/paste-pre-001-expected.txt: Removed.
+        * platform/chromium-win/editing/pasteboard/paste-pre-002-expected.txt: Removed.
+        * platform/qt-mac/editing/pasteboard/paste-code-in-pre-expected.txt: Removed.
+
+2011-08-10  Ryosuke Niwa  <rniwa@webkit.org>
+
         Skip a test added by r92769 on Windows port since drag and drop won't work well on Windows port.
 
         * platform/win/Skipped:
index 9509689..8d2c79c 100644 (file)
@@ -13,7 +13,7 @@ After:
 | <div>
 |   "This should not be underlined.<#selection-caret>"
 |   <span>
-|     style="text-decoration: underline; color: blue;"
+|     style="text-decoration: underline; color: blue; "
 |     <span>
 |       style="color:red;"
 |       "This should be underlined."
index 418930c..7aa656f 100644 (file)
@@ -4,7 +4,7 @@ This tests list creation in an editable context but across non-editable content.
 |     "Editabl<#selection-anchor>e paragraph containing a "
 |     <span>
 |       contenteditable="false"
-|       style="background-color: LightGray;"
+|       style="background-color: rgb(211, 211, 211); "
 |       "non-editable"
 |     " in the middle"
 |     <br>
index 64278b9..707615b 100644 (file)
@@ -20,6 +20,6 @@ Testing if DataTransferItems can be accessed outside an event handler...
 DataTransferItem accessed outside event handler!
 copy: items[0] value: Hello World!
 copy: items[1] value: <b>Hello World!
-paste: items[0] value: <span class="Apple-style-span" style="color: rgb(0, 0, 0); font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; font-size: medium; ">This file tests the basic functionality and properties of DataTransferItems. This test requires DRT.</span>
+paste: items[0] value: <span class="Apple-style-span" style="color: rgb(0, 0, 0); font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; font-size: medium; display: inline !important; float: none; ">This file tests the basic functionality and properties of DataTransferItems. This test requires DRT.</span>
 paste: items[1] value: This file tests the basic functionality and properties of DataTransferItems. This test requires DRT.
 
index 504fd9e..db08373 100644 (file)
@@ -1,5 +1,5 @@
 CONSOLE MESSAGE: line 21: text/plain: This test verifies that we can get text/html from the clipboard during an onpaste event. 
-CONSOLE MESSAGE: line 23: text/html: <span class="Apple-style-span" style="color: rgb(0, 0, 0);  font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; font-size: medium; ">This test verifies that we can get text/html from the clipboard during an onpaste event.<span class="Apple-converted-space"> </span></span>
+CONSOLE MESSAGE: line 23: text/html: <span class="Apple-style-span" style="color: rgb(0, 0, 0);  font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; font-size: medium; display: inline !important; float: none; ">This test verifies that we can get text/html from the clipboard during an onpaste event.<span class="Apple-converted-space"> </span></span>
 This test verifies that we can get text/html from the clipboard during an onpaste event. This test requires DRT.
 Paste content in this div.This test verifies that we can get text/html from the clipboard during an onpaste event. 
 PASS
index 78dc4c6..2a23d74 100644 (file)
@@ -51,11 +51,10 @@ Actual result:
 "
 | <ul>
 |   style="text-align:right;"
-|   <ul>
-|     style="text-align: right; "
-|     <li>
-|       "A"
+|   <li>
+|     "A"
 |   <div>
+|     style="text-align: -webkit-auto; "
 |     <ul>
 |       <li>
 |         <a>
index ac8097b..1494e22 100644 (file)
@@ -3,10 +3,7 @@ You should see markup in the editable region below. See <rdar://5027857>.
 | <pre>
 |   contenteditable="true"
 |   id="pre"
-|   <span>
-|     class="Apple-style-span"
-|     style="font-family: Times; white-space: normal; "
-|     <pre>
-|       contenteditable="true"
-|       id="pre"
-|       "<input type='button'>foo<br>bar<b>baz</b><#selection-caret>"
+|   <pre>
+|     contenteditable="true"
+|     id="pre"
+|     "<input type='button'>foo<br>bar<b>baz</b><#selection-caret>"
index 63971a4..3f1f2ff 100644 (file)
@@ -7,4 +7,4 @@ bar
 foo
 bar
 execCutCommand: <div id="test" class="editing"> <pre><br></pre> </div>
-execPasteCommand: <div id="test" class="editing"> <pre><span class="Apple-style-span" style="font-family: Times; white-space: normal; "><pre>foo bar</pre></span></pre> </div>
+execPasteCommand: <div id="test" class="editing"> <pre><pre>foo bar</pre></pre> </div>
index 583fa89..44c6a6b 100644 (file)
@@ -2,4 +2,4 @@ This is a layout test for rdar://problem/4370209 "Reproducible crash when pastin
 foo
 bar
 execCopyCommand: <div id="test" class="editing"> <pre>foo bar</pre> </div>
-execPasteCommand: <div id="test" class="editing"> <pre><span class="Apple-style-span" style="font-family: Times; white-space: normal; "><pre>foo bar</pre></span></pre> </div>
+execPasteCommand: <div id="test" class="editing"> <pre><pre>foo bar</pre></pre> </div>
index 52bb358..1138ddd 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"><span class="Apple-style-span" style="font-size: medium; "><div id="test" class="editing"><div><blockquote>foo</blockquote></div><div><br></div></div></span></div>
+execPasteCommand: <div id="test" class="editing"><div><blockquote>foo</blockquote></div></div> <div class="editing"><div id="test" class="editing"><div><blockquote>foo</blockquote></div><div><br></div></div></div>
index 6a01cc4..b1ad2cd 100644 (file)
@@ -9,9 +9,7 @@ Before copy:
 After paste:
 | <b>
 |   style="border: solid 5px blue;padding: 5px;"
-|   <span>
-|     class="Apple-style-span"
+|   <i>
 |     style="font-weight: normal; "
-|     <i>
-|       "hello<#selection-caret>"
+|     "hello<#selection-caret>"
 |   <br>
index 199c5f0..f9284b3 100644 (file)
@@ -1,4 +1,4 @@
 CONSOLE MESSAGE: line 21: text/plain: This test verifies that we can get text/html from the drag object during an ondrop event. 
-CONSOLE MESSAGE: line 23: text/html: <span class="Apple-style-span" style="color: rgb(0, 0, 0);  font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; font-size: medium; ">This test verifies that we can get text/html from the drag object during an ondrop event.<span class="Apple-converted-space"> </span></span>
+CONSOLE MESSAGE: line 23: text/html: <span class="Apple-style-span" style="color: rgb(0, 0, 0);  font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; font-size: medium; display: inline !important; float: none; ">This test verifies that we can get text/html from the drag object during an ondrop event.<span class="Apple-converted-space"> </span></span>
 This test verifies that we can get text/html from the drag object during an ondrop event. This test requires DRT.
 PASS
diff --git a/LayoutTests/platform/chromium-win/editing/pasteboard/paste-code-in-pre-expected.txt b/LayoutTests/platform/chromium-win/editing/pasteboard/paste-code-in-pre-expected.txt
deleted file mode 100644 (file)
index 22deea0..0000000
+++ /dev/null
@@ -1,12 +0,0 @@
-This tests a bug when copying HTML markup inside <pre> tags. When pasted, this content would appear as the rendered form of that markup.
-You should see markup in the editable region below. See <rdar://5027857>.
-| <pre>
-|   contenteditable="true"
-|   id="pre"
-|   <span>
-|     class="Apple-style-span"
-|     style="font-family: 'times new roman'; white-space: normal; "
-|     <pre>
-|       contenteditable="true"
-|       id="pre"
-|       "<input type='button'>foo<br>bar<b>baz</b><#selection-caret>"
diff --git a/LayoutTests/platform/chromium-win/editing/pasteboard/paste-pre-001-expected.txt b/LayoutTests/platform/chromium-win/editing/pasteboard/paste-pre-001-expected.txt
deleted file mode 100644 (file)
index 5cc52d2..0000000
+++ /dev/null
@@ -1,10 +0,0 @@
-Tests: 
-Copying and pasting content inside of a PRE tag. This test was created after fixing 3918056.
-Expected Results: 
-The PRE tag and the formatting of the text inside of the PRE should be maintained. Should see this content in the red box below:
-foo
-bar
-foo
-bar
-execCutCommand: <div id="test" class="editing"> <pre><br></pre> </div>
-execPasteCommand: <div id="test" class="editing"> <pre><span class="Apple-style-span" style="font-family: 'times new roman'; white-space: normal; "><pre>foo bar</pre></span></pre> </div>
diff --git a/LayoutTests/platform/chromium-win/editing/pasteboard/paste-pre-002-expected.txt b/LayoutTests/platform/chromium-win/editing/pasteboard/paste-pre-002-expected.txt
deleted file mode 100644 (file)
index 7540326..0000000
+++ /dev/null
@@ -1,5 +0,0 @@
-This is a layout test for rdar://problem/4370209 "Reproducible crash when pasting over whitespace:pre text". The text below is selected, copied, and pasted over itself. You'll see foo/nbar if successful.
-foo
-bar
-execCopyCommand: <div id="test" class="editing"> <pre>foo bar</pre> </div>
-execPasteCommand: <div id="test" class="editing"> <pre><span class="Apple-style-span" style="font-family: 'times new roman'; white-space: normal; "><pre>foo bar</pre></span></pre> </div>
diff --git a/LayoutTests/platform/qt-mac/editing/pasteboard/paste-code-in-pre-expected.txt b/LayoutTests/platform/qt-mac/editing/pasteboard/paste-code-in-pre-expected.txt
deleted file mode 100644 (file)
index 39e549f..0000000
+++ /dev/null
@@ -1,12 +0,0 @@
-This tests a bug when copying HTML markup inside <pre> tags. When pasted, this content would appear as the rendered form of that markup.
-You should see markup in the editable region below. See <rdar://5027857>.
-| <pre>
-|   contenteditable="true"
-|   id="pre"
-|   <span>
-|     class="Apple-style-span"
-|     style="font-family: 'Times New Roman'; white-space: normal; "
-|     <pre>
-|       contenteditable="true"
-|       id="pre"
-|       "<input type='button'>foo<br>bar<b>baz</b><#selection-caret>"
index 2804fec..3915b55 100644 (file)
@@ -1,3 +1,42 @@
+2011-08-10  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Copying can result in span around block elements on the clipboard
+        https://bugs.webkit.org/show_bug.cgi?id=34564
+
+        Reviewed by Tony Chang.
+
+        Completely overhauled the way WebKit preserves style in copy and paste. Instead of wrapping the entire
+        serialized contents by a Apple style span, WebKit now adds inline style to the top level elements,
+        wrap top level text nodes by a style span.
+
+        * editing/EditingStyle.cpp:
+        (WebCore::EditingStyle::collapseTextDecorationProperties): Remove text-decoration property when the value
+        of -webkit-text-decorations-in-effect is none.
+        (WebCore::EditingStyle::removeStyleFromRulesAndContext): Since display: inline and float: none are now
+        added on copy, remove these properties on paste.
+        (WebCore::EditingStyle::removePropertiesInElementDefaultStyle): Takes Element* instead of StyledElement*.
+        (WebCore::EditingStyle::forceInline): Added.
+        (WebCore::getPropertiesNotIn): Remove properties only when the base style has them.
+        * editing/EditingStyle.h:
+        * editing/markup.cpp:
+        (WebCore::StyledMarkupAccumulator::shouldApplyWrappingStyle): Added.
+        (WebCore::StyledMarkupAccumulator::StyledMarkupAccumulator): Takes highestNodeToBeSerialized.
+        (WebCore::StyledMarkupAccumulator::wrapWithStyleNode): Calls appendStyleNodeOpenTag and styleNodeCloseTag.
+        (WebCore::StyledMarkupAccumulator::appendStyleNodeOpenTag): Extracted from wrapWithStyleNode.
+        (WebCore::StyledMarkupAccumulator::styleNodeCloseTag): Ditto.
+        (WebCore::StyledMarkupAccumulator::appendText): Wraps text node with a style span if needed.
+        Set display: inline and float: none so that it won't be converted to a block on paste side.
+        (WebCore::StyledMarkupAccumulator::appendElement): Add wrapping style if appropriate; Remove any properties
+        that are overridden by default style and any style that may conflict with the computed style of node to
+        avoid modifying the appearance of the serialized nodes.
+        (WebCore::StyledMarkupAccumulator::serializeNodes): Compute wrapping style; copies of this style are
+        modified as needed when serializing top-level elements or text nodes. We call traverseNodesForSerialization
+        with NodeTraversalMode set to DoNotEmitString first to compute the highest node to be serialized. The second
+        call to the function actually serialize the nodes.
+        (WebCore::StyledMarkupAccumulator::traverseNodesForSerialization): Extracted from serializeNodes.
+        Outputs string only if NodeTraversalMode is set to EmitString.
+        (WebCore::createMarkup): No longer adds wrapping spans.
+
 2011-08-10  Adam Barth  <abarth@webkit.org>
 
         Add tests of optional arguments for Geolocation
index dfb78f5..ed7b669 100644 (file)
@@ -557,7 +557,10 @@ void EditingStyle::collapseTextDecorationProperties()
     if (!textDecorationsInEffect)
         return;
 
-    m_mutableStyle->setProperty(CSSPropertyTextDecoration, textDecorationsInEffect->cssText(), m_mutableStyle->getPropertyPriority(CSSPropertyTextDecoration));
+    if (textDecorationsInEffect->isValueList())
+        m_mutableStyle->setProperty(CSSPropertyTextDecoration, textDecorationsInEffect->cssText(), m_mutableStyle->getPropertyPriority(CSSPropertyTextDecoration));
+    else
+        m_mutableStyle->removeProperty(CSSPropertyTextDecoration);
     m_mutableStyle->removeProperty(CSSPropertyWebkitTextDecorationsInEffect);
 }
 
@@ -957,9 +960,18 @@ void EditingStyle::removeStyleFromRulesAndContext(StyledElement* element, Node*
         computedStyle->removePropertiesInElementDefaultStyle(element);
         m_mutableStyle = getPropertiesNotIn(m_mutableStyle.get(), computedStyle->m_mutableStyle.get());
     }
+
+    // 3. If this element is a span and has display: inline or float: none, remove them unless they are overriden by rules.
+    // These rules are added by serialization code to wrap text nodes.
+    if (isStyleSpan(element)) {
+        if (!styleFromMatchedRules->getPropertyCSSValue(CSSPropertyDisplay) && getIdentifierValue(m_mutableStyle.get(), CSSPropertyDisplay) == CSSValueInline)
+            m_mutableStyle->removeProperty(CSSPropertyDisplay);
+        if (!styleFromMatchedRules->getPropertyCSSValue(CSSPropertyFloat) && getIdentifierValue(m_mutableStyle.get(), CSSPropertyFloat) == CSSValueNone)
+            m_mutableStyle->removeProperty(CSSPropertyFloat);
+    }
 }
 
-void EditingStyle::removePropertiesInElementDefaultStyle(StyledElement* element)
+void EditingStyle::removePropertiesInElementDefaultStyle(Element* element)
 {
     if (!m_mutableStyle || !m_mutableStyle->length())
         return;
@@ -974,6 +986,14 @@ void EditingStyle::removePropertiesInElementDefaultStyle(StyledElement* element)
 
     m_mutableStyle->removePropertiesInSet(propertiesToRemove.data(), propertiesToRemove.size());
 }
+
+void EditingStyle::forceInline()
+{
+    if (!m_mutableStyle)
+        m_mutableStyle = CSSMutableStyleDeclaration::create();
+    const bool propertyIsImportant = true;
+    m_mutableStyle->setProperty(CSSPropertyDisplay, CSSValueInline, propertyIsImportant);
+}
     
 
 static void reconcileTextDecorationProperties(CSSMutableStyleDeclaration* style)
@@ -1178,13 +1198,13 @@ RefPtr<CSSMutableStyleDeclaration> getPropertiesNotIn(CSSStyleDeclaration* style
     diffTextDecorations(result.get(), CSSPropertyTextDecoration, baseTextDecorationsInEffect.get());
     diffTextDecorations(result.get(), CSSPropertyWebkitTextDecorationsInEffect, baseTextDecorationsInEffect.get());
 
-    if (fontWeightIsBold(result.get()) == fontWeightIsBold(baseStyle))
+    if (baseStyle->getPropertyCSSValue(CSSPropertyFontSize) && fontWeightIsBold(result.get()) == fontWeightIsBold(baseStyle))
         result->removeProperty(CSSPropertyFontWeight);
 
-    if (getRGBAFontColor(result.get()) == getRGBAFontColor(baseStyle))
+    if (baseStyle->getPropertyCSSValue(CSSPropertyColor) && getRGBAFontColor(result.get()) == getRGBAFontColor(baseStyle))
         result->removeProperty(CSSPropertyColor);
 
-    if (getTextAlignment(result.get()) == getTextAlignment(baseStyle))
+    if (baseStyle->getPropertyCSSValue(CSSPropertyTextAlign) && getTextAlignment(result.get()) == getTextAlignment(baseStyle))
         result->removeProperty(CSSPropertyTextAlign);
 
     return result;
index c47bf63..42c11be 100644 (file)
@@ -47,6 +47,7 @@ class CSSMutableStyleDeclaration;
 class CSSPrimitiveValue;
 class CSSValue;
 class Document;
+class Element;
 class HTMLElement;
 class Node;
 class Position;
@@ -126,6 +127,8 @@ public:
     void mergeStyleFromRules(StyledElement*);
     void mergeStyleFromRulesForSerialization(StyledElement*);
     void removeStyleFromRulesAndContext(StyledElement*, Node* context);
+    void removePropertiesInElementDefaultStyle(Element*);
+    void forceInline();
 
     float fontSizeDelta() const { return m_fontSizeDelta; }
     bool hasFontSizeDelta() const { return m_fontSizeDelta != NoFontDelta; }
@@ -144,7 +147,6 @@ private:
     void extractFontSizeDelta();
     bool conflictsWithInlineStyleOfElement(StyledElement*, EditingStyle* extractedStyle, Vector<CSSPropertyID>* conflictingProperties) const;
     void mergeStyle(CSSMutableStyleDeclaration*);
-    void removePropertiesInElementDefaultStyle(StyledElement*);
 
     RefPtr<CSSMutableStyleDeclaration> m_mutableStyle;
     bool m_shouldUseFixedDefaultFontSize;
index 5ee7fcc..680530e 100644 (file)
@@ -120,12 +120,7 @@ class StyledMarkupAccumulator : public MarkupAccumulator {
 public:
     enum RangeFullySelectsNode { DoesFullySelectNode, DoesNotFullySelectNode };
 
-    StyledMarkupAccumulator(Vector<Node*>* nodes, EAbsoluteURLs shouldResolveURLs, EAnnotateForInterchange shouldAnnotate, const Range* range)
-    : MarkupAccumulator(nodes, shouldResolveURLs, range)
-    , m_shouldAnnotate(shouldAnnotate)
-    {
-    }
-
+    StyledMarkupAccumulator(Vector<Node*>* nodes, EAbsoluteURLs, EAnnotateForInterchange, const Range*, Node* highestNodeToBeSerialized = 0);
     Node* serializeNodes(Node* startNode, Node* pastEnd);
     virtual void appendString(const String& s) { return MarkupAccumulator::appendString(s); }
     void wrapWithNode(Node*, bool convertBlocksToInlines = false, RangeFullySelectsNode = DoesFullySelectNode);
@@ -133,18 +128,38 @@ public:
     String takeResults();
 
 private:
+    void appendStyleNodeOpenTag(Vector<UChar>&, CSSStyleDeclaration*, Document*, bool isBlock = false);
+    const String styleNodeCloseTag(bool isBlock = false);
     virtual void appendText(Vector<UChar>& out, Text*);
     String renderedText(const Node*, const Range*);
     String stringValueForRange(const Node*, const Range*);
     void appendElement(Vector<UChar>& out, Element* element, bool addDisplayInline, RangeFullySelectsNode);
     void appendElement(Vector<UChar>& out, Element* element, Namespaces*) { appendElement(out, element, false, DoesFullySelectNode); }
 
+    enum NodeTraversalMode { EmitString, DoNotEmitString };
+    Node* traverseNodesForSerialization(Node* startNode, Node* pastEnd, NodeTraversalMode);
+
     bool shouldAnnotate() { return m_shouldAnnotate == AnnotateForInterchange; }
+    bool shouldApplyWrappingStyle(Node* node) const
+    {
+        return m_highestNodeToBeSerialized && m_highestNodeToBeSerialized->parentNode() == node->parentNode()
+            && m_wrappingStyle && m_wrappingStyle->style();
+    }
 
     Vector<String> m_reversedPrecedingMarkup;
     const EAnnotateForInterchange m_shouldAnnotate;
+    Node* m_highestNodeToBeSerialized;
+    RefPtr<EditingStyle> m_wrappingStyle;
 };
 
+inline StyledMarkupAccumulator::StyledMarkupAccumulator(Vector<Node*>* nodes, EAbsoluteURLs shouldResolveURLs, EAnnotateForInterchange shouldAnnotate,
+    const Range* range, Node* highestNodeToBeSerialized)
+    : MarkupAccumulator(nodes, shouldResolveURLs, range)
+    , m_shouldAnnotate(shouldAnnotate)
+    , m_highestNodeToBeSerialized(highestNodeToBeSerialized)
+{
+}
+
 void StyledMarkupAccumulator::wrapWithNode(Node* node, bool convertBlocksToInlines, RangeFullySelectsNode rangeFullySelectsNode)
 {
     Vector<UChar> markup;
@@ -160,20 +175,30 @@ void StyledMarkupAccumulator::wrapWithNode(Node* node, bool convertBlocksToInlin
 
 void StyledMarkupAccumulator::wrapWithStyleNode(CSSStyleDeclaration* style, Document* document, bool isBlock)
 {
+    Vector<UChar> openTag;
+    appendStyleNodeOpenTag(openTag, style, document, isBlock);
+    m_reversedPrecedingMarkup.append(String::adopt(openTag));
+    appendString(styleNodeCloseTag(isBlock));
+}
+
+void StyledMarkupAccumulator::appendStyleNodeOpenTag(Vector<UChar>& out, CSSStyleDeclaration* style, Document* document, bool isBlock)
+{
     // All text-decoration-related elements should have been treated as special ancestors
     // If we ever hit this ASSERT, we should export StyleChange in ApplyStyleCommand and use it here
     ASSERT(propertyMissingOrEqualToNone(style, CSSPropertyTextDecoration) && propertyMissingOrEqualToNone(style, CSSPropertyWebkitTextDecorationsInEffect));
     DEFINE_STATIC_LOCAL(const String, divStyle, ("<div style=\""));
-    DEFINE_STATIC_LOCAL(const String, divClose, ("</div>"));
     DEFINE_STATIC_LOCAL(const String, styleSpanOpen, ("<span class=\"" AppleStyleSpanClass "\" style=\""));
+    append(out, isBlock ? divStyle : styleSpanOpen);
+    appendAttributeValue(out, style->cssText(), document->isHTMLDocument());
+    out.append('\"');
+    out.append('>');
+}
+
+const String StyledMarkupAccumulator::styleNodeCloseTag(bool isBlock)
+{
+    DEFINE_STATIC_LOCAL(const String, divClose, ("</div>"));
     DEFINE_STATIC_LOCAL(const String, styleSpanClose, ("</span>"));
-    Vector<UChar> openTag;
-    append(openTag, isBlock ? divStyle : styleSpanOpen);
-    appendAttributeValue(openTag, style->cssText(), document->isHTMLDocument());
-    openTag.append('\"');
-    openTag.append('>');
-    m_reversedPrecedingMarkup.append(String::adopt(openTag));
-    appendString(isBlock ? divClose : styleSpanClose);
+    return isBlock ? divClose : styleSpanClose;
 }
 
 String StyledMarkupAccumulator::takeResults()
@@ -191,17 +216,34 @@ String StyledMarkupAccumulator::takeResults()
 }
 
 void StyledMarkupAccumulator::appendText(Vector<UChar>& out, Text* text)
-{
-    if (!shouldAnnotate() || (text->parentElement() && text->parentElement()->tagQName() == textareaTag)) {
+{    
+    const bool parentIsTextarea = text->parentElement() && text->parentElement()->tagQName() == textareaTag;
+    const bool wrappingSpan = shouldApplyWrappingStyle(text) && !parentIsTextarea;
+    if (wrappingSpan) {
+        RefPtr<EditingStyle> wrappingStyle = m_wrappingStyle->copy();
+        // FIXME: <rdar://problem/5371536> Style rules that match pasted content can change it's appearance
+        // Make sure spans are inline style in paste side e.g. span { display: block }.
+        wrappingStyle->forceInline();
+        // FIXME: Should this be included in forceInline?
+        wrappingStyle->style()->setProperty(CSSPropertyFloat, CSSValueNone);
+
+        Vector<UChar> openTag;
+        appendStyleNodeOpenTag(openTag, wrappingStyle->style(), text->document());
+        append(out, String::adopt(openTag));
+    }
+
+    if (!shouldAnnotate() || parentIsTextarea)
         MarkupAccumulator::appendText(out, text);
-        return;
+    else {
+        const bool useRenderedText = !enclosingNodeWithTag(firstPositionInNode(text), selectTag);
+        String content = useRenderedText ? renderedText(text, m_range) : stringValueForRange(text, m_range);
+        Vector<UChar> buffer;
+        appendCharactersReplacingEntities(buffer, content.characters(), content.length(), EntityMaskInPCDATA);
+        append(out, convertHTMLTextToInterchangeFormat(String::adopt(buffer), text));
     }
 
-    bool useRenderedText = !enclosingNodeWithTag(firstPositionInNode(text), selectTag);
-    String content = useRenderedText ? renderedText(text, m_range) : stringValueForRange(text, m_range);
-    Vector<UChar> buffer;
-    appendCharactersReplacingEntities(buffer, content.characters(), content.length(), EntityMaskInPCDATA);
-    append(out, convertHTMLTextToInterchangeFormat(String::adopt(buffer), text));
+    if (wrappingSpan)
+        append(out, styleNodeCloseTag());
 }
     
 String StyledMarkupAccumulator::renderedText(const Node* node, const Range* range)
@@ -240,35 +282,51 @@ String StyledMarkupAccumulator::stringValueForRange(const Node* node, const Rang
 
 void StyledMarkupAccumulator::appendElement(Vector<UChar>& out, Element* element, bool addDisplayInline, RangeFullySelectsNode rangeFullySelectsNode)
 {
-    bool documentIsHTML = element->document()->isHTMLDocument();
+    const bool documentIsHTML = element->document()->isHTMLDocument();
     appendOpenTag(out, element, 0);
 
     NamedNodeMap* attributes = element->attributes();
-    unsigned length = attributes->length();
+    const unsigned length = attributes->length();
+    const bool shouldAnnotateOrForceInline = element->isHTMLElement() && (shouldAnnotate() || addDisplayInline);
+    const bool shouldOverrideStyleAttr = shouldAnnotateOrForceInline || shouldApplyWrappingStyle(element);
     for (unsigned int i = 0; i < length; i++) {
         Attribute* attribute = attributes->attributeItem(i);
         // We'll handle the style attribute separately, below.
-        if (attribute->name() == styleAttr && element->isHTMLElement() && (shouldAnnotate() || addDisplayInline))
+        if (attribute->name() == styleAttr && shouldOverrideStyleAttr)
             continue;
         appendAttribute(out, element, *attribute, 0);
     }
 
-    if (element->isHTMLElement() && (shouldAnnotate() || addDisplayInline)) {
-        RefPtr<EditingStyle> style = EditingStyle::create(toHTMLElement(element)->getInlineStyleDecl());
-        if (shouldAnnotate())
-            style->mergeStyleFromRulesForSerialization(toHTMLElement(element));
-        if (addDisplayInline)
-            style->style()->setProperty(CSSPropertyDisplay, CSSValueInline, true);
+    if (shouldOverrideStyleAttr) {
+        RefPtr<EditingStyle> newInlineStyle;
+
+        if (shouldApplyWrappingStyle(element)) {
+            newInlineStyle = m_wrappingStyle->copy();
+            newInlineStyle->removePropertiesInElementDefaultStyle(element);
+            newInlineStyle->removeStyleConflictingWithStyleOfNode(element);
+        } else
+            newInlineStyle = EditingStyle::create();
+
+        if (element->isStyledElement() && static_cast<StyledElement*>(element)->inlineStyleDecl())
+            newInlineStyle->overrideWithStyle(static_cast<StyledElement*>(element)->inlineStyleDecl());
 
-        // If the node is not fully selected by the range, then we don't want to keep styles that affect its relationship to the nodes around it
-        // only the ones that affect it and the nodes within it.
-        if (rangeFullySelectsNode == DoesNotFullySelectNode)
-            style->style()->removeProperty(CSSPropertyFloat);
+        if (shouldAnnotateOrForceInline) {
+            if (shouldAnnotate())
+                newInlineStyle->mergeStyleFromRulesForSerialization(toHTMLElement(element));
 
-        if (!style->isEmpty()) {
+            if (addDisplayInline)
+                newInlineStyle->forceInline();
+
+            // If the node is not fully selected by the range, then we don't want to keep styles that affect its relationship to the nodes around it
+            // only the ones that affect it and the nodes within it.
+            if (rangeFullySelectsNode == DoesNotFullySelectNode && newInlineStyle->style())
+                newInlineStyle->style()->removeProperty(CSSPropertyFloat);
+        }
+
+        if (!newInlineStyle->isEmpty()) {
             DEFINE_STATIC_LOCAL(const String, stylePrefix, (" style=\""));
             append(out, stylePrefix);
-            appendAttributeValue(out, style->style()->cssText(), documentIsHTML);
+            appendAttributeValue(out, newInlineStyle->style()->cssText(), documentIsHTML);
             out.append('\"');
         }
     }
@@ -278,6 +336,30 @@ void StyledMarkupAccumulator::appendElement(Vector<UChar>& out, Element* element
 
 Node* StyledMarkupAccumulator::serializeNodes(Node* startNode, Node* pastEnd)
 {
+    if (!m_highestNodeToBeSerialized) {
+        Node* lastClosed = traverseNodesForSerialization(startNode, pastEnd, DoNotEmitString);
+        m_highestNodeToBeSerialized = lastClosed;
+    }
+
+    Node* parentOfHighestNode = m_highestNodeToBeSerialized ? m_highestNodeToBeSerialized->parentNode() : 0;
+    if (parentOfHighestNode) {
+        m_wrappingStyle = EditingStyle::create(parentOfHighestNode, EditingStyle::EditingInheritablePropertiesAndBackgroundColorInEffect);
+
+        // Styles that Mail blockquotes contribute should only be placed on the Mail blockquote,
+        // to help us differentiate those styles from ones that the user has applied.
+        // This helps us get the color of content pasted into blockquotes right.
+        m_wrappingStyle->removeStyleAddedByNode(enclosingNodeOfType(firstPositionInOrBeforeNode(parentOfHighestNode), isMailBlockquote, CanCrossEditingBoundary));
+
+        // Call collapseTextDecorationProperties first or otherwise it'll copy the value over from in-effect to text-decorations.
+        m_wrappingStyle->collapseTextDecorationProperties();
+    }
+
+    return traverseNodesForSerialization(startNode, pastEnd, EmitString);
+}
+
+Node* StyledMarkupAccumulator::traverseNodesForSerialization(Node* startNode, Node* pastEnd, NodeTraversalMode traversalMode)
+{
+    const bool shouldEmit = traversalMode == EmitString;
     Vector<Node*> ancestorsToClose;
     Node* next;
     Node* lastClosed = 0;
@@ -304,11 +386,13 @@ Node* StyledMarkupAccumulator::serializeNodes(Node* startNode, Node* pastEnd)
                 next = pastEnd;
         } else {
             // Add the node to the markup if we're not skipping the descendants
-            appendStartTag(n);
+            if (shouldEmit)
+                appendStartTag(n);
 
             // If node has no children, close the tag now.
             if (!n->childNodeCount()) {
-                appendEndTag(n);
+                if (shouldEmit)
+                    appendEndTag(n);
                 lastClosed = n;
             } else {
                 openedTag = true;
@@ -325,7 +409,8 @@ Node* StyledMarkupAccumulator::serializeNodes(Node* startNode, Node* pastEnd)
                 if (next != pastEnd && next->isDescendantOf(ancestor))
                     break;
                 // Not at the end of the range, close ancestors up to sibling of next node.
-                appendEndTag(ancestor);
+                if (shouldEmit)
+                    appendEndTag(ancestor);
                 lastClosed = ancestor;
                 ancestorsToClose.removeLast();
             }
@@ -340,7 +425,8 @@ Node* StyledMarkupAccumulator::serializeNodes(Node* startNode, Node* pastEnd)
                         continue;
                     // or b) ancestors that we never encountered during a pre-order traversal starting at startNode:
                     ASSERT(startNode->isDescendantOf(parent));
-                    wrapWithNode(parent);
+                    if (shouldEmit)
+                        wrapWithNode(parent);
                     lastClosed = parent;
                 }
             }
@@ -513,7 +599,13 @@ String createMarkup(const Range* range, Vector<Node*>* nodes, EAnnotateForInterc
 
     document->updateLayoutIgnorePendingStylesheets();
 
-    StyledMarkupAccumulator accumulator(nodes, shouldResolveURLs, shouldAnnotate, updatedRange.get());
+    Node* body = enclosingNodeWithTag(firstPositionInNode(commonAncestor), bodyTag);
+    Node* fullySelectedRoot = 0;
+    // FIXME: Do this for all fully selected blocks, not just the body.
+    if (body && areRangesEqual(VisibleSelection::selectionFromContentsOfNode(body).toNormalizedRange().get(), range))
+        fullySelectedRoot = body;
+    Node* specialCommonAncestor = highestAncestorToWrapMarkup(updatedRange.get(), fullySelectedRoot, shouldAnnotate);
+    StyledMarkupAccumulator accumulator(nodes, shouldResolveURLs, shouldAnnotate, updatedRange.get(), specialCommonAncestor);
     Node* pastEnd = updatedRange->pastLastNode();
 
     Node* startNode = updatedRange->firstNode();
@@ -539,14 +631,6 @@ String createMarkup(const Range* range, Vector<Node*>* nodes, EAnnotateForInterc
         ASSERT(!ec);
     }
 
-    Node* body = enclosingNodeWithTag(firstPositionInNode(commonAncestor), bodyTag);
-    Node* fullySelectedRoot = 0;
-    // FIXME: Do this for all fully selected blocks, not just the body.
-    if (body && areRangesEqual(VisibleSelection::selectionFromContentsOfNode(body).toNormalizedRange().get(), range))
-        fullySelectedRoot = body;
-
-    Node* specialCommonAncestor = highestAncestorToWrapMarkup(updatedRange.get(), fullySelectedRoot, shouldAnnotate);
-
     Node* lastClosed = accumulator.serializeNodes(startNode, pastEnd);
 
     if (specialCommonAncestor && lastClosed) {
@@ -586,20 +670,6 @@ String createMarkup(const Range* range, Vector<Node*>* nodes, EAnnotateForInterc
         }
     }
 
-    // Add a wrapper span with the styles that all of the nodes in the markup inherit.
-    ContainerNode* parentOfLastClosed = lastClosed ? lastClosed->parentNode() : 0;
-    if (parentOfLastClosed && parentOfLastClosed->renderer()) {
-        RefPtr<EditingStyle> style = EditingStyle::create(parentOfLastClosed, EditingStyle::EditingInheritablePropertiesAndBackgroundColorInEffect);
-
-        // Styles that Mail blockquotes contribute should only be placed on the Mail blockquote, to help
-        // us differentiate those styles from ones that the user has applied.  This helps us
-        // get the color of content pasted into blockquotes right.
-        style->removeStyleAddedByNode(enclosingNodeOfType(firstPositionInNode(parentOfLastClosed), isMailBlockquote, CanCrossEditingBoundary));
-
-        if (!style->isEmpty())
-            accumulator.wrapWithStyleNode(style->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()))
         accumulator.appendString(interchangeNewlineString);