Title: [87400] trunk
Revision
87400
Author
[email protected]
Date
2011-05-26 10:49:36 -0700 (Thu, 26 May 2011)

Log Message

2011-05-25  Ryosuke Niwa  <[email protected]>

        Reviewed by Enrica Casucci.

        WebKit duplicates styles from css rules on copy and paste
        https://bugs.webkit.org/show_bug.cgi?id=61466

        Fixed the bug by removing duplicate properties from inline style declarations in ReplaceSelectionCommand.
        Also moved the code to obtain style from rules from markup.cpp to EditingStyle.cpp to share code.

        Test: editing/pasteboard/style-from-rules.html

        * editing/EditingStyle.cpp:
        (WebCore::EditingStyle::EditingStyle): Added a null check.
        (WebCore::EditingStyle::extractFontSizeDelta): Ditto.
        (WebCore::styleFromMatchedRulesForElement): Moved from markup.cpp.
        (WebCore::EditingStyle::mergeStyleFromRules): Extracted from StyledMarkupAccumulator::appendElement.
        (WebCore::EditingStyle::mergeStyleFromRulesForSerialization): Ditto.
        (WebCore::EditingStyle::removeStyleFromRules): Added.
        * editing/EditingStyle.h:
        * editing/ReplaceSelectionCommand.cpp:
        (WebCore::ReplaceSelectionCommand::removeRedundantStylesAndKeepStyleSpanInline): Renamed from
        negateStyleRulesThatAffectAppearance; Calls removeStyleFromRules.
        * editing/markup.cpp:
        (WebCore::StyledMarkupAccumulator::appendElement): Calls mergeStyleFromRulesForSerialization.
        (WebCore::styleFromMatchedRulesAndInlineDecl): Calls mergeStyleFromRules; changed the return type
        from CSSMutableStyleDeclaration to EditingStyle.
        (WebCore::isElementPresentational): Calls styleFromMatchedRulesAndInlineDecl.
        (WebCore::shouldIncludeWrapperForFullySelectedRoot): Ditto.
        (WebCore::highestAncestorToWrapMarkup): Calls shouldIncludeWrapperForFullySelectedRoot.
        (WebCore::createMarkup): Calls styleFromMatchedRulesAndInlineDecl.
2011-05-25  Ryosuke Niwa  <[email protected]>

        Reviewed by Enrica Casucci.

        WebKit duplicates styles from css rules on copy and paste
        https://bugs.webkit.org/show_bug.cgi?id=61466

        Added a test to ensure Webkit does not leave redundant inline style declarations after paste.
        Also rebaselined several tests that progressed.

        * editing/pasteboard/4922709-expected.txt:
        * editing/pasteboard/5780697-2-expected.txt:
        * editing/pasteboard/paste-text-012-expected.txt:
        * editing/pasteboard/style-from-rules-expected.txt: Added.
        * editing/pasteboard/style-from-rules.html: Added.
        * platform/chromium-win/editing/pasteboard/5780697-2-expected.txt:

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (87399 => 87400)


--- trunk/LayoutTests/ChangeLog	2011-05-26 17:16:28 UTC (rev 87399)
+++ trunk/LayoutTests/ChangeLog	2011-05-26 17:49:36 UTC (rev 87400)
@@ -1,3 +1,20 @@
+2011-05-25  Ryosuke Niwa  <[email protected]>
+
+        Reviewed by Enrica Casucci.
+
+        WebKit duplicates styles from css rules on copy and paste
+        https://bugs.webkit.org/show_bug.cgi?id=61466
+
+        Added a test to ensure Webkit does not leave redundant inline style declarations after paste.
+        Also rebaselined several tests that progressed.
+
+        * editing/pasteboard/4922709-expected.txt:
+        * editing/pasteboard/5780697-2-expected.txt:
+        * editing/pasteboard/paste-text-012-expected.txt:
+        * editing/pasteboard/style-from-rules-expected.txt: Added.
+        * editing/pasteboard/style-from-rules.html: Added.
+        * platform/chromium-win/editing/pasteboard/5780697-2-expected.txt:
+
 2011-05-26  Michael Schneider  <[email protected]>
 
         Reviewed by Pavel Feldman.

Modified: trunk/LayoutTests/editing/pasteboard/4922709-expected.txt (87399 => 87400)


--- trunk/LayoutTests/editing/pasteboard/4922709-expected.txt	2011-05-26 17:16:28 UTC (rev 87399)
+++ trunk/LayoutTests/editing/pasteboard/4922709-expected.txt	2011-05-26 17:49:36 UTC (rev 87400)
@@ -1,7 +1,7 @@
 This tests copying/pasting less than a paragraph of quoted content. It should appear quoted.
 
 
