The CSSParser should not consume negative or unit-less lengths for the SVG properties...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Nov 2019 22:45:15 +0000 (22:45 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Nov 2019 22:45:15 +0000 (22:45 +0000)
https://bugs.webkit.org/show_bug.cgi?id=204200

Patch by Said Abou-Hallawa <sabouhallawa@apple.com> on 2019-11-14
Reviewed by Simon Fraser.

LayoutTests/imported/w3c:

* web-platform-tests/svg/geometry/parsing/r-computed-expected.txt:
* web-platform-tests/svg/geometry/parsing/r-invalid-expected.txt:
* web-platform-tests/svg/geometry/parsing/rx-computed-expected.txt:
* web-platform-tests/svg/geometry/parsing/rx-invalid-expected.txt:
* web-platform-tests/svg/geometry/parsing/ry-computed-expected.txt:
* web-platform-tests/svg/geometry/parsing/ry-invalid-expected.txt:

Source/WebCore:

Both the CSSParser and SVGElement::parseAttribute() should be consistent
when parsing these properties.

The type of these properties is <length-percentage>, which requires specifying
the unit for the length: https://www.w3.org/TR/css-values/#length-value.

* css/parser/CSSParserFastPaths.cpp:
(WebCore::isSimpleLengthPropertyID):
* css/parser/CSSPropertyParser.cpp:
(WebCore::consumeRxOrRy):
(WebCore::CSSPropertyParser::parseSingleValue):

LayoutTests:

* svg/css/parse-length-expected.txt:
* svg/css/parse-length.html:

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

13 files changed:
LayoutTests/ChangeLog
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/svg/geometry/parsing/r-computed-expected.txt
LayoutTests/imported/w3c/web-platform-tests/svg/geometry/parsing/r-invalid-expected.txt
LayoutTests/imported/w3c/web-platform-tests/svg/geometry/parsing/rx-computed-expected.txt
LayoutTests/imported/w3c/web-platform-tests/svg/geometry/parsing/rx-invalid-expected.txt
LayoutTests/imported/w3c/web-platform-tests/svg/geometry/parsing/ry-computed-expected.txt
LayoutTests/imported/w3c/web-platform-tests/svg/geometry/parsing/ry-invalid-expected.txt
LayoutTests/svg/css/parse-length-expected.txt
LayoutTests/svg/css/parse-length.html
Source/WebCore/ChangeLog
Source/WebCore/css/parser/CSSParserFastPaths.cpp
Source/WebCore/css/parser/CSSPropertyParser.cpp

index 1d8ad5b..a543fd0 100644 (file)
@@ -1,3 +1,13 @@
+2019-11-14  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        The CSSParser should not consume negative or unit-less lengths for the SVG properties 'r', 'rx' and 'ry'
+        https://bugs.webkit.org/show_bug.cgi?id=204200
+
+        Reviewed by Simon Fraser.
+
+        * svg/css/parse-length-expected.txt:
+        * svg/css/parse-length.html:
+
 2019-11-14  Antoine Quint  <graouts@apple.com>
 
         [Web Animations] Accelerated transitions do not always remove their backing accelerated animation
index 00b7caf..3150da2 100644 (file)
@@ -1,3 +1,17 @@
+2019-11-14  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        The CSSParser should not consume negative or unit-less lengths for the SVG properties 'r', 'rx' and 'ry'
+        https://bugs.webkit.org/show_bug.cgi?id=204200
+
+        Reviewed by Simon Fraser.
+
+        * web-platform-tests/svg/geometry/parsing/r-computed-expected.txt:
+        * web-platform-tests/svg/geometry/parsing/r-invalid-expected.txt:
+        * web-platform-tests/svg/geometry/parsing/rx-computed-expected.txt:
+        * web-platform-tests/svg/geometry/parsing/rx-invalid-expected.txt:
+        * web-platform-tests/svg/geometry/parsing/ry-computed-expected.txt:
+        * web-platform-tests/svg/geometry/parsing/ry-invalid-expected.txt:
+
 2019-11-14  Antoine Quint  <graouts@apple.com>
 
         [Web Animations] Accelerated transitions do not always remove their backing accelerated animation
index 039173f..2c0af0e 100644 (file)
@@ -2,7 +2,7 @@
 PASS Property r value '10px' computes to '10px' 
 PASS Property r value '0.5em' computes to '20px' 
 PASS Property r value 'calc(10px + 0.5em)' computes to '30px' 
-FAIL Property r value 'calc(10px - 0.5em)' computes to '0px' assert_equals: expected "0px" but got "-10px"
+PASS Property r value 'calc(10px - 0.5em)' computes to '0px' 
 PASS Property r value '40%' computes to '40%' 
 PASS Property r value 'calc(50% + 60px)' computes to 'calc(50% + 60px)' 
 
index 2730d04..aa20b30 100644 (file)
@@ -1,7 +1,7 @@
 
-FAIL e.style['r'] = "10" should not set the property value assert_equals: expected "" but got "10px"
+PASS e.style['r'] = "10" should not set the property value 
 PASS e.style['r'] = "auto" should not set the property value 
 PASS e.style['r'] = "10px 20px" should not set the property value 
-FAIL e.style['r'] = "-1px" should not set the property value assert_equals: expected "" but got "-1px"
-FAIL e.style['r'] = "-10%" should not set the property value assert_equals: expected "" but got "-10%"
+PASS e.style['r'] = "-1px" should not set the property value 
+PASS e.style['r'] = "-10%" should not set the property value 
 
index 66c54dd..39b824c 100644 (file)
@@ -3,7 +3,7 @@ PASS Property rx value 'auto' computes to 'auto'
 PASS Property rx value '10px' computes to '10px' 
 PASS Property rx value '0.5em' computes to '20px' 
 PASS Property rx value 'calc(10px + 0.5em)' computes to '30px' 
-FAIL Property rx value 'calc(10px - 0.5em)' computes to '0px' assert_equals: expected "0px" but got "-10px"
+PASS Property rx value 'calc(10px - 0.5em)' computes to '0px' 
 PASS Property rx value '40%' computes to '40%' 
 PASS Property rx value 'calc(50% + 60px)' computes to 'calc(50% + 60px)' 
 
index 78d9c5f..a5c13e9 100644 (file)
@@ -1,6 +1,6 @@
 
-FAIL e.style['rx'] = "10" should not set the property value assert_equals: expected "" but got "10px"
+PASS e.style['rx'] = "10" should not set the property value 
 PASS e.style['rx'] = "none" should not set the property value 
 PASS e.style['rx'] = "10px 20px" should not set the property value 
-FAIL e.style['rx'] = "-1px" should not set the property value assert_equals: expected "" but got "-1px"
+PASS e.style['rx'] = "-1px" should not set the property value 
 
index bdfef6c..25e6e8c 100644 (file)
@@ -3,7 +3,7 @@ PASS Property ry value 'auto' computes to 'auto'
 PASS Property ry value '10px' computes to '10px' 
 PASS Property ry value '0.5em' computes to '20px' 
 PASS Property ry value 'calc(10px + 0.5em)' computes to '30px' 
-FAIL Property ry value 'calc(10px - 0.5em)' computes to '0px' assert_equals: expected "0px" but got "-10px"
+PASS Property ry value 'calc(10px - 0.5em)' computes to '0px' 
 PASS Property ry value '40%' computes to '40%' 
 PASS Property ry value 'calc(50% + 60px)' computes to 'calc(50% + 60px)' 
 
index 63356e9..dbbafc7 100644 (file)
@@ -1,6 +1,6 @@
 
-FAIL e.style['ry'] = "10" should not set the property value assert_equals: expected "" but got "10px"
+PASS e.style['ry'] = "10" should not set the property value 
 PASS e.style['ry'] = "none" should not set the property value 
 PASS e.style['ry'] = "10px 20px" should not set the property value 
-FAIL e.style['ry'] = "-1px" should not set the property value assert_equals: expected "" but got "-1px"
+PASS e.style['ry'] = "-1px" should not set the property value 
 
index 38a99a3..acf1e4d 100644 (file)
@@ -61,21 +61,21 @@ PASS computedStyle("r", "100px") is "100px"
 PASS computedStyle("r", "1em") is "16px"
 PASS computedStyle("r", "1ex") is "12.800000190734863px"
 PASS computedStyle("r", "20%") is "20%"
-PASS computedStyle("r", "-200px") is "-200px"
+PASS computedStyle("r", "-200px") is "0px"
 PASS computedStyle("rx", "  100") is "100px"
 PASS computedStyle("rx", "100   ") is "100px"
 PASS computedStyle("rx", "100px") is "100px"
 PASS computedStyle("rx", "1em") is "16px"
 PASS computedStyle("rx", "1ex") is "12.800000190734863px"
 PASS computedStyle("rx", "20%") is "20%"
-PASS computedStyle("rx", "-200px") is "-200px"
+PASS computedStyle("rx", "-200px") is "auto"
 PASS computedStyle("ry", "  100") is "100px"
 PASS computedStyle("ry", "100   ") is "100px"
 PASS computedStyle("ry", "100px") is "100px"
 PASS computedStyle("ry", "1em") is "16px"
 PASS computedStyle("ry", "1ex") is "12.800000190734863px"
 PASS computedStyle("ry", "20%") is "20%"
-PASS computedStyle("ry", "-200px") is "-200px"
+PASS computedStyle("ry", "-200px") is "auto"
 PASS computedStyle("width", "auto") is "auto"
 PASS computedStyle("width", "  100") is "100px"
 PASS computedStyle("width", "100   ") is "100px"
index 39404a8..839d446 100644 (file)
@@ -57,7 +57,7 @@ testComputed("r", "100px", "100px");
 testComputed("r", "1em", "16px");
 testComputed("r", "1ex", "12.800000190734863px");
 testComputed("r", "20%", "20%");
-testComputed("r", "-200px", "-200px");
+testComputed("r", "-200px", "0px");
 
 // Test 'rx'.
 testComputed("rx", "  100", "100px");
@@ -66,7 +66,7 @@ testComputed("rx", "100px", "100px");
 testComputed("rx", "1em", "16px");
 testComputed("rx", "1ex", "12.800000190734863px");
 testComputed("rx", "20%", "20%");
-testComputed("rx", "-200px", "-200px");
+testComputed("rx", "-200px", "auto");
 
 // Test 'ry'.
 testComputed("ry", "  100", "100px");
@@ -75,7 +75,7 @@ testComputed("ry", "100px", "100px");
 testComputed("ry", "1em", "16px");
 testComputed("ry", "1ex", "12.800000190734863px");
 testComputed("ry", "20%", "20%");
-testComputed("ry", "-200px", "-200px");
+testComputed("ry", "-200px", "auto");
 
 // Test 'width'.
 testComputed("width", "auto", "auto");
index 48700e3..cfb1ada 100644 (file)
@@ -1,3 +1,22 @@
+2019-11-14  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        The CSSParser should not consume negative or unit-less lengths for the SVG properties 'r', 'rx' and 'ry'
+        https://bugs.webkit.org/show_bug.cgi?id=204200
+
+        Reviewed by Simon Fraser.
+
+        Both the CSSParser and SVGElement::parseAttribute() should be consistent
+        when parsing these properties.
+
+        The type of these properties is <length-percentage>, which requires specifying
+        the unit for the length: https://www.w3.org/TR/css-values/#length-value.
+
+        * css/parser/CSSParserFastPaths.cpp:
+        (WebCore::isSimpleLengthPropertyID):
+        * css/parser/CSSPropertyParser.cpp:
+        (WebCore::consumeRxOrRy):
+        (WebCore::CSSPropertyParser::parseSingleValue):
+
 2019-11-14  Zalan Bujtas  <zalan@apple.com>
 
         [LFC][Invalidation] Skip non-dirty out-of-flow boxes.
index c3ad8ba..a4f72d6 100644 (file)
@@ -64,6 +64,9 @@ static inline bool isSimpleLengthPropertyID(CSSPropertyID propertyId, bool& acce
     case CSSPropertyPaddingBlockStart:
     case CSSPropertyPaddingInlineEnd:
     case CSSPropertyPaddingInlineStart:
+    case CSSPropertyR:
+    case CSSPropertyRx:
+    case CSSPropertyRy:
     case CSSPropertyShapeMargin:
         acceptsNegativeNumbers = false;
         return true;
@@ -87,9 +90,6 @@ static inline bool isSimpleLengthPropertyID(CSSPropertyID propertyId, bool& acce
     case CSSPropertyMarginInlineStart:
     case CSSPropertyX:
     case CSSPropertyY:
-    case CSSPropertyR:
-    case CSSPropertyRx:
-    case CSSPropertyRy:
         acceptsNegativeNumbers = true;
         return true;
     default:
index dbfcb18..f91cd78 100644 (file)
@@ -2140,11 +2140,11 @@ static RefPtr<CSSPrimitiveValue> consumeBaselineShift(CSSParserTokenRange& range
     return consumeLengthOrPercent(range, SVGAttributeMode, ValueRangeAll);
 }
 
-static RefPtr<CSSPrimitiveValue> consumeRxOrRy(CSSParserTokenRange& range)
+static RefPtr<CSSPrimitiveValue> consumeRxOrRy(CSSParserTokenRange& range, CSSParserMode cssParserMode)
 {
     if (range.peek().id() == CSSValueAuto)
         return consumeIdent(range);
-    return consumeLengthOrPercent(range, SVGAttributeMode, ValueRangeAll, UnitlessQuirk::Forbid);
+    return consumeLengthOrPercent(range, cssParserMode, ValueRangeNonNegative, UnitlessQuirk::Forbid);
 }
 
 static RefPtr<CSSValue> consumeCursor(CSSParserTokenRange& range, const CSSParserContext& context, bool inQuirksMode)
@@ -4110,11 +4110,12 @@ RefPtr<CSSValue> CSSPropertyParser::parseSingleValue(CSSPropertyID property, CSS
     case CSSPropertyCy:
     case CSSPropertyX:
     case CSSPropertyY:
-    case CSSPropertyR:
         return consumeLengthOrPercent(m_range, SVGAttributeMode, ValueRangeAll, UnitlessQuirk::Forbid);
+    case CSSPropertyR:
+        return consumeLengthOrPercent(m_range, m_context.mode, ValueRangeNonNegative, UnitlessQuirk::Forbid);
     case CSSPropertyRx:
     case CSSPropertyRy:
-        return consumeRxOrRy(m_range);
+        return consumeRxOrRy(m_range, m_context.mode);
     case CSSPropertyCursor:
         return consumeCursor(m_range, m_context, inQuirksMode());
     case CSSPropertyContent: