Copy and paste can strip !important CSS rules due to a bug in mergeStyleFromRules
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Apr 2013 04:51:28 +0000 (04:51 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Apr 2013 04:51:28 +0000 (04:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=115217

Reviewed by Darin Adler.

Source/WebCore:

The bug was caused by mergeStyleFromRules overriding "important" style rules with "unimportant" inline styles.
Fixed the bug by using addParsedProperty, which respects !important, in MutableStylePropertySet's
mergeAndOverrideOnConflict, which was only used in editing code. Now that we've fixed this function, we can use
it in ViewportStyleResolver::addViewportRule as well.

Test: editing/pasteboard/copy-paste-with-important-rules.html

* css/StylePropertySet.cpp:
(WebCore::MutableStylePropertySet::mergeAndOverrideOnConflict): Fixed to respect !important.
* css/ViewportStyleResolver.cpp:
(WebCore::ViewportStyleResolver::addViewportRule): Use mergeAndOverrideOnConflict now that the code is identical.

LayoutTests:

Added a regression test.

* editing/pasteboard/copy-paste-with-important-rules-expected.txt: Added.
* editing/pasteboard/copy-paste-with-important-rules.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/editing/pasteboard/copy-paste-with-important-rules-expected.txt [new file with mode: 0644]
LayoutTests/editing/pasteboard/copy-paste-with-important-rules.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/StylePropertySet.cpp
Source/WebCore/css/ViewportStyleResolver.cpp

index d67571f4603a06d74acd467f90a980989eb08977..58feef94bd5a5d50f55b05e2244d80e343766fc0 100644 (file)
@@ -1,3 +1,15 @@
+2013-04-25  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Copy and paste can strip !important CSS rules due to a bug in mergeStyleFromRules
+        https://bugs.webkit.org/show_bug.cgi?id=115217
+
+        Reviewed by Darin Adler.
+
+        Added a regression test.
+
+        * editing/pasteboard/copy-paste-with-important-rules-expected.txt: Added.
+        * editing/pasteboard/copy-paste-with-important-rules.html: Added.
+
 2013-04-24  Oliver Hunt  <oliver@apple.com>
 
         Add support for Math.imul
diff --git a/LayoutTests/editing/pasteboard/copy-paste-with-important-rules-expected.txt b/LayoutTests/editing/pasteboard/copy-paste-with-important-rules-expected.txt
new file mode 100644 (file)
index 0000000..5101aa7
--- /dev/null
@@ -0,0 +1,38 @@
+This test ensures copying and paste respects !important in style rules.
+To test manually, copy and paste the content in the first box to the second box. All text should remain in blue and should remain unboldened.
+
+Original content:
+| "
+"
+| <p>
+|   "<#selection-anchor>hello "
+|   <span>
+|     style="color: red; font-weight: bold;"
+|     "world"
+| "
+"
+| <p>
+|   <span>
+|     class="Apple-style-span"
+|     style="color: red; font-weight: bold;"
+|     "WebKit<#selection-focus>"
+| "
+"
+
+Pasted content:
+| "
+"
+| <p>
+|   "hello "
+|   <span>
+|     style="color: red; font-weight: bold;"
+|     "world"
+| "
+"
+| <p>
+|   <span>
+|     class="Apple-style-span"
+|     style="color: red; font-weight: bold;"
+|     "WebKit"
+| "
+"
diff --git a/LayoutTests/editing/pasteboard/copy-paste-with-important-rules.html b/LayoutTests/editing/pasteboard/copy-paste-with-important-rules.html
new file mode 100644 (file)
index 0000000..3c4d93d
--- /dev/null
@@ -0,0 +1,41 @@
+<!DOCTYPE html>
+<html>
+<body>
+<style>
+#source * {
+    color: blue !important;
+    font-weight: normal !important;
+}
+body > div {
+       border: 2px solid black;
+       margin: 10px;
+}
+</style>
+<p id="description">This test ensures copying and paste respects !important in style rules.
+To test manually, copy and paste the content in the first box to the second box. All text should remain in blue and should remain unboldened.</p>
+<div id="source" contenteditable>
+<p>hello <span style="color: red; font-weight: bold;">world</span></p>
+<p><span class="Apple-style-span" style="color: red; font-weight: bold;">WebKit</span></p>
+</div>
+<div  id="destination" contenteditable></div>
+<script src="../../resources/dump-as-markup.js"></script>
+<script>
+
+Markup.description(document.getElementById('description').textContent);
+
+var source = document.getElementById('source');
+source.focus();
+getSelection().selectAllChildren(source);
+
+if (document.queryCommandEnabled('paste', false, null) && document.queryCommandEnabled('paste', false, null)) {
+    Markup.dump('source', 'Original content');
+    document.execCommand('copy');
+    getSelection().collapse(document.getElementById('destination'));
+    document.execCommand('paste');
+    Markup.dump('source', 'Pasted content');
+} else
+    Markup.noAutoDump();
+
+</script>
+</body>
+</html>
index 7a7b4e4872426a20deacddbf0109f7c7b8f931f1..9cd19700638ac47c4809f64aeae4bf3624dbeada 100644 (file)
@@ -1,3 +1,22 @@
+2013-04-25  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Copy and paste can strip !important CSS rules due to a bug in mergeStyleFromRules
+        https://bugs.webkit.org/show_bug.cgi?id=115217
+
+        Reviewed by Darin Adler.
+
+        The bug was caused by mergeStyleFromRules overriding "important" style rules with "unimportant" inline styles.
+        Fixed the bug by using addParsedProperty, which respects !important, in MutableStylePropertySet's
+        mergeAndOverrideOnConflict, which was only used in editing code. Now that we've fixed this function, we can use
+        it in ViewportStyleResolver::addViewportRule as well.
+
+        Test: editing/pasteboard/copy-paste-with-important-rules.html
+
+        * css/StylePropertySet.cpp:
+        (WebCore::MutableStylePropertySet::mergeAndOverrideOnConflict): Fixed to respect !important.
+        * css/ViewportStyleResolver.cpp:
+        (WebCore::ViewportStyleResolver::addViewportRule): Use mergeAndOverrideOnConflict now that the code is identical.
+
 2013-04-25  Andreas Kling  <akling@apple.com>
 
         StylePropertySet::getPropertyShorthand() should return a String.
index 16079aaf598c9e085e88c0821bcd4a60936006b2..6c9271946644853e04665ee00b4389cfd55ca243 100644 (file)
@@ -1030,14 +1030,8 @@ String StylePropertySet::asText() const
 void MutableStylePropertySet::mergeAndOverrideOnConflict(const StylePropertySet* other)
 {
     unsigned size = other->propertyCount();
-    for (unsigned n = 0; n < size; ++n) {
-        PropertyReference toMerge = other->propertyAt(n);
-        CSSProperty* old = findCSSPropertyWithID(toMerge.id());
-        if (old)
-            setProperty(toMerge.toCSSProperty(), old);
-        else
-            appendPrefixingVariantProperty(toMerge.toCSSProperty());
-    }
+    for (unsigned i = 0; i < size; ++i)
+        addParsedProperty(other->propertyAt(i).toCSSProperty());
 }
 
 void StylePropertySet::addSubresourceStyleURLs(ListHashSet<KURL>& urls, StyleSheetContents* contextStyleSheet) const
index f5bf480857011ad97db2cdd2a016116078ffbc2c..4085ef05714ad5a0da391d4e639e25e5fd3e1a98 100644 (file)
@@ -65,10 +65,7 @@ void ViewportStyleResolver::addViewportRule(StyleRuleViewport* viewportRule)
         return;
     }
 
-    // We cannot use mergeAndOverrideOnConflict() here because it doesn't
-    // respect the !important declaration (but addParsedProperty() does).
-    for (unsigned i = 0; i < propertyCount; ++i)
-        m_propertySet->addParsedProperty(propertySet->propertyAt(i).toCSSProperty());
+    m_propertySet->mergeAndOverrideOnConflict(propertySet);
 }
 
 void ViewportStyleResolver::clearDocument()