-<blockquote id="blockquote" type="cite" style="border-left-width: 2px; border-left-style: solid; border-left-color: blue; margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; padding-left: 10px; color: blue; ">Hello World</blockquote>
+<blockquote id="blockquote" type="cite">Hello World</blockquote>
 <br>
 <div>On Tuesday, Dave wrote:</div>
 <div><br></div>

Modified: trunk/LayoutTests/editing/pasteboard/5780697-2-expected.txt (87399 => 87400)


--- trunk/LayoutTests/editing/pasteboard/5780697-2-expected.txt	2011-05-26 17:16:28 UTC (rev 87399)
+++ trunk/LayoutTests/editing/pasteboard/5780697-2-expected.txt	2011-05-26 17:49:36 UTC (rev 87400)
@@ -1,3 +1,3 @@
 This tests for a bug where copying content from a document in quirksmode and pasting it would produce overlapping text because of a height: 1%; overflow: visible; rule. To run manually, paste into a document not in quirksmode. The paragraphs should not overlap. When you inspect the source, the paragraphs should have pixel values for the height property.
 
-<p style="height: 54px; overflow-x: visible; overflow-y: visible; ">Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Etiam interdum lacus id lectus. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos hymenaeos. Quisque pulvinar, libero eu tincidunt adipiscing, quam arcu pharetra libero, sed aliquet leo eros vitae sapien.</p><p style="height: 54px; overflow-x: visible; overflow-y: visible; ">Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Etiam interdum lacus id lectus. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos hymenaeos. Quisque pulvinar, libero eu tincidunt adipiscing, quam arcu pharetra libero, sed aliquet leo eros vitae sapien.</p><p style="height: 54px; overflow-x: visible; overflow-y: visible; ">Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Etiam interdum lacus id lectus. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos hymenaeos. Quisque pulvinar, libero eu tincidunt adipiscing, quam arcu pharetra libero, sed aliquet leo eros vitae sapien.</p>
+<p style="height: 54px; ">Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Etiam interdum lacus id lectus. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos hymenaeos. Quisque pulvinar, libero eu tincidunt adipiscing, quam arcu pharetra libero, sed aliquet leo eros vitae sapien.</p><p style="height: 54px; ">Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Etiam interdum lacus id lectus. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos hymenaeos. Quisque pulvinar, libero eu tincidunt adipiscing, quam arcu pharetra libero, sed aliquet leo eros vitae sapien.</p><p style="height: 54px; ">Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Etiam interdum lacus id lectus. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos hymenaeos. Quisque pulvinar, libero eu tincidunt adipiscing, quam arcu pharetra libero, sed aliquet leo eros vitae sapien.</p>

Modified: trunk/LayoutTests/editing/pasteboard/paste-text-012-expected.txt (87399 => 87400)


--- trunk/LayoutTests/editing/pasteboard/paste-text-012-expected.txt	2011-05-26 17:16:28 UTC (rev 87399)
+++ trunk/LayoutTests/editing/pasteboard/paste-text-012-expected.txt	2011-05-26 17:49:36 UTC (rev 87400)
@@ -6,4 +6,4 @@
 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" 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>
+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>

Added: trunk/LayoutTests/editing/pasteboard/style-from-rules-expected.txt (0 => 87400)


--- trunk/LayoutTests/editing/pasteboard/style-from-rules-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/pasteboard/style-from-rules-expected.txt	2011-05-26 17:49:36 UTC (rev 87400)
@@ -0,0 +1,40 @@
+This test ensures WebKit does not duplicate styles from matched rules in inline style declarations when copying and pasting contents.
+To manually test, copy "hello world WebKit" and paste it in the box blow.
+Each element should have exact properties stated in its title attribute
+
+Original:
+| <blockquote>
+|   title="none"
+|   "hello"
+| "
+"
+| <p>
+|   title="none"
+|   <span>
+|     class="red"
+|     style="font-size: 1em; font-weight: bold;"
+|     title="font-size: 1em; font-weight: bold;"
+|     "world"
+|   "
+"
+|   <em>
+|     style="color: blue;"
+|     title="font-style: normal; font-weight: bold; color: blue;"
+|     "WebKit"
+
+Pasted:
+| <blockquote>
+|   title="none"
+|   "hello"
+| <p>
+|   title="none"
+|   <span>
+|     class="red"
+|     style="font-size: 1em; font-weight: bold; "
+|     title="font-size: 1em; font-weight: bold;"
+|     "world"
+|   " "
+|   <em>
+|     style="font-style: normal; font-weight: bold; color: blue; "
+|     title="font-style: normal; font-weight: bold; color: blue;"
+|     "WebKit<#selection-caret>"

Added: trunk/LayoutTests/editing/pasteboard/style-from-rules.html (0 => 87400)


--- trunk/LayoutTests/editing/pasteboard/style-from-rules.html	                        (rev 0)
+++ trunk/LayoutTests/editing/pasteboard/style-from-rules.html	2011-05-26 17:49:36 UTC (rev 87400)
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style type="text/css">
+div { border: solid 1px; padding: 5px; margin: 5px; width: 20ex; }
+.red { color: red; font-size: x-large; }
+blockquote { margin: 0px; padding: 0px 0px 0px 2ex; border-left: solid 2px #06f; }
+#copy em { font-style: normal; font-weight: bold; color: yellow; }
+#container p { padding: 0px; margin: 1em 0px 1em 0px; background-color: blue;}
+</style>
+</head>
+<body>
+<p>This test ensures WebKit does not duplicate styles from matched rules in inline style declarations when copying and pasting contents.
+To manually test, copy "hello world WebKit" and paste it in the box blow.
+Each element should have exact properties stated in its title attribute</p>
+<section id="container">
+<div id="copy" contenteditable><blockquote title="none">hello</blockquote>
+<p title="none"><span class="red" style="font-size: 1em; font-weight: bold;" title="font-size: 1em; font-weight: bold;">world</span>
+<em style="color: blue;" title="font-style: normal; font-weight: bold; color: blue;">WebKit</em></p></div>
+<div id="paste" _onpaste_="setTimeout(dump, 0)" contenteditable></div>
+</section>
+<script src=""
+<script>
+
+Markup.description(document.getElementsByTagName('p')[0].textContent);
+Markup.dump('copy', 'Original');
+
+document.getElementById('copy').focus();
+document.execCommand('SelectAll', false, null);
+
+function dump() { Markup.dump('paste', 'Pasted'); }
+
+if (window.layoutTestController) {
+    document.execCommand('Copy', false, null);
+    document.getElementById('paste').focus();
+    document.execCommand('Paste', false, null);
+    dump();
+    document.getElementById('container').style.display = 'none';
+}
+
+</script>
+</body>
+</html>

Modified: trunk/LayoutTests/platform/chromium-win/editing/pasteboard/5780697-2-expected.txt (87399 => 87400)


--- trunk/LayoutTests/platform/chromium-win/editing/pasteboard/5780697-2-expected.txt	2011-05-26 17:16:28 UTC (rev 87399)
+++ trunk/LayoutTests/platform/chromium-win/editing/pasteboard/5780697-2-expected.txt	2011-05-26 17:49:36 UTC (rev 87400)
@@ -1,3 +1,3 @@
 This tests for a bug where copying content from a document in quirksmode and pasting it would produce overlapping text because of a height: 1%; overflow: visible; rule. To run manually, paste into a document not in quirksmode. The paragraphs should not overlap. When you inspect the source, the paragraphs should have pixel values for the height property.
 
-<p style="height: 60px; overflow-x: visible; overflow-y: visible; ">Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Etiam interdum lacus id lectus. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos hymenaeos. Quisque pulvinar, libero eu tincidunt adipiscing, quam arcu pharetra libero, sed aliquet leo eros vitae sapien.</p><p style="height: 60px; overflow-x: visible; overflow-y: visible; ">Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Etiam interdum lacus id lectus. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos hymenaeos. Quisque pulvinar, libero eu tincidunt adipiscing, quam arcu pharetra libero, sed aliquet leo eros vitae sapien.</p><p style="height: 60px; overflow-x: visible; overflow-y: visible; ">Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Etiam interdum lacus id lectus. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos hymenaeos. Quisque pulvinar, libero eu tincidunt adipiscing, quam arcu pharetra libero, sed aliquet leo eros vitae sapien.</p>
+<p style="height: 60px; ">Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Etiam interdum lacus id lectus. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos hymenaeos. Quisque pulvinar, libero eu tincidunt adipiscing, quam arcu pharetra libero, sed aliquet leo eros vitae sapien.</p><p style="height: 60px; ">Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Etiam interdum lacus id lectus. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos hymenaeos. Quisque pulvinar, libero eu tincidunt adipiscing, quam arcu pharetra libero, sed aliquet leo eros vitae sapien.</p><p style="height: 60px; ">Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Etiam interdum lacus id lectus. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos hymenaeos. Quisque pulvinar, libero eu tincidunt adipiscing, quam arcu pharetra libero, sed aliquet leo eros vitae sapien.</p>

Modified: trunk/Source/WebCore/ChangeLog (87399 => 87400)


--- trunk/Source/WebCore/ChangeLog	2011-05-26 17:16:28 UTC (rev 87399)
+++ trunk/Source/WebCore/ChangeLog	2011-05-26 17:49:36 UTC (rev 87400)
@@ -1,3 +1,35 @@
+2011-05-25  Ryosuke Niwa  <[email protected]>
+
+        Reviewed by Enrica Casucci.
+
+        WebKit duplicates styles from css rules on copy and paste
+        https://bugs.webkit.org/show_bug.cgi?id=61466
+
+        Fixed the bug by removing duplicate properties from inline style declarations in ReplaceSelectionCommand.
+        Also moved the code to obtain style from rules from markup.cpp to EditingStyle.cpp to share code.
+
+        Test: editing/pasteboard/style-from-rules.html
+
+        * editing/EditingStyle.cpp:
+        (WebCore::EditingStyle::EditingStyle): Added a null check.
+        (WebCore::EditingStyle::extractFontSizeDelta): Ditto.
+        (WebCore::styleFromMatchedRulesForElement): Moved from markup.cpp.
+        (WebCore::EditingStyle::mergeStyleFromRules): Extracted from StyledMarkupAccumulator::appendElement.
+        (WebCore::EditingStyle::mergeStyleFromRulesForSerialization): Ditto.
+        (WebCore::EditingStyle::removeStyleFromRules): Added.
+        * editing/EditingStyle.h:
+        * editing/ReplaceSelectionCommand.cpp:
+        (WebCore::ReplaceSelectionCommand::removeRedundantStylesAndKeepStyleSpanInline): Renamed from
+        negateStyleRulesThatAffectAppearance; Calls removeStyleFromRules.
+        * editing/markup.cpp:
+        (WebCore::StyledMarkupAccumulator::appendElement): Calls mergeStyleFromRulesForSerialization.
+        (WebCore::styleFromMatchedRulesAndInlineDecl): Calls mergeStyleFromRules; changed the return type
+        from CSSMutableStyleDeclaration to EditingStyle.
+        (WebCore::isElementPresentational): Calls styleFromMatchedRulesAndInlineDecl.
+        (WebCore::shouldIncludeWrapperForFullySelectedRoot): Ditto.
+        (WebCore::highestAncestorToWrapMarkup): Calls shouldIncludeWrapperForFullySelectedRoot.
+        (WebCore::createMarkup): Calls styleFromMatchedRulesAndInlineDecl.
+
 2011-05-26  Michael Schneider  <[email protected]>
 
         Reviewed by Pavel Feldman.

Modified: trunk/Source/WebCore/editing/EditingStyle.cpp (87399 => 87400)


--- trunk/Source/WebCore/editing/EditingStyle.cpp	2011-05-26 17:16:28 UTC (rev 87399)
+++ trunk/Source/WebCore/editing/EditingStyle.cpp	2011-05-26 17:49:36 UTC (rev 87400)
@@ -31,6 +31,7 @@
 #include "CSSComputedStyleDeclaration.h"
 #include "CSSMutableStyleDeclaration.h"
 #include "CSSParser.h"
+#include "CSSStyleRule.h"
 #include "CSSStyleSelector.h"
 #include "CSSValueKeywords.h"
 #include "CSSValueList.h"
@@ -283,7 +284,7 @@
 }
 
 EditingStyle::EditingStyle(const CSSStyleDeclaration* style)
