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 5f05b42..f6e5978 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.
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 b832b03..4683181 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.
index 403cc18..44dad70 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.
 }
 
+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();
index 5b55c2b..f71f05e 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);
+    // This does no change notifications since it's only called by createMarkup.
+    void addParsedProperty(const CSSProperty&);
  
     PassRefPtr<CSSMutableStyleDeclaration> copyBlockProperties() const;
     void removeBlockProperties();
index 57f008b..abea55a 100644 (file)
 
 #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 "CSSValue.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;
+                    
+                    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);