Reviewed by Darin.
authorrwlbuis <rwlbuis@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Dec 2006 08:02:35 +0000 (08:02 +0000)
committerrwlbuis <rwlbuis@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Dec 2006 08:02:35 +0000 (08:02 +0000)
        http://bugs.webkit.org/show_bug.cgi?id=6074
        WebKit+SVG and FireFox disagree on invalid "transform" handling

        Test for parsing failure on transform attribute and clear the transform
        list upon failure to match FF behaviour.

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

15 files changed:
LayoutTests/ChangeLog
LayoutTests/svg/custom/invalid-transforms-expected.checksum [new file with mode: 0644]
LayoutTests/svg/custom/invalid-transforms-expected.png [new file with mode: 0644]
LayoutTests/svg/custom/invalid-transforms-expected.txt [new file with mode: 0644]
LayoutTests/svg/custom/invalid-transforms.svg [new file with mode: 0644]
LayoutTests/svg/custom/transform-ignore-after-invalid-expected.checksum
LayoutTests/svg/custom/transform-ignore-after-invalid-expected.png
LayoutTests/svg/custom/transform-ignore-after-invalid-expected.txt
WebCore/ChangeLog
WebCore/ksvg2/svg/SVGGradientElement.cpp
WebCore/ksvg2/svg/SVGPatternElement.cpp
WebCore/ksvg2/svg/SVGStyledTransformableElement.cpp
WebCore/ksvg2/svg/SVGTextElement.cpp
WebCore/ksvg2/svg/SVGTransformable.cpp
WebCore/ksvg2/svg/SVGTransformable.h

index 3e965145e7e43fac80a241b1b2778787bac54d76..1be66ae57b8f4a3709562a6379aede304ea51357 100644 (file)
@@ -1,3 +1,21 @@
+2006-12-08  Rob Buis  <buis@kde.org>
+
+        Reviewed by Darin.
+
+        Testcase for:
+        http://bugs.webkit.org/show_bug.cgi?id=6074
+        WebKit+SVG and FireFox disagree on invalid "transform" handling
+
+        Also new results for transform-ignore-after-invalid.svg.
+
+        * svg/custom/invalid-transforms-expected.checksum: Added.
+        * svg/custom/invalid-transforms-expected.png: Added.
+        * svg/custom/invalid-transforms-expected.txt: Added.
+        * svg/custom/invalid-transforms.svg: Added.
+        * svg/custom/transform-ignore-after-invalid-expected.checksum:
+        * svg/custom/transform-ignore-after-invalid-expected.png:
+        * svg/custom/transform-ignore-after-invalid-expected.txt:
+
 2006-12-07  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Beth Dakin.