-    : m_mutableStyle(style->copy())
+    : m_mutableStyle(style ? style->copy() : 0)
     , m_shouldUseFixedDefaultFontSize(false)
     , m_fontSizeDelta(NoFontDelta)
 {
@@ -382,6 +383,9 @@
 
 void EditingStyle::extractFontSizeDelta()
 {
+    if (!m_mutableStyle)
+        return;
+
     if (m_mutableStyle->getPropertyCSSValue(CSSPropertyFontSize)) {
         // Explicit font size overrides any delta.
         m_mutableStyle->removeProperty(CSSPropertyWebkitFontSizeDelta);
@@ -805,6 +809,70 @@
     }
 }
 
+static PassRefPtr<CSSMutableStyleDeclaration> styleFromMatchedRulesForElement(Element* element)
+{
+    RefPtr<CSSMutableStyleDeclaration> style = CSSMutableStyleDeclaration::create();
+    RefPtr<CSSRuleList> matchedRules = element->document()->styleSelector()->styleRulesForElement(element,
+        CSSStyleSelector::AuthorCSSRules | CSSStyleSelector::CrossOriginCSSRules);
+    if (matchedRules) {
+        for (unsigned i = 0; i < matchedRules->length(); i++) {
+            if (matchedRules->item(i)->type() == CSSRule::STYLE_RULE) {
+                RefPtr<CSSMutableStyleDeclaration> s = static_cast<CSSStyleRule*>(matchedRules->item(i))->style();
+                style->merge(s.get(), true);
+            }
+        }
+    }
+    
+    return style.release();
+}
+
+void EditingStyle::mergeStyleFromRules(StyledElement* element)
+{
+    RefPtr<CSSMutableStyleDeclaration> styleFromMatchedRules = styleFromMatchedRulesForElement(element);
+    // Styles from the inline style declaration, held in the variable "style", take precedence 
+    // over those from matched rules.
+    if (m_mutableStyle)
+        styleFromMatchedRules->merge(m_mutableStyle.get());
+
+    clear();
+    m_mutableStyle = styleFromMatchedRules;
+}
+
+void EditingStyle::mergeStyleFromRulesForSerialization(StyledElement* element)
+{
+    mergeStyleFromRules(element);
+
+    // The property value, if it's a percentage, may not reflect the actual computed value.  
+    // For example: style="height: 1%; overflow: visible;" in quirksmode
+    // FIXME: There are others like this, see <rdar://problem/5195123> Slashdot copy/paste fidelity problem
+    RefPtr<CSSComputedStyleDeclaration> computedStyleForElement = computedStyle(element);
+    RefPtr<CSSMutableStyleDeclaration> fromComputedStyle = CSSMutableStyleDeclaration::create();
+    {
+        CSSMutableStyleDeclaration::const_iterator end = m_mutableStyle->end();
+        for (CSSMutableStyleDeclaration::const_iterator it = m_mutableStyle->begin(); it != end; ++it) {
+            const CSSProperty& property = *it;
+            CSSValue* value = property.value();
+            if (value->cssValueType() == CSSValue::CSS_PRIMITIVE_VALUE)
+                if (static_cast<CSSPrimitiveValue*>(value)->primitiveType() == CSSPrimitiveValue::CSS_PERCENTAGE)
+                    if (RefPtr<CSSValue> computedPropertyValue = computedStyleForElement->getPropertyCSSValue(property.id()))
+                        fromComputedStyle->addParsedProperty(CSSProperty(property.id(), computedPropertyValue));
+        }
+    }
+    m_mutableStyle->merge(fromComputedStyle.get());
+}
+
+void EditingStyle::removeStyleFromRules(StyledElement* element)
+{
+    if (!m_mutableStyle)
+        return;
+
+    RefPtr<CSSMutableStyleDeclaration> styleFromMatchedRules = styleFromMatchedRulesForElement(element);
+    if (!styleFromMatchedRules)
+        return;
+
+    m_mutableStyle = getPropertiesNotIn(m_mutableStyle.get(), styleFromMatchedRules.get());
+}
+
 static void reconcileTextDecorationProperties(CSSMutableStyleDeclaration* style)
 {    
     RefPtr<CSSValue> textDecorationsInEffect = style->getPropertyCSSValue(CSSPropertyWebkitTextDecorationsInEffect);

Modified: trunk/Source/WebCore/editing/EditingStyle.h (87399 => 87400)


--- trunk/Source/WebCore/editing/EditingStyle.h	2011-05-26 17:16:28 UTC (rev 87399)
+++ trunk/Source/WebCore/editing/EditingStyle.h	2011-05-26 17:49:36 UTC (rev 87400)
@@ -120,6 +120,9 @@
     void prepareToApplyAt(const Position&, ShouldPreserveWritingDirection = DoNotPreserveWritingDirection);
     void mergeTypingStyle(Document*);
     void mergeInlineStyleOfElement(StyledElement*);
+    void mergeStyleFromRules(StyledElement*);
+    void mergeStyleFromRulesForSerialization(StyledElement*);
+    void removeStyleFromRules(StyledElement*);
 
     float fontSizeDelta() const { return m_fontSizeDelta; }
     bool hasFontSizeDelta() const { return m_fontSizeDelta != NoFontDelta; }

Modified: trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp (87399 => 87400)


--- trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp	2011-05-26 17:16:28 UTC (rev 87399)
+++ trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp	2011-05-26 17:49:36 UTC (rev 87400)
@@ -476,7 +476,7 @@
 
 // Style rules that match just inserted elements could change their appearance, like
 // a div inserted into a document with div { display:inline; }.
-void ReplaceSelectionCommand::negateStyleRulesThatAffectAppearance()
+void ReplaceSelectionCommand::removeRedundantStylesAndKeepStyleSpanInline()
 {
     for (RefPtr<Node> node = m_firstNodeInserted.get(); node; node = node->traverseNextNode()) {
         // FIXME: <rdar://problem/5371536> Style rules that match pasted content can change it's appearance
@@ -492,7 +492,18 @@
                 e->getInlineStyleDecl()->setProperty(CSSPropertyDisplay, CSSValueInline);
             if (e->renderer() && e->renderer()->style()->floating() != FNONE)
                 e->getInlineStyleDecl()->setProperty(CSSPropertyFloat, CSSValueNone);
+        } else if (node->isStyledElement()) {
+            StyledElement* element = static_cast<StyledElement*>(node.get());
+            if (CSSMutableStyleDeclaration* inlineStyle = element->inlineStyleDecl()) {
+                RefPtr<EditingStyle> newInlineStyle = EditingStyle::create(inlineStyle);
+                newInlineStyle->removeStyleFromRules(element);
+                if (!newInlineStyle->style() || !newInlineStyle->style()->length())
+                    removeNodeAttribute(element, styleAttr);
+                else if (newInlineStyle->style()->length() < inlineStyle->length())
+                    setNodeAttribute(element, styleAttr, newInlineStyle->style()->cssText());                    
+            }
         }
+
         if (node == m_lastLeafInserted)
             break;
     }
@@ -995,7 +1006,7 @@
     
     removeUnrenderedTextNodesAtEnds();
     
-    negateStyleRulesThatAffectAppearance();
+    removeRedundantStylesAndKeepStyleSpanInline();
     
     if (!handledStyleSpans)
         handleStyleSpans();

Modified: trunk/Source/WebCore/editing/ReplaceSelectionCommand.h (87399 => 87400)


--- trunk/Source/WebCore/editing/ReplaceSelectionCommand.h	2011-05-26 17:16:28 UTC (rev 87399)
+++ trunk/Source/WebCore/editing/ReplaceSelectionCommand.h	2011-05-26 17:49:36 UTC (rev 87400)
@@ -77,7 +77,7 @@
     
     void removeUnrenderedTextNodesAtEnds();
     
-    void negateStyleRulesThatAffectAppearance();
+    void removeRedundantStylesAndKeepStyleSpanInline();
     void handleStyleSpans();
     void copyStyleToChildren(Node* parentNode, const CSSMutableStyleDeclaration* parentStyle);
     void handlePasteAsQuotationNode();

Modified: trunk/Source/WebCore/editing/markup.cpp (87399 => 87400)


--- trunk/Source/WebCore/editing/markup.cpp	2011-05-26 17:16:28 UTC (rev 87399)
+++ trunk/Source/WebCore/editing/markup.cpp	2011-05-26 17:49:36 UTC (rev 87400)
@@ -136,7 +136,6 @@
     virtual void appendText(Vector<UChar>& out, Text*);
     String renderedText(const Node*, const Range*);
     String stringValueForRange(const Node*, const Range*);
-    void removeExteriorStyles(CSSMutableStyleDeclaration*);
     void appendElement(Vector<UChar>& out, Element* element, bool addDisplayInline, RangeFullySelectsNode);
     void appendElement(Vector<UChar>& out, Element* element, Namespaces*) { appendElement(out, element, false, DoesFullySelectNode); }
 
@@ -239,23 +238,6 @@
     return str;
 }
 
-static PassRefPtr<CSSMutableStyleDeclaration> styleFromMatchedRulesForElement(Element* element)
-{
-    RefPtr<CSSMutableStyleDeclaration> style = CSSMutableStyleDeclaration::create();
-    RefPtr<CSSRuleList> matchedRules = element->document()->styleSelector()->styleRulesForElement(element,
-        CSSStyleSelector::AuthorCSSRules | CSSStyleSelector::CrossOriginCSSRules);
-    if (matchedRules) {
-        for (unsigned i = 0; i < matchedRules->length(); i++) {
-            if (matchedRules->item(i)->type() == CSSRule::STYLE_RULE) {
-                RefPtr<CSSMutableStyleDeclaration> s = static_cast<CSSStyleRule*>(matchedRules->item(i))->style();
-                style->merge(s.get(), true);
-            }
-        }
-    }
-
-    return style.release();
-}
-
 void StyledMarkupAccumulator::appendElement(Vector<UChar>& out, Element* element, bool addDisplayInline, RangeFullySelectsNode rangeFullySelectsNode)
 {
     bool documentIsHTML = element->document()->isHTMLDocument();
@@ -272,43 +254,21 @@
     }
 
     if (element->isHTMLElement() && (shouldAnnotate() || addDisplayInline)) {
-        RefPtr<CSSMutableStyleDeclaration> style = toHTMLElement(element)->getInlineStyleDecl()->copy();
-        if (shouldAnnotate()) {
-            RefPtr<CSSMutableStyleDeclaration> styleFromMatchedRules = styleFromMatchedRulesForElement(const_cast<Element*>(element));
-            // Styles from the inline style declaration, held in the variable "style", take precedence 
-            // over those from matched rules.
-            styleFromMatchedRules->merge(style.get());
-            style = styleFromMatchedRules;
-
-            RefPtr<CSSComputedStyleDeclaration> computedStyleForElement = computedStyle(element);
-            RefPtr<CSSMutableStyleDeclaration> fromComputedStyle = CSSMutableStyleDeclaration::create();
-
-            {
-                CSSMutableStyleDeclaration::const_iterator end = style->end();
-                for (CSSMutableStyleDeclaration::const_iterator it = style->begin(); it != end; ++it) {
-                    const CSSProperty& property = *it;
-                    CSSValue* value = property.value();
-                    // The property value, if it's a percentage, may not reflect the actual computed value.  
-                    // For example: style="height: 1%; overflow: visible;" in quirksmode
-                    // FIXME: There are others like this, see <rdar://problem/5195123> Slashdot copy/paste fidelity problem
-                    if (value->cssValueType() == CSSValue::CSS_PRIMITIVE_VALUE)
-                        if (static_cast<CSSPrimitiveValue*>(value)->primitiveType() == CSSPrimitiveValue::CSS_PERCENTAGE)
-                            if (RefPtr<CSSValue> computedPropertyValue = computedStyleForElement->getPropertyCSSValue(property.id()))
-                                fromComputedStyle->addParsedProperty(CSSProperty(property.id(), computedPropertyValue));
-                }
-            }
-            style->merge(fromComputedStyle.get());
-        }
+        RefPtr<EditingStyle> style = EditingStyle::create(toHTMLElement(element)->getInlineStyleDecl());
+        if (shouldAnnotate())
+            style->mergeStyleFromRulesForSerialization(toHTMLElement(element));
         if (addDisplayInline)
-            style->setProperty(CSSPropertyDisplay, CSSValueInline, true);
+            style->style()->setProperty(CSSPropertyDisplay, CSSValueInline, true);
+
         // 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)
-            removeExteriorStyles(style.get());
-        if (style->length() > 0) {
+            style->style()->removeProperty(CSSPropertyFloat);
+
+        if (!style->isEmpty()) {
             DEFINE_STATIC_LOCAL(const String, stylePrefix, (" style=\""));
             append(out, stylePrefix);
-            appendAttributeValue(out, style->cssText(), documentIsHTML);
+            appendAttributeValue(out, style->style()->cssText(), documentIsHTML);
             out.append('\"');
         }
     }
@@ -316,11 +276,6 @@
     appendCloseTag(out, element);
 }
 
-void StyledMarkupAccumulator::removeExteriorStyles(CSSMutableStyleDeclaration* style)
-{
-    style->removeProperty(CSSPropertyFloat);
-}
-
 Node* StyledMarkupAccumulator::serializeNodes(Node* startNode, Node* pastEnd)
 {
     Vector<Node*> ancestorsToClose;
@@ -447,7 +402,7 @@
     return isEndOfParagraph(v) && isStartOfParagraph(next) && !(upstreamNode->hasTagName(brTag) && upstreamNode == downstreamNode);
 }
 
-static PassRefPtr<CSSMutableStyleDeclaration> styleFromMatchedRulesAndInlineDecl(const Node* node)
+static PassRefPtr<EditingStyle> styleFromMatchedRulesAndInlineDecl(const Node* node)
 {
     if (!node->isHTMLElement())
         return 0;
@@ -455,9 +410,8 @@
     // FIXME: Having to const_cast here is ugly, but it is quite a bit of work to untangle
     // the non-const-ness of styleFromMatchedRulesForElement.
     HTMLElement* element = const_cast<HTMLElement*>(static_cast<const HTMLElement*>(node));
-    RefPtr<CSSMutableStyleDeclaration> style = styleFromMatchedRulesForElement(element);
-    RefPtr<CSSMutableStyleDeclaration> inlineStyleDecl = element->getInlineStyleDecl();
-    style->merge(inlineStyleDecl.get());
+    RefPtr<EditingStyle> style = EditingStyle::create(element->inlineStyleDecl());
+    style->mergeStyleFromRules(element);
     return style.release();
 }
 
@@ -466,18 +420,20 @@
     if (node->hasTagName(uTag) || node->hasTagName(sTag) || node->hasTagName(strikeTag)
         || node->hasTagName(iTag) || node->hasTagName(emTag) || node->hasTagName(bTag) || node->hasTagName(strongTag))
         return true;
-    RefPtr<CSSMutableStyleDeclaration> style = styleFromMatchedRulesAndInlineDecl(node);
-    if (!style)
-        return false;
-    return !propertyMissingOrEqualToNone(style.get(), CSSPropertyTextDecoration);
+    RefPtr<EditingStyle> style = styleFromMatchedRulesAndInlineDecl(node);
+    return style && style->style() && !propertyMissingOrEqualToNone(style->style(), CSSPropertyTextDecoration);
 }
 
