WebCore:
authorjustin.garcia@apple.com <justin.garcia@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Mar 2008 03:15:17 +0000 (03:15 +0000)
committerjustin.garcia@apple.com <justin.garcia@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Mar 2008 03:15:17 +0000 (03:15 +0000)
2008-03-19  Justin Garcia  <justin.garcia@apple.com>

        Reviewed by Oliver.

        <rdar://problem/5780697> Copying content with CSS property values that are percentages can cause fidelity issues

        Elements with height: x%; overflow: visible; overlap what's below them when they are copied from
        a document in quirksmode and pasted into to one in standards mode.  This fix uses the computed
        the value for a property if its value is a percentage.

        * css/CSSMutableStyleDeclaration.cpp:
        (WebCore::CSSMutableStyleDeclaration::addParsedProperty): Added so that we don't have to use
        setProperty from appendStartMarkup.  We already have a parsed property value, so we shouldn't
        use setProperty, since it takes in a String.  If we did, we would have to call CSSValue::cssText()
        for a String only to re-parse it in setProperty.  This wasn't extremely important now, but it will
        be as we compute more properties to fix the rest of the copy/paste fidelity bugs.
        * css/CSSMutableStyleDeclaration.h:
        * editing/markup.cpp:
        (WebCore::appendStartMarkup): Compute values for properties that have percentage values.  We could
        perhaps narrow this special case to only include properties that are effected by quirksmode.

LayoutTests:

2008-03-19  Justin Garcia  <justin.garcia@apple.com>

        Reviewed by Oliver.

        <rdar://problem/5780697> Copying content with CSS property values that are percentages can cause fidelity issues

        * editing/pasteboard/5780697-2-expected.txt: Added.
        * editing/pasteboard/5780697-2.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/editing/pasteboard/5780697-2-expected.txt [new file with mode: 0644]
LayoutTests/editing/pasteboard/5780697-2.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/css/CSSMutableStyleDeclaration.cpp
WebCore/css/CSSMutableStyleDeclaration.h
WebCore/editing/markup.cpp

index 5f05b42a463f266a4605cc0c1a54bfab3895e406..f6e59783cca0550a742d5440bff3862c6cef3990 100644 (file)
@@ -1,3 +1,12 @@
+2008-03-19  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Oliver.
+        
+        <rdar://problem/5780697> Copying content with CSS property values that are percentages can cause fidelity issues
+
+        * editing/pasteboard/5780697-2-expected.txt: Added.
+        * editing/pasteboard/5780697-2.html: Added.
+
 2008-03-19  Oliver Hunt  <oliver@apple.com>
 
         Reviewed by Antti.
 2008-03-19  Oliver Hunt  <oliver@apple.com>
 
         Reviewed by Antti.
diff --git a/LayoutTests/editing/pasteboard/5780697-2-expected.txt b/LayoutTests/editing/pasteboard/5780697-2-expected.txt
new file mode 100644 (file)
index 0000000..a10a620
--- /dev/null
@@ -0,0 +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="overflow-x: visible; overflow-y: visible; 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="overflow-x: visible; overflow-y: visible; 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="overflow-x: visible; overflow-y: visible; 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>
diff --git a/LayoutTests/editing/pasteboard/5780697-2.html b/LayoutTests/editing/pasteboard/5780697-2.html
new file mode 100644 (file)
index 0000000..fd59366
--- /dev/null
@@ -0,0 +1,36 @@
+<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
+<html>
+<head>
+<style>
+p {
+    height: 1%;
+    overflow: visible;
+}
+</style>
+</head>
+<body>
+<div id="description">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.</div>
+<div id="copy" contenteditable="true">
+<p>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>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>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>
+</div>
+
+<div id="paste" contenteditable="true"></div>
+
+<script>
+copy = document.getElementById("copy");
+copy.focus();
+document.execCommand("selectall");
+document.execCommand("copy");
+if (window.layoutTestController) {
+    window.layoutTestController.dumpAsText();
+    paste = document.getElementById("paste");
+    description = document.getElementById("description");
+    document.getElementById("paste").focus();
+    document.execCommand("paste");
+    document.body.innerText = description.innerText + "\n\n" + paste.innerHTML;
+}
+</script>
+</body>
+</html>
index b832b034a20ca949d2b0a038830008693e121042..468318141d32585a07cdb414aeff5b1b2bbd2bb6 100644 (file)
@@ -1,3 +1,24 @@
+2008-03-19  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Oliver.
+        
+        <rdar://problem/5780697> Copying content with CSS property values that are percentages can cause fidelity issues
+         
+        Elements with height: x%; overflow: visible; overlap what's below them when they are copied from
+        a document in quirksmode and pasted into to one in standards mode.  This fix uses the computed 
+        the value for a property if its value is a percentage.
+        
+        * css/CSSMutableStyleDeclaration.cpp:
+        (WebCore::CSSMutableStyleDeclaration::addParsedProperty): Added so that we don't have to use
+        setProperty from appendStartMarkup.  We already have a parsed property value, so we shouldn't
+        use setProperty, since it takes in a String.  If we did, we would have to call CSSValue::cssText()
+        for a String only to re-parse it in setProperty.  This wasn't extremely important now, but it will 
+        be as we compute more properties to fix the rest of the copy/paste fidelity bugs.
+        * css/CSSMutableStyleDeclaration.h:
+        * editing/markup.cpp:
+        (WebCore::appendStartMarkup): Compute values for properties that have percentage values.  We could
+        perhaps narrow this special case to only include properties that are effected by quirksmode.
+
 2008-03-19  Sam Weinig  <sam@webkit.org>
 
         Reviewed by Anders Carlsson.
 2008-03-19  Sam Weinig  <sam@webkit.org>
 
         Reviewed by Anders Carlsson.
