Browser does not fall back to SVG attribute value when CSS style value is invalid...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 29 Nov 2015 22:03:18 +0000 (22:03 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 29 Nov 2015 22:03:18 +0000 (22:03 +0000)
https://bugs.webkit.org/show_bug.cgi?id=147932

Patch by Antoine Quint <graouts@apple.com> on 2015-11-29
Reviewed by Dean Jackson.

Source/WebCore:

Instead of returning an SVGPaint object of type SVG_PAINTTYPE_UNKNOWN when we encounter an SVG paint
value that cannot be parsed, we now return `nullptr` which will cause that value to be ignored and
let another paint value in the cascade be used instead. This is the same approach used for SVGColor.
Since we're removing the only call site for `SVGPaint::createUnknown()`, we remove that function entirely.

Tests: svg/css/invalid-color-cascade.svg
       svg/css/invalid-paint-cascade.svg

* css/SVGCSSParser.cpp:
(WebCore::CSSParser::parseSVGPaint):
* svg/SVGPaint.h:
(WebCore::SVGPaint::createUnknown): Deleted.

LayoutTests:

Testing that we correctly fall back to the presentation attribute for SVGPaint and SVGColor values
specified with an invalid keyword in a `style` attribute. We also update the expected output for
svg/css/svg-attribute-parser-mode.html which is now in line with values returned by Firefox and
Chrome, where we correctly use the default value instead of null objects, which was definitely
an error.

* svg/css/invalid-color-cascade-expected.svg: Added.
* svg/css/invalid-color-cascade.svg: Added.
* svg/css/invalid-paint-cascade-expected.svg: Added.
* svg/css/invalid-paint-cascade.svg: Added.
* svg/css/script-tests/svg-attribute-parser-mode.js:
* svg/css/svg-attribute-parser-mode-expected.txt:

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

LayoutTests/ChangeLog
LayoutTests/svg/css/invalid-color-cascade-expected.svg [new file with mode: 0644]
LayoutTests/svg/css/invalid-color-cascade.svg [new file with mode: 0644]
LayoutTests/svg/css/invalid-paint-cascade-expected.svg [new file with mode: 0644]
LayoutTests/svg/css/invalid-paint-cascade.svg [new file with mode: 0644]
LayoutTests/svg/css/script-tests/svg-attribute-parser-mode.js
LayoutTests/svg/css/svg-attribute-parser-mode-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/css/SVGCSSParser.cpp
Source/WebCore/svg/SVGPaint.h

index 694f0c5..748720e 100644 (file)
@@ -1,3 +1,23 @@
+2015-11-29  Antoine Quint  <graouts@apple.com>
+
+        Browser does not fall back to SVG attribute value when CSS style value is invalid or not supported
+        https://bugs.webkit.org/show_bug.cgi?id=147932
+
+        Reviewed by Dean Jackson.
+
+        Testing that we correctly fall back to the presentation attribute for SVGPaint and SVGColor values
+        specified with an invalid keyword in a `style` attribute. We also update the expected output for
+        svg/css/svg-attribute-parser-mode.html which is now in line with values returned by Firefox and
+        Chrome, where we correctly use the default value instead of null objects, which was definitely
+        an error.
+
+        * svg/css/invalid-color-cascade-expected.svg: Added.
+        * svg/css/invalid-color-cascade.svg: Added.
+        * svg/css/invalid-paint-cascade-expected.svg: Added.
+        * svg/css/invalid-paint-cascade.svg: Added.
+        * svg/css/script-tests/svg-attribute-parser-mode.js:
+        * svg/css/svg-attribute-parser-mode-expected.txt:
+
 2015-11-18  Andy Estes  <aestes@apple.com>
 
         [Content Filtering] Crash in DocumentLoader::notifyFinished() when allowing a media document to load
diff --git a/LayoutTests/svg/css/invalid-color-cascade-expected.svg b/LayoutTests/svg/css/invalid-color-cascade-expected.svg
new file mode 100644 (file)
index 0000000..73c0cdd
--- /dev/null
@@ -0,0 +1,3 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+    <rect width="100%" height="100%" fill="green" />
+</svg>
\ No newline at end of file
diff --git a/LayoutTests/svg/css/invalid-color-cascade.svg b/LayoutTests/svg/css/invalid-color-cascade.svg
new file mode 100644 (file)
index 0000000..c1cde13
--- /dev/null
@@ -0,0 +1,9 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+    <defs>
+        <linearGradient id="gradient">
+            <stop offset="0" stop-color="green" style="stop-color: invalid" />
+            <stop offset="1" stop-color="green" style="stop-color: invalid" />
+        </linearGradient>
+    </defs>
+    <rect width="100%" height="100%" fill="url(#gradient)" />
+</svg>
\ No newline at end of file
diff --git a/LayoutTests/svg/css/invalid-paint-cascade-expected.svg b/LayoutTests/svg/css/invalid-paint-cascade-expected.svg
new file mode 100644 (file)
index 0000000..73c0cdd
--- /dev/null
@@ -0,0 +1,3 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+    <rect width="100%" height="100%" fill="green" />
+</svg>
\ No newline at end of file
diff --git a/LayoutTests/svg/css/invalid-paint-cascade.svg b/LayoutTests/svg/css/invalid-paint-cascade.svg
new file mode 100644 (file)
index 0000000..764a592
--- /dev/null
@@ -0,0 +1,3 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+    <rect width="100%" height="100%" fill="green" style="fill: invalid" />
+</svg>
\ No newline at end of file
index b06c8a3..4f425a5 100644 (file)
@@ -21,25 +21,25 @@ shouldBeEqualToString("document.defaultView.getComputedStyle(rect, null).fill",
 
 // Set following colors should be invalid.
 rect.setAttribute("fill", "f00");
-shouldBeNull("document.defaultView.getComputedStyle(rect, null).fill");
+shouldBeEqualToString("document.defaultView.getComputedStyle(rect, null).fill", "rgb(0, 0, 0)");
 // Reset to green.
 rect.setAttribute("fill", "green");
 shouldBeEqualToString("document.defaultView.getComputedStyle(rect, null).fill", "rgb(0, 128, 0)");
 
 rect.setAttribute("fill", "ff00");
-shouldBeNull("document.defaultView.getComputedStyle(rect, null).fill");
+shouldBeEqualToString("document.defaultView.getComputedStyle(rect, null).fill", "rgb(0, 0, 0)");
 // Reset to green.
 rect.setAttribute("fill", "green");
 shouldBeEqualToString("document.defaultView.getComputedStyle(rect, null).fill", "rgb(0, 128, 0)");
 
 rect.setAttribute("fill", "ff0000");
-shouldBeNull("document.defaultView.getComputedStyle(rect, null).fill");
+shouldBeEqualToString("document.defaultView.getComputedStyle(rect, null).fill", "rgb(0, 0, 0)");
 // Reset to green.
 rect.setAttribute("fill", "green");
 shouldBeEqualToString("document.defaultView.getComputedStyle(rect, null).fill", "rgb(0, 128, 0)");
 
 rect.setAttribute("fill", "ff00");
-shouldBeNull("document.defaultView.getComputedStyle(rect, null).fill");
+shouldBeEqualToString("document.defaultView.getComputedStyle(rect, null).fill", "rgb(0, 0, 0)");
 // Reset to green.
 rect.setAttribute("fill", "green");
 shouldBeEqualToString("document.defaultView.getComputedStyle(rect, null).fill", "rgb(0, 128, 0)");
@@ -73,25 +73,25 @@ shouldBeEqualToString("document.defaultView.getComputedStyle(rect, null).stroke"
 
 // Set following colors should be invalid.
 rect.setAttribute("stroke", "f00");
-shouldBeNull("document.defaultView.getComputedStyle(rect, null).stroke");
+shouldBeEqualToString("document.defaultView.getComputedStyle(rect, null).stroke", "none");
 // Reset to green.
 rect.setAttribute("stroke", "green");
 shouldBeEqualToString("document.defaultView.getComputedStyle(rect, null).stroke", "rgb(0, 128, 0)");
 
 rect.setAttribute("stroke", "ff00");
-shouldBeNull("document.defaultView.getComputedStyle(rect, null).stroke");
+shouldBeEqualToString("document.defaultView.getComputedStyle(rect, null).stroke", "none");
 // Reset to green.
 rect.setAttribute("stroke", "green");
 shouldBeEqualToString("document.defaultView.getComputedStyle(rect, null).stroke", "rgb(0, 128, 0)");
 
 rect.setAttribute("stroke", "ff0000");
-shouldBeNull("document.defaultView.getComputedStyle(rect, null).stroke");
+shouldBeEqualToString("document.defaultView.getComputedStyle(rect, null).stroke", "none");
 // Reset to green.
 rect.setAttribute("stroke", "green");
 shouldBeEqualToString("document.defaultView.getComputedStyle(rect, null).stroke", "rgb(0, 128, 0)");
 
 rect.setAttribute("stroke", "ff00");
-shouldBeNull("document.defaultView.getComputedStyle(rect, null).stroke");
+shouldBeEqualToString("document.defaultView.getComputedStyle(rect, null).stroke", "none");
 // Reset to green.
 rect.setAttribute("stroke", "green");
 shouldBeEqualToString("document.defaultView.getComputedStyle(rect, null).stroke", "rgb(0, 128, 0)");
index d0a9556..219fadc 100644 (file)
@@ -5,13 +5,13 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 PASS document.defaultView.getComputedStyle(rect, null).fill is "rgb(0, 0, 0)"
 PASS document.defaultView.getComputedStyle(rect, null).fill is "rgb(0, 128, 0)"
-PASS document.defaultView.getComputedStyle(rect, null).fill is null
+PASS document.defaultView.getComputedStyle(rect, null).fill is "rgb(0, 0, 0)"
 PASS document.defaultView.getComputedStyle(rect, null).fill is "rgb(0, 128, 0)"
-PASS document.defaultView.getComputedStyle(rect, null).fill is null
+PASS document.defaultView.getComputedStyle(rect, null).fill is "rgb(0, 0, 0)"
 PASS document.defaultView.getComputedStyle(rect, null).fill is "rgb(0, 128, 0)"
-PASS document.defaultView.getComputedStyle(rect, null).fill is null
+PASS document.defaultView.getComputedStyle(rect, null).fill is "rgb(0, 0, 0)"
 PASS document.defaultView.getComputedStyle(rect, null).fill is "rgb(0, 128, 0)"
-PASS document.defaultView.getComputedStyle(rect, null).fill is null
+PASS document.defaultView.getComputedStyle(rect, null).fill is "rgb(0, 0, 0)"
 PASS document.defaultView.getComputedStyle(rect, null).fill is "rgb(0, 128, 0)"
 PASS document.defaultView.getComputedStyle(rect, null).fill is "rgb(0, 0, 0)"
 PASS document.defaultView.getComputedStyle(rect, null).fill is "rgb(0, 128, 0)"
@@ -21,13 +21,13 @@ PASS document.defaultView.getComputedStyle(rect, null).fill is "url(#reference)
 PASS document.defaultView.getComputedStyle(rect, null).fill is "rgb(0, 128, 0)"
 PASS document.defaultView.getComputedStyle(rect, null).stroke is "none"
 PASS document.defaultView.getComputedStyle(rect, null).stroke is "rgb(0, 128, 0)"
-PASS document.defaultView.getComputedStyle(rect, null).stroke is null
+PASS document.defaultView.getComputedStyle(rect, null).stroke is "none"
 PASS document.defaultView.getComputedStyle(rect, null).stroke is "rgb(0, 128, 0)"
-PASS document.defaultView.getComputedStyle(rect, null).stroke is null
+PASS document.defaultView.getComputedStyle(rect, null).stroke is "none"
 PASS document.defaultView.getComputedStyle(rect, null).stroke is "rgb(0, 128, 0)"
-PASS document.defaultView.getComputedStyle(rect, null).stroke is null
+PASS document.defaultView.getComputedStyle(rect, null).stroke is "none"
 PASS document.defaultView.getComputedStyle(rect, null).stroke is "rgb(0, 128, 0)"
-PASS document.defaultView.getComputedStyle(rect, null).stroke is null
+PASS document.defaultView.getComputedStyle(rect, null).stroke is "none"
 PASS document.defaultView.getComputedStyle(rect, null).stroke is "rgb(0, 128, 0)"
 PASS document.defaultView.getComputedStyle(rect, null).stroke is "none"
 PASS document.defaultView.getComputedStyle(rect, null).stroke is "rgb(0, 128, 0)"
index 274a328..b19f350 100644 (file)
@@ -1,3 +1,23 @@
+2015-11-29  Antoine Quint  <graouts@apple.com>
+
+        Browser does not fall back to SVG attribute value when CSS style value is invalid or not supported
+        https://bugs.webkit.org/show_bug.cgi?id=147932
+
+        Reviewed by Dean Jackson.
+
+        Instead of returning an SVGPaint object of type SVG_PAINTTYPE_UNKNOWN when we encounter an SVG paint
+        value that cannot be parsed, we now return `nullptr` which will cause that value to be ignored and
+        let another paint value in the cascade be used instead. This is the same approach used for SVGColor.
+        Since we're removing the only call site for `SVGPaint::createUnknown()`, we remove that function entirely.
+
+        Tests: svg/css/invalid-color-cascade.svg
+               svg/css/invalid-paint-cascade.svg
+
+        * css/SVGCSSParser.cpp:
+        (WebCore::CSSParser::parseSVGPaint):
+        * svg/SVGPaint.h:
+        (WebCore::SVGPaint::createUnknown): Deleted.
+
 2015-11-29  Simon Fraser  <simon.fraser@apple.com>
 
         Use SVGTransform::SVGTransformType instead of an unsigned short
index 6050069..e707477 100644 (file)
@@ -368,7 +368,7 @@ RefPtr<CSSValue> CSSParser::parseSVGPaint()
 {
     RGBA32 c = Color::transparent;
     if (!parseColorFromValue(*m_valueList->current(), c))
-        return SVGPaint::createUnknown();
+        return nullptr;
     return SVGPaint::createColor(Color(c));
 }
 
index 5b313f4..bc15760 100644 (file)
@@ -43,11 +43,6 @@ public:
         SVG_PAINTTYPE_URI = 107
     };
 
-    static Ref<SVGPaint> createUnknown()
-    {
-        return adoptRef(*new SVGPaint(SVG_PAINTTYPE_UNKNOWN));
-    }
-
     static Ref<SVGPaint> createNone()
     {
         return adoptRef(*new SVGPaint(SVG_PAINTTYPE_NONE));