-static bool shouldIncludeWrapperForFullySelectedRoot(Node* fullySelectedRoot, CSSMutableStyleDeclaration* style)
+static bool shouldIncludeWrapperForFullySelectedRoot(Node* fullySelectedRoot)
 {
     if (fullySelectedRoot->isElementNode() && static_cast<Element*>(fullySelectedRoot)->hasAttribute(backgroundAttr))
         return true;
     
-    return style->getPropertyCSSValue(CSSPropertyBackgroundImage) || style->getPropertyCSSValue(CSSPropertyBackgroundColor);
+    RefPtr<EditingStyle> style = styleFromMatchedRulesAndInlineDecl(fullySelectedRoot);
+    if (!style || !style->style())
+        return false;
+
+    return style->style()->getPropertyCSSValue(CSSPropertyBackgroundImage) || style->style()->getPropertyCSSValue(CSSPropertyBackgroundColor);
 }
 
 static Node* highestAncestorToWrapMarkup(const Range* range, Node* fullySelectedRoot, EAnnotateForInterchange shouldAnnotate)
@@ -515,11 +471,9 @@
     if (Node *enclosingAnchor = enclosingNodeWithTag(firstPositionInNode(specialCommonAncestor ? specialCommonAncestor : commonAncestor), aTag))
         specialCommonAncestor = enclosingAnchor;
 