index 403cc187df0dc5ebd88caa3bee902747cb054105..44dad706586bbc727d3576975d3cc251803fb4c4 100644 (file)
@@ -578,6 +578,12 @@ void CSSMutableStyleDeclaration::addParsedProperties(const CSSProperty * const *
     // a notifyChanged argument to this function to follow the model of other functions in this class.
 }
 
     // a notifyChanged argument to this function to follow the model of other functions in this class.
 }
 
+void CSSMutableStyleDeclaration::addParsedProperty(const CSSProperty& property)
+{
+    removeProperty(property.id(), false);
+    m_values.append(property);
+}
+
 void CSSMutableStyleDeclaration::setLengthProperty(int propertyId, const String& value, bool important, bool /*multiLength*/)
 {
     bool parseMode = useStrictParsing();
 void CSSMutableStyleDeclaration::setLengthProperty(int propertyId, const String& value, bool important, bool /*multiLength*/)
 {
     bool parseMode = useStrictParsing();
index 5b55c2bf670de37cae74999cb4f363f128512910..f71f05ef239d576712525e00547e26f55ce3422a 100644 (file)
@@ -91,6 +91,8 @@ public:
     // Besides adding the properties, this also removes any existing properties with these IDs.
     // It does no notification since it's called by the parser.
     void addParsedProperties(const CSSProperty* const *, int numProperties);
     // Besides adding the properties, this also removes any existing properties with these IDs.
     // It does no notification since it's called by the parser.
     void addParsedProperties(const CSSProperty* const *, int numProperties);
+    // This does no change notifications since it's only called by createMarkup.
+    void addParsedProperty(const CSSProperty&);
  
     PassRefPtr<CSSMutableStyleDeclaration> copyBlockProperties() const;
     void removeBlockProperties();
  
     PassRefPtr<CSSMutableStyleDeclaration> copyBlockProperties() const;
     void removeBlockProperties();
index 57f008b87b9498e16b261c29a248d3bf5dbe16f0..abea55a21640a7b21f9ffb28bd422748429b4d62 100644 (file)
 
 #include "CDATASection.h"
 #include "CSSComputedStyleDeclaration.h"
 
 #include "CDATASection.h"
 #include "CSSComputedStyleDeclaration.h"
+#include "CSSPrimitiveValue.h"
+#include "CSSProperty.h"
 #include "CSSPropertyNames.h"
 #include "CSSRule.h"
 #include "CSSRuleList.h"
 #include "CSSStyleRule.h"
 #include "CSSStyleSelector.h"
 #include "CSSPropertyNames.h"
 #include "CSSRule.h"
 #include "CSSRuleList.h"
 #include "CSSStyleRule.h"
 #include "CSSStyleSelector.h"
+#include "CSSValue.h"
 #include "CSSValueKeywords.h"
 #include "Comment.h"
 #include "DeleteButtonController.h"
 #include "CSSValueKeywords.h"
 #include "Comment.h"
 #include "DeleteButtonController.h"
@@ -434,6 +437,24 @@ static void appendStartMarkup(Vector<UChar>& result, const Node *node, const Ran
                     // over those from matched rules.
                     styleFromMatchedRules->merge(style.get());
                     style = styleFromMatchedRules;
                     // over those from matched rules.
                     styleFromMatchedRules->merge(style.get());
                     style = styleFromMatchedRules;
+                    
+                    RefPtr<CSSComputedStyleDeclaration> computedStyleForElement = computedStyle(element);
+                    RefPtr<CSSMutableStyleDeclaration> fromComputedStyle = new CSSMutableStyleDeclaration();
+                    
+                    DeprecatedValueListConstIterator<CSSProperty> end;
+                    for (DeprecatedValueListConstIterator<CSSProperty> it = style->valuesIterator(); 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());
                 }
                 if (convert)
                     style->setProperty(CSSPropertyDisplay, CSSValueInline, true);
                 }
                 if (convert)
                     style->setProperty(CSSPropertyDisplay, CSSValueInline, true);