REGRESSION (r92823): Background color not preserved when copying and pasting a table row
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Jan 2012 22:05:18 +0000 (22:05 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Jan 2012 22:05:18 +0000 (22:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=75330

Reviewed by Tony Chang.

Source/WebCore:

The bug was caused by the background color of the wrapping style overriding the background color
in a matched rule of a highest element to be serialized. Fixed the bug by removing the conflicting
background color prior to the merge.

Tests: editing/pasteboard/copy-element-with-conflicting-background-color-from-rule-expected.html
       editing/pasteboard/copy-element-with-conflicting-background-color-from-rule.html

* editing/EditingStyle.cpp:
(WebCore::editingStyleFromComputedStyle):
(WebCore::EditingStyle::removeStyleAddedByNode):
(WebCore::EditingStyle::removeStyleConflictingWithStyleOfNode):

LayoutTests:

Add a regression test.

* editing/pasteboard/copy-element-with-conflicting-background-color-from-rule-expected.html: Added.
* editing/pasteboard/copy-element-with-conflicting-background-color-from-rule.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/editing/pasteboard/copy-element-with-conflicting-background-color-from-rule-expected.html [new file with mode: 0644]
LayoutTests/editing/pasteboard/copy-element-with-conflicting-background-color-from-rule.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Element.cpp
Source/WebCore/editing/EditingStyle.cpp

index e5281674e6168d404c2e990da607b26495076cb2..d9ca7d9608901464bf16308215279bb5176d8af5 100644 (file)
@@ -1,3 +1,15 @@
+2012-01-03  Ryosuke Niwa  <rniwa@webkit.org>
+
+        REGRESSION (r92823): Background color not preserved when copying and pasting a table row
+        https://bugs.webkit.org/show_bug.cgi?id=75330
+
+        Reviewed by Tony Chang.
+
+        Add a regression test.
+
+        * editing/pasteboard/copy-element-with-conflicting-background-color-from-rule-expected.html: Added.
+        * editing/pasteboard/copy-element-with-conflicting-background-color-from-rule.html: Added.
+
 2012-01-04  Alpha Lam  <hclam@chromium.org>
 
         Not reviewed. Update Chromium port test expectations.
diff --git a/LayoutTests/editing/pasteboard/copy-element-with-conflicting-background-color-from-rule-expected.html b/LayoutTests/editing/pasteboard/copy-element-with-conflicting-background-color-from-rule-expected.html
new file mode 100644 (file)
index 0000000..b47affa
--- /dev/null
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This tests copying an element with a background color specified in a style rule that conflicts with the background color of a wrapping style.
+The pasted text should have lightgreen background color.</p>
+Paste here:
+<div contenteditable><div style="background-color: lightgreen;">Copy this line</div>
+<span id="target"></span><br>
+</div>
+<script>
+var target = document.getElementById('target');
+target.parentNode.focus();
+getSelection().collapse(target, 0);
+</script>
+</body>
+</html>
diff --git a/LayoutTests/editing/pasteboard/copy-element-with-conflicting-background-color-from-rule.html b/LayoutTests/editing/pasteboard/copy-element-with-conflicting-background-color-from-rule.html
new file mode 100644 (file)
index 0000000..d2133f0
--- /dev/null
@@ -0,0 +1,41 @@
+<!DOCTYPE html>
+<html>
+<body onpaste="finish()">
+<p>This tests copying an element with a background color specified in a style rule that conflicts with the background color of a wrapping style.
+The pasted text should have lightgreen background color.</p>
+<style>
+.add { background-color: lightgreen; }
+</style>
+<div style="background-color: red;">
+<div id="target" class='add'>
+Copy this line
+</div>
+<div>some other text</div>
+</div>
+
+Paste here:
+<div id="destination" contenteditable></div>
+
+<script>
+
+function finish() {
+    document.body.removeChild(document.querySelector('style'));
+    document.body.removeChild(target.parentNode);
+}
+
+document.body.focus();
+var target = document.getElementById('target');
+getSelection().setPosition(target, 0);
+getSelection().modify('move', 'forward', 'line');
+getSelection().modify('move', 'backward', 'lineboundary');
+getSelection().modify('extend', 'backward', 'line');
+
+if (window.layoutTestController) {
+    document.execCommand('Copy', false, null);
+    document.getElementById('destination').focus();
+    document.execCommand('Paste', false, null);
+}
+
+</script>
+</body>
+</html>
index 3302450a25e0b1c5c589051e128b44ed5b94c8ea..f8aa8b2816ab4a5c2c0ef33d750ea4e9b4112b73 100644 (file)
@@ -1,3 +1,22 @@
+2012-01-03  Ryosuke Niwa  <rniwa@webkit.org>
+
+        REGRESSION (r92823): Background color not preserved when copying and pasting a table row
+        https://bugs.webkit.org/show_bug.cgi?id=75330
+
+        Reviewed by Tony Chang.
+
+        The bug was caused by the background color of the wrapping style overriding the background color
+        in a matched rule of a highest element to be serialized. Fixed the bug by removing the conflicting
+        background color prior to the merge.
+
+        Tests: editing/pasteboard/copy-element-with-conflicting-background-color-from-rule-expected.html
+               editing/pasteboard/copy-element-with-conflicting-background-color-from-rule.html
+
+        * editing/EditingStyle.cpp:
+        (WebCore::editingStyleFromComputedStyle):
+        (WebCore::EditingStyle::removeStyleAddedByNode):
+        (WebCore::EditingStyle::removeStyleConflictingWithStyleOfNode):
+
 2012-01-03  Antti Koivisto  <antti@apple.com>
 
         Reviewed by Dave Hyatt.
index 9679b1229fd12a57a8d4f6e2f0268fc637d7bef1..016efb619ef8620cd5807cf43e70304238ca3072 100644 (file)
@@ -706,7 +706,10 @@ void Element::updateAfterAttributeChanged(Attribute* attr)
     
 void Element::recalcStyleIfNeededAfterAttributeChanged(Attribute* attr)
 {
-    if (document()->attached() && document()->styleSelector()->hasSelectorForAttribute(attr->name().localName()))
+    if (!needsStyleRecalc())
+        return;
+
+    if (document()->attached() && document()->styleSelectorIfExists() && document()->styleSelectorIfExists()->hasSelectorForAttribute(attr->name().localName()))
         setNeedsStyleRecalc();
 }
 
index aa3f13db52c30b5896dcfda88fb484f727ac7936..5da2d2bfb98b10bace1c876355b3983c740cfd8f 100644 (file)
@@ -80,9 +80,11 @@ static const int editingProperties[] = {
     CSSPropertyWebkitTextStrokeWidth,
 };
 
-static PassRefPtr<CSSMutableStyleDeclaration> copyEditingProperties(CSSStyleDeclaration* style, bool includeNonInheritableProperties = false)
+enum EditingPropertiesType { OnlyInheritableEditingProperties, AllEditingProperties };
+
+static PassRefPtr<CSSMutableStyleDeclaration> copyEditingProperties(CSSStyleDeclaration* style, EditingPropertiesType type = OnlyInheritableEditingProperties)
 {
-    if (includeNonInheritableProperties)
+    if (type == AllEditingProperties)
         return style->copyPropertiesInSet(editingProperties, WTF_ARRAY_LENGTH(editingProperties));
     return style->copyPropertiesInSet(editingProperties + 2, WTF_ARRAY_LENGTH(editingProperties) - 2);
 }
@@ -96,11 +98,11 @@ static inline bool isEditingProperty(int id)
     return false;
 }
 
-static PassRefPtr<CSSMutableStyleDeclaration> editingStyleFromComputedStyle(PassRefPtr<CSSComputedStyleDeclaration> style)
+static PassRefPtr<CSSMutableStyleDeclaration> editingStyleFromComputedStyle(PassRefPtr<CSSComputedStyleDeclaration> style, EditingPropertiesType type = OnlyInheritableEditingProperties)
 {
     if (!style)
         return CSSMutableStyleDeclaration::create();
-    return copyEditingProperties(style.get());
+    return copyEditingProperties(style.get(), type);
 }
 
 static RefPtr<CSSMutableStyleDeclaration> getPropertiesNotIn(CSSStyleDeclaration* styleWithRedundantProperties, CSSStyleDeclaration* baseStyle);
@@ -543,8 +545,8 @@ void EditingStyle::removeStyleAddedByNode(Node* node)
 {
     if (!node || !node->parentNode())
         return;
-    RefPtr<CSSMutableStyleDeclaration> parentStyle = editingStyleFromComputedStyle(computedStyle(node->parentNode()));
-    RefPtr<CSSMutableStyleDeclaration> nodeStyle = editingStyleFromComputedStyle(computedStyle(node));
+    RefPtr<CSSMutableStyleDeclaration> parentStyle = editingStyleFromComputedStyle(computedStyle(node->parentNode()), AllEditingProperties);
+    RefPtr<CSSMutableStyleDeclaration> nodeStyle = editingStyleFromComputedStyle(computedStyle(node), AllEditingProperties);
     parentStyle->diff(nodeStyle.get());
     nodeStyle->diff(m_mutableStyle.get());
 }
@@ -553,8 +555,9 @@ void EditingStyle::removeStyleConflictingWithStyleOfNode(Node* node)
 {
     if (!node || !node->parentNode() || !m_mutableStyle)
         return;
-    RefPtr<CSSMutableStyleDeclaration> parentStyle = editingStyleFromComputedStyle(computedStyle(node->parentNode()));
-    RefPtr<CSSMutableStyleDeclaration> nodeStyle = editingStyleFromComputedStyle(computedStyle(node));
+
+    RefPtr<CSSMutableStyleDeclaration> parentStyle = editingStyleFromComputedStyle(computedStyle(node->parentNode()), AllEditingProperties);
+    RefPtr<CSSMutableStyleDeclaration> nodeStyle = editingStyleFromComputedStyle(computedStyle(node), AllEditingProperties);
     parentStyle->diff(nodeStyle.get());
 
     CSSMutableStyleDeclaration::const_iterator end = nodeStyle->end();
@@ -903,8 +906,10 @@ void EditingStyle::mergeInlineStyleOfElement(StyledElement* element, CSSProperty
         mergeStyle(element->inlineStyleDecl(), mode);
         return;
     case OnlyEditingInheritableProperties:
+        mergeStyle(copyEditingProperties(element->inlineStyleDecl(), OnlyInheritableEditingProperties).get(), mode);
+        return;
     case EditingPropertiesInEffect:
-        mergeStyle(copyEditingProperties(element->inlineStyleDecl(), propertiesToInclude == EditingPropertiesInEffect).get(), mode);
+        mergeStyle(copyEditingProperties(element->inlineStyleDecl(), AllEditingProperties).get(), mode);
         return;
     }
 }