-    if (shouldAnnotate == AnnotateForInterchange && fullySelectedRoot) {
-        RefPtr<CSSMutableStyleDeclaration> fullySelectedRootStyle = styleFromMatchedRulesAndInlineDecl(fullySelectedRoot);
-        if (shouldIncludeWrapperForFullySelectedRoot(fullySelectedRoot, fullySelectedRootStyle.get()))
-            specialCommonAncestor = fullySelectedRoot;
-    }
+    if (shouldAnnotate == AnnotateForInterchange && fullySelectedRoot && shouldIncludeWrapperForFullySelectedRoot(fullySelectedRoot))
+        specialCommonAncestor = fullySelectedRoot;
+
     return specialCommonAncestor;
 }
 
@@ -599,22 +553,23 @@
         // Also include all of the ancestors of lastClosed up to this special ancestor.
         for (ContainerNode* ancestor = lastClosed->parentNode(); ancestor; ancestor = ancestor->parentNode()) {
             if (ancestor == fullySelectedRoot && !convertBlocksToInlines) {
-                RefPtr<CSSMutableStyleDeclaration> fullySelectedRootStyle = styleFromMatchedRulesAndInlineDecl(fullySelectedRoot);
+                RefPtr<EditingStyle> fullySelectedRootStyle = styleFromMatchedRulesAndInlineDecl(fullySelectedRoot);
 
                 // Bring the background attribute over, but not as an attribute because a background attribute on a div
                 // appears to have no effect.
-                if (!fullySelectedRootStyle->getPropertyCSSValue(CSSPropertyBackgroundImage) && static_cast<Element*>(fullySelectedRoot)->hasAttribute(backgroundAttr))
-                    fullySelectedRootStyle->setProperty(CSSPropertyBackgroundImage, "url('" + static_cast<Element*>(fullySelectedRoot)->getAttribute(backgroundAttr) + "')");
-                
-                if (fullySelectedRootStyle->length()) {
+                if ((!fullySelectedRootStyle || !fullySelectedRootStyle->style() || !fullySelectedRootStyle->style()->getPropertyCSSValue(CSSPropertyBackgroundImage))
+                    && static_cast<Element*>(fullySelectedRoot)->hasAttribute(backgroundAttr))
+                    fullySelectedRootStyle->style()->setProperty(CSSPropertyBackgroundImage, "url('" + static_cast<Element*>(fullySelectedRoot)->getAttribute(backgroundAttr) + "')");
+
+                if (fullySelectedRootStyle->style()) {
                     // Reset the CSS properties to avoid an assertion error in addStyleMarkup().
                     // This assertion is caused at least when we select all text of a <body> element whose
                     // 'text-decoration' property is "inherit", and copy it.
-                    if (!propertyMissingOrEqualToNone(fullySelectedRootStyle.get(), CSSPropertyTextDecoration))
-                        fullySelectedRootStyle->setProperty(CSSPropertyTextDecoration, CSSValueNone);
-                    if (!propertyMissingOrEqualToNone(fullySelectedRootStyle.get(), CSSPropertyWebkitTextDecorationsInEffect))
-                        fullySelectedRootStyle->setProperty(CSSPropertyWebkitTextDecorationsInEffect, CSSValueNone);
-                    accumulator.wrapWithStyleNode(fullySelectedRootStyle.get(), document, true);
+                    if (!propertyMissingOrEqualToNone(fullySelectedRootStyle->style(), CSSPropertyTextDecoration))
+                        fullySelectedRootStyle->style()->setProperty(CSSPropertyTextDecoration, CSSValueNone);
+                    if (!propertyMissingOrEqualToNone(fullySelectedRootStyle->style(), CSSPropertyWebkitTextDecorationsInEffect))
+                        fullySelectedRootStyle->style()->setProperty(CSSPropertyWebkitTextDecorationsInEffect, CSSValueNone);
+                    accumulator.wrapWithStyleNode(fullySelectedRootStyle->style(), document, true);
                 }
             } else {
                 // Since this node and all the other ancestors are not in the selection we want to set RangeFullySelectsNode to DoesNotFullySelectNode
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to