diff --git a/LayoutTests/svg/custom/invalid-transforms-expected.checksum b/LayoutTests/svg/custom/invalid-transforms-expected.checksum
new file mode 100644 (file)
index 0000000..21cbe4f
--- /dev/null
@@ -0,0 +1 @@
+65e3faef73df388a545546ead8eddc2d
\ No newline at end of file
diff --git a/LayoutTests/svg/custom/invalid-transforms-expected.png b/LayoutTests/svg/custom/invalid-transforms-expected.png
new file mode 100644 (file)
index 0000000..57ac462
Binary files /dev/null and b/LayoutTests/svg/custom/invalid-transforms-expected.png differ
diff --git a/LayoutTests/svg/custom/invalid-transforms-expected.txt b/LayoutTests/svg/custom/invalid-transforms-expected.txt
new file mode 100644 (file)
index 0000000..86ca854
--- /dev/null
@@ -0,0 +1,5 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+    KCanvasContainer {svg} at (0,0) size 100x100
+      KCanvasItem {rect} at (0,0) size 100x100 [fill={[type=SOLID] [color=#FF0000]}] [data="M0.00,0.00L100.00,0.00L100.00,100.00L0.00,100.00"]
+      KCanvasItem {rect} at (0,0) size 50x50 [fill={[type=SOLID] [color=#008000]}] [data="M0.00,0.00L50.00,0.00L50.00,50.00L0.00,50.00"]
diff --git a/LayoutTests/svg/custom/invalid-transforms.svg b/LayoutTests/svg/custom/invalid-transforms.svg
new file mode 100644 (file)
index 0000000..3b9d83b
--- /dev/null
@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1 Basic//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11-basic.dtd">
+<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100" viewBox="0 0 100 100" onload="runTest()">
+    <script type="text/ecmascript"><![CDATA[
+    function runTest() {
+        var element = document.getElementById('test');
+        element.setAttribute("transform", "rotate(45) scale(1,2,3)");
+    }
+    ]]></script>
+    <!-- The current behavior is to ignore invalid transforms (and any transforms after).
+         We might choose to enter an "error state" on an invlalid transform in the future. -->
+    <rect width="100" height="100" fill="red"/>
+    <rect id="test" width="50" height="50" transform="scale(2,2) scale(1,2,3) translate(50,50)" fill="green"/>
+</svg>
index 736384124a3002d4f089de3cb33b8bcfc8632d28..21cbe4fa1333ad05138651f0804361bd4f754b45 100644 (file)
@@ -1 +1 @@
-778803df0a824ed8f2c7dfa07c56832e
\ No newline at end of file
+65e3faef73df388a545546ead8eddc2d
\ No newline at end of file
index 3db2634a02cf66b801aa1b94cb06d666a42e429f..57ac462b99b92f30cb92aa89a4a1ab5840825a02 100644 (file)
Binary files a/LayoutTests/svg/custom/transform-ignore-after-invalid-expected.png and b/LayoutTests/svg/custom/transform-ignore-after-invalid-expected.png differ
index b5e0de1ac30a10ee5d1e302c883df6276ddfdf93..86ca854ab6951b74c5c311a5276a4daada4f1523 100644 (file)
@@ -2,4 +2,4 @@ layer at (0,0) size 800x600
   RenderView at (0,0) size 800x600
     KCanvasContainer {svg} at (0,0) size 100x100
       KCanvasItem {rect} at (0,0) size 100x100 [fill={[type=SOLID] [color=#FF0000]}] [data="M0.00,0.00L100.00,0.00L100.00,100.00L0.00,100.00"]
-      KCanvasItem {rect} at (0,0) size 100x100 [transform={m=((2.00,0.00)(0.00,2.00)) t=(0.00,0.00)}] [fill={[type=SOLID] [color=#008000]}] [data="M0.00,0.00L50.00,0.00L50.00,50.00L0.00,50.00"]
+      KCanvasItem {rect} at (0,0) size 50x50 [fill={[type=SOLID] [color=#008000]}] [data="M0.00,0.00L50.00,0.00L50.00,50.00L0.00,50.00"]
index 46099a3ce4661f6d043855e4b08ac100c34dfa3a..af3ad06b7b152775d4a7843b97c183cfe7a3acc1 100644 (file)
@@ -1,3 +1,25 @@
+2006-12-08  Rob Buis  <buis@kde.org>
+
+        Reviewed by Darin.
+
+        http://bugs.webkit.org/show_bug.cgi?id=6074
+        WebKit+SVG and FireFox disagree on invalid "transform" handling
+
+        Test for parsing failure on transform attribute and clear the transform
+        list upon failure to match FF behaviour.
+
+        * ksvg2/svg/SVGGradientElement.cpp:
+        (WebCore::SVGGradientElement::parseMappedAttribute):
+        * ksvg2/svg/SVGPatternElement.cpp:
+        (WebCore::SVGPatternElement::parseMappedAttribute):
+        * ksvg2/svg/SVGStyledTransformableElement.cpp:
+        (WebCore::SVGStyledTransformableElement::parseMappedAttribute):
+        * ksvg2/svg/SVGTextElement.cpp:
+        (WebCore::SVGTextElement::parseMappedAttribute):
+        * ksvg2/svg/SVGTransformable.cpp:
+        (WebCore::SVGTransformable::parseTransformAttribute):
+        * ksvg2/svg/SVGTransformable.h:
+
 2006-12-07  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Beth Dakin.
         (WebCore::fillContainerFromString): Call stringWithRelabacedWhitespace.  Pass 
         it the startOfParagraph/endOfParagraph bools.
 
+>>>>>>> .r18054
 2006-12-05  John Sullivan  <sullivan@apple.com>
 
         Reviewed by Beth
index fd8637fdac73d17877a826e90e13f2553cb9cf68..f2821ccaa22ff3e108717956aef9fe2d5a5e5d1b 100644 (file)
@@ -67,7 +67,10 @@ void SVGGradientElement::parseMappedAttribute(MappedAttribute* attr)
             setGradientUnitsBaseValue(SVGUnitTypes::SVG_UNIT_TYPE_OBJECTBOUNDINGBOX);
     } else if (attr->name() == SVGNames::gradientTransformAttr) {
         SVGTransformList* gradientTransforms = gradientTransformBaseValue();
-        SVGTransformable::parseTransformAttribute(gradientTransforms, attr->value());
+        if (!SVGTransformable::parseTransformAttribute(gradientTransforms, attr->value())) {
+            ExceptionCode ec = 0;
+            gradientTransforms->clear(ec);
+        }
     } else if (attr->name() == SVGNames::spreadMethodAttr) {
         if (value == "reflect")
             setSpreadMethodBaseValue(SVG_SPREADMETHOD_REFLECT);
index 257f6c5430109d26ba1dba11a3f5f0031332f375..05fac791ba38ca315a2bd77ba82c522cd25a7635 100644 (file)
@@ -90,7 +90,10 @@ void SVGPatternElement::parseMappedAttribute(MappedAttribute* attr)
             setPatternContentUnitsBaseValue(SVGUnitTypes::SVG_UNIT_TYPE_OBJECTBOUNDINGBOX);
     } else if (attr->name() == SVGNames::patternTransformAttr) {
         SVGTransformList* patternTransforms = patternTransformBaseValue();
-        SVGTransformable::parseTransformAttribute(patternTransforms, value);
+        if (!SVGTransformable::parseTransformAttribute(patternTransforms, value)) {
+            ExceptionCode ec = 0;
+            patternTransforms->clear(ec);
+        }
     } else if (attr->name() == SVGNames::xAttr)
         xBaseValue()->setValueAsString(value);
     else if (attr->name() == SVGNames::yAttr)
index 21c6090070c6a688369251696975a0574fb1535c..abccdbc14e7853b429a79155959a1c55a1d897b7 100644 (file)
@@ -85,9 +85,11 @@ void SVGStyledTransformableElement::parseMappedAttribute(MappedAttribute* attr)
 
         ExceptionCode ec = 0;
         localTransforms->clear(ec);
-        
-        SVGTransformable::parseTransformAttribute(localTransforms, attr->value());
-        updateLocalTransform(localTransforms);
+        if (!SVGTransformable::parseTransformAttribute(localTransforms, attr->value()))
+            localTransforms->clear(ec);
+        else
+            updateLocalTransform(localTransforms);
     } else
         SVGStyledLocatableElement::parseMappedAttribute(attr);
 }
index 4048e97ca81403852c17c92740f6d8557a4e03f8..74de596553fe068c63d50c5b5e4201dea015e32c 100644 (file)
@@ -59,9 +59,11 @@ void SVGTextElement::parseMappedAttribute(MappedAttribute* attr)
 
         ExceptionCode ec = 0;
         localTransforms->clear(ec);
-        
-        SVGTransformable::parseTransformAttribute(localTransforms, attr->value());
-        updateLocalTransform(localTransforms);
+
+        if (!SVGTransformable::parseTransformAttribute(localTransforms, attr->value()))
+            localTransforms->clear(ec);
+        else
+            updateLocalTransform(localTransforms);
     } else
         SVGTextPositioningElement::parseMappedAttribute(attr);
 }
index 918f4d54d8887d2f5506605a212a32d87826b076..c4bdaaf20aee6869859d7d47204d6321071dd627 100644 (file)
@@ -55,16 +55,16 @@ SVGMatrix* SVGTransformable::getScreenCTM(const SVGElement* element) const
     return ctm;
 }
 
-void SVGTransformable::parseTransformAttribute(SVGTransformList* list, const AtomicString& transform)
+bool SVGTransformable::parseTransformAttribute(SVGTransformList* list, const AtomicString& transform)
 {
     // Split string for handling 1 transform statement at a time
     Vector<String> subtransforms = transform.domString().simplifyWhiteSpace().split(')');
     Vector<String>::const_iterator end = subtransforms.end();
     for (Vector<String>::const_iterator it = subtransforms.begin(); it != end; ++it) {
         Vector<String> subtransform = (*it).split('(');
-        
+
         if (subtransform.size() < 2)
-            break; // invalid transform, ignore.
+            return false; // invalid transform, ignore.
 
         subtransform[0] = subtransform[0].stripWhiteSpace().lower();
         subtransform[1] = subtransform[1].simplifyWhiteSpace();
@@ -72,7 +72,7 @@ void SVGTransformable::parseTransformAttribute(SVGTransformList* list, const Ato
 
         int pos = 0;
         Vector<String> params;
-        
+
         while (pos >= 0) {
             pos = reg.search(subtransform[1].deprecatedString(), pos);
             if (pos != -1) {
@@ -80,37 +80,38 @@ void SVGTransformable::parseTransformAttribute(SVGTransformList* list, const Ato
                 pos += reg.matchedLength();
             }
         }
-        
-        if (params.size() < 1)
-            break;
+        int nparams = params.size();
+
+        if (nparams < 1)
+            return false; // invalid transform, ignore.
 
         if (subtransform[0].startsWith(";") || subtransform[0].startsWith(","))
             subtransform[0] = subtransform[0].substring(1).stripWhiteSpace();
 
         SVGTransform* t = new SVGTransform();
 
-        if (subtransform[0] == "rotate") {
-            if (params.size() == 3)
+        if (subtransform[0] == "rotate" && (nparams == 1 || nparams == 3)) {
+            if (nparams == 3)
                 t->setRotate(params[0].toDouble(),
                              params[1].toDouble(),
                               params[2].toDouble());
-            else if (params.size() == 1)
+            else
                 t->setRotate(params[0].toDouble(), 0, 0);
-        } else if (subtransform[0] == "translate") {
-            if (params.size() == 2)
+        } else if (subtransform[0] == "translate" && (nparams == 1 || nparams == 2)) {
+            if (nparams == 2)
                 t->setTranslate(params[0].toDouble(), params[1].toDouble());
-            else if (params.size() == 1) // Spec: if only one param given, assume 2nd param to be 0
+            else // Spec: if only one param given, assume 2nd param to be 0
                 t->setTranslate(params[0].toDouble(), 0);
-        } else if (subtransform[0] == "scale") {
-            if (params.size() == 2)
+        } else if (subtransform[0] == "scale" && (nparams == 1 || nparams == 2)) {
+            if (nparams == 2)
                 t->setScale(params[0].toDouble(), params[1].toDouble());
-            else if (params.size() == 1) // Spec: if only one param given, assume uniform scaling
+            else // Spec: if only one param given, assume uniform scaling
                 t->setScale(params[0].toDouble(), params[0].toDouble());
-        } else if (subtransform[0] == "skewx" && (params.size() == 1))
+        } else if (subtransform[0] == "skewx" && nparams == 1)
             t->setSkewX(params[0].toDouble());
-        else if (subtransform[0] == "skewy" && (params.size() == 1))
+        else if (subtransform[0] == "skewy" && nparams == 1)
             t->setSkewY(params[0].toDouble());
-        else if (subtransform[0] == "matrix" && (params.size() == 6)) {
+        else if (subtransform[0] == "matrix" && nparams == 6) {
             SVGMatrix* ret = new SVGMatrix(params[0].toDouble(),
                                            params[1].toDouble(),
                                            params[2].toDouble(),
@@ -118,16 +119,16 @@ void SVGTransformable::parseTransformAttribute(SVGTransformList* list, const Ato
                                            params[4].toDouble(),
                                            params[5].toDouble());
             t->setMatrix(ret);
-        }
-        
-        if (t->type() == SVGTransform::SVG_TRANSFORM_UNKNOWN) {
+        } else {
             delete t;
-            break; // failed to parse a valid transform, abort.
+            return false; // failed to parse a valid transform, abort.
         }
 
         ExceptionCode ec = 0;
         list->appendItem(t, ec);
     }
+
+    return true;
 }
 
 }
index b52dedfa2c62565cd7d4d63821cf6ed47ba6e3e6..391d27b6e3030adefcc9739597bbe48141c46159 100644 (file)
@@ -42,7 +42,7 @@ namespace WebCore {
 
         virtual void updateLocalTransform(SVGTransformList*) = 0;
 
-        static void parseTransformAttribute(SVGTransformList*, const AtomicString& transform);
+        static bool parseTransformAttribute(SVGTransformList*, const AtomicString& transform);
         SVGMatrix* getCTM(const SVGElement*) const;
         SVGMatrix* getScreenCTM(const SVGElement*) const;
     };