Don't do range checking for calc() at parse time
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Dec 2019 16:32:12 +0000 (16:32 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Dec 2019 16:32:12 +0000 (16:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=204737

Reviewed by Antti Koivisto.
LayoutTests/imported/w3c:

* web-platform-tests/css/css-text-decor/parsing/text-shadow-computed-expected.txt:
* web-platform-tests/css/css-values/calc-numbers-expected.txt:
* web-platform-tests/css/css-values/getComputedStyle-border-radius-001-expected.txt:
* web-platform-tests/css/css-values/getComputedStyle-border-radius-003-expected.txt:
* web-platform-tests/css/css-values/minmax-number-serialize-expected.txt:

Source/WebCore:

As specified in https://drafts.csswg.org/css-values-4/#calc-range, we should not do range checking for calc
at parse time; the specified value can be a calc which results in a negative values; values are clamped
at used value time (i.e. when building RenderStyle).

So CSSCalculationValue doesn't need to have isInt(), isPositive(), isNegative(), and CSSCalcExpressionNode
doesn't have to keep track of whether its value is an integer.

Also do some cleanup of CSSPrimitiveValue, using categories to answer more isFoo type questions, and adding
isZero(), isPositive() and isNegative() that return Optional<bool> for cases where parse-time range checking
occurs.

Tested by existing tests.

* css/CSSCalculationValue.cpp:
(WebCore::CSSCalcOperationNode::createSimplified):
(WebCore::CSSCalcExpressionNodeParser::parseValue):
(WebCore::createBlendHalf):
(WebCore::createCSS):
(WebCore::CSSCalcValue::isCalcFunction):
(WebCore::isIntegerResult): Deleted.
* css/CSSCalculationValue.h:
(WebCore::CSSCalcExpressionNode::equals const):
(WebCore::CSSCalcExpressionNode::category const):
(WebCore::CSSCalcExpressionNode::CSSCalcExpressionNode):
(WebCore::CSSCalcExpressionNode::isInteger const): Deleted.
* css/CSSComputedStyleDeclaration.cpp:
(WebCore::zoomAdjustedPixelValueForLength):
(WebCore::percentageOrZoomAdjustedValue):
(WebCore::autoOrZoomAdjustedValue):
* css/CSSPrimitiveValue.cpp:
(WebCore::CSSPrimitiveValue::conversionToCanonicalUnitsScaleFactor): I missed the Q unit here when I added it.
(WebCore::CSSPrimitiveValue::isZero const):
(WebCore::CSSPrimitiveValue::isPositive const):
(WebCore::CSSPrimitiveValue::isNegative const):
(WebCore::CSSPrimitiveValue::canonicalUnitTypeForCategory): Deleted.
* css/CSSPrimitiveValue.h:
(WebCore::CSSPrimitiveValue::isAngle const): Deleted.
* css/CSSUnits.cpp:
(WebCore::unitCategory):
(WebCore::canonicalUnitTypeForCategory):
(WebCore::canonicalUnitType):
* css/CSSUnits.h:
* css/parser/CSSPropertyParser.cpp:
(WebCore::consumeColumnWidth):
(WebCore::consumePerspective):
(WebCore::consumeWebkitAspectRatio):
* css/parser/CSSPropertyParserHelpers.cpp:
(WebCore::CSSPropertyParserHelpers::CalcParser::CalcParser):
(WebCore::CSSPropertyParserHelpers::CalcParser::consumeInteger):
(WebCore::CSSPropertyParserHelpers::consumeNumber):
(WebCore::CSSPropertyParserHelpers::consumeSingleShadow): This parsing is a little tricky. The blur radius value is optional,
but we need to distinguish between failing to parse it, and its value being negative, so an explicit check for calc is the easiest way
to handlel that.
(WebCore::CSSPropertyParserHelpers::CalcParser::consumePositiveIntegerRaw): Deleted. It was unused.
(WebCore::CSSPropertyParserHelpers::consumePositiveIntegerRaw): Deleted.
* css/parser/CSSPropertyParserHelpers.h:

LayoutTests:

Update tests that assumed calc range-checked at parse time.

* fast/css/column-width-calculated-value-expected.txt:
* fast/css/column-width-calculated-value.html:
* fast/css/flex-shrink-calculated-value-expected.txt:
* fast/css/flex-shrink-calculated-value.html:
* fast/css/negative-calc-values-expected.txt:
* fast/css/negative-calc-values.html:
* fast/css/text-shadow-calc-value-expected.txt:
* fast/css/text-shadow-calc-value.html:
* fast/shapes/parsing/parsing-shape-image-threshold-expected.txt:
* fast/shapes/parsing/parsing-shape-image-threshold.html:

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

28 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/css/column-width-calculated-value-expected.txt
LayoutTests/fast/css/column-width-calculated-value.html
LayoutTests/fast/css/flex-shrink-calculated-value-expected.txt
LayoutTests/fast/css/flex-shrink-calculated-value.html
LayoutTests/fast/css/negative-calc-values-expected.txt
LayoutTests/fast/css/negative-calc-values.html
LayoutTests/fast/css/text-shadow-calc-value-expected.txt
LayoutTests/fast/css/text-shadow-calc-value.html
LayoutTests/fast/shapes/parsing/parsing-shape-image-threshold-expected.txt
LayoutTests/fast/shapes/parsing/parsing-shape-image-threshold.html
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/css/css-text-decor/parsing/text-shadow-computed-expected.txt
LayoutTests/imported/w3c/web-platform-tests/css/css-values/calc-numbers-expected.txt
LayoutTests/imported/w3c/web-platform-tests/css/css-values/getComputedStyle-border-radius-001-expected.txt
LayoutTests/imported/w3c/web-platform-tests/css/css-values/getComputedStyle-border-radius-003-expected.txt
LayoutTests/imported/w3c/web-platform-tests/css/css-values/minmax-number-serialize-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/css/CSSCalculationValue.cpp
Source/WebCore/css/CSSCalculationValue.h
Source/WebCore/css/CSSComputedStyleDeclaration.cpp
Source/WebCore/css/CSSPrimitiveValue.cpp
Source/WebCore/css/CSSPrimitiveValue.h
Source/WebCore/css/CSSUnits.cpp
Source/WebCore/css/CSSUnits.h
Source/WebCore/css/parser/CSSPropertyParser.cpp
Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp
Source/WebCore/css/parser/CSSPropertyParserHelpers.h

index 8eea8a6..6b62557 100644 (file)
@@ -1,3 +1,23 @@
+2019-12-02  Simon Fraser  <simon.fraser@apple.com>
+
+        Don't do range checking for calc() at parse time
+        https://bugs.webkit.org/show_bug.cgi?id=204737
+
+        Reviewed by Antti Koivisto.
+
+        Update tests that assumed calc range-checked at parse time.
+
+        * fast/css/column-width-calculated-value-expected.txt:
+        * fast/css/column-width-calculated-value.html:
+        * fast/css/flex-shrink-calculated-value-expected.txt:
+        * fast/css/flex-shrink-calculated-value.html:
+        * fast/css/negative-calc-values-expected.txt:
+        * fast/css/negative-calc-values.html:
+        * fast/css/text-shadow-calc-value-expected.txt:
+        * fast/css/text-shadow-calc-value.html:
+        * fast/shapes/parsing/parsing-shape-image-threshold-expected.txt:
+        * fast/shapes/parsing/parsing-shape-image-threshold.html:
+
 2019-11-30  youenn fablet  <youenn@apple.com>
 
         Update RealtimeOutgoingAudioSourceCocoa::m_writeCount when sampleRate changes
index 4d64583..62e3a1b 100644 (file)
@@ -9,8 +9,8 @@ PASS testDiv.style['column-width'] is "calc(200px)"
 PASS window.getComputedStyle(testDiv).getPropertyValue('column-width') is "200px"
 0px is not a valid value
 testDiv.style['column-width'] = 'calc(0px * 2)'
-PASS testDiv.style['column-width'] is "calc(200px)"
-PASS window.getComputedStyle(testDiv).getPropertyValue('column-width') is "200px"
+PASS testDiv.style['column-width'] is "calc(0px)"
+PASS window.getComputedStyle(testDiv).getPropertyValue('column-width') is "0px"
 PASS successfullyParsed is true
 
 TEST COMPLETE
index b9bce5a..6324f8f 100644 (file)
@@ -14,8 +14,8 @@ shouldBeEqualToString("window.getComputedStyle(testDiv).getPropertyValue('column
 
 debug("0px is not a valid value");
 evalAndLog("testDiv.style['column-width'] = 'calc(0px * 2)'");
-shouldBeEqualToString("testDiv.style['column-width']", "calc(200px)");
-shouldBeEqualToString("window.getComputedStyle(testDiv).getPropertyValue('column-width')", "200px");
+shouldBeEqualToString("testDiv.style['column-width']", "calc(0px)");
+shouldBeEqualToString("window.getComputedStyle(testDiv).getPropertyValue('column-width')", "0px");
 
 </script>
 <script src="../../resources/js-test-post.js"></script>
index e6d636b..9face3b 100644 (file)
@@ -5,7 +5,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 PASS testDiv.style['flex-shrink'] is ""
 testDiv.style['flex-shrink'] = 'calc(2 * 3)'
-PASS testDiv.style['flex-shrink'] is "6"
+PASS testDiv.style['flex-shrink'] is "calc(6)"
 PASS window.getComputedStyle(testDiv).getPropertyValue('flex-shrink') is "6"
 PASS successfullyParsed is true
 
index 5fc21e8..edf805f 100644 (file)
@@ -9,7 +9,7 @@ var testDiv = document.getElementById("testDiv");
 
 shouldBeEmptyString("testDiv.style['flex-shrink']");
 evalAndLog("testDiv.style['flex-shrink'] = 'calc(2 * 3)'");
-shouldBeEqualToString("testDiv.style['flex-shrink']", "6");
+shouldBeEqualToString("testDiv.style['flex-shrink']", "calc(6)");
 shouldBeEqualToString("window.getComputedStyle(testDiv).getPropertyValue('flex-shrink')", "6");
 
 </script>
index cba134c..251352e 100644 (file)
@@ -18,8 +18,8 @@ PASS testDiv.style['line-height'] is "calc(-10%)"
 PASS window.getComputedStyle(testDiv).getPropertyValue('line-height') is "0px"
 testPre.style['tab-size'] = '8'
 testPre.style['tab-size'] = 'calc(2 - 4)'
-PASS testPre.style['tab-size'] is "8"
-PASS window.getComputedStyle(testPre).getPropertyValue('tab-size') is "8"
+PASS testPre.style['tab-size'] is "calc(-2)"
+PASS window.getComputedStyle(testPre).getPropertyValue('tab-size') is "0"
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 18bf70b..a5c7d9a 100644 (file)
@@ -28,8 +28,8 @@ shouldBeEqualToString("window.getComputedStyle(testDiv).getPropertyValue('line-h
 
 evalAndLog("testPre.style['tab-size'] = '8'");
 evalAndLog("testPre.style['tab-size'] = 'calc(2 - 4)'");
-shouldBeEqualToString("testPre.style['tab-size']", "8");
-shouldBeEqualToString("window.getComputedStyle(testPre).getPropertyValue('tab-size')", "8");
+shouldBeEqualToString("testPre.style['tab-size']", "calc(-2)");
+shouldBeEqualToString("window.getComputedStyle(testPre).getPropertyValue('tab-size')", "0");
 
 </script>
 <script src="../../resources/js-test-post.js"></script>
index 1325a29..81268c5 100644 (file)
@@ -3,16 +3,19 @@ Tests assigning a calculated value to 'text-shadow' CSS property.
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
+
 PASS testDiv.style['text-shadow'] is ""
 testDiv.style['text-shadow'] = 'calc(1 * 3px) calc(2 * 3px) calc(3 * 3px) rgb(255, 255, 255)'
 PASS testDiv.style['text-shadow'] is "rgb(255, 255, 255) calc(3px) calc(6px) calc(9px)"
 PASS window.getComputedStyle(testDiv).getPropertyValue('text-shadow') is "rgb(255, 255, 255) 3px 6px 9px"
+
 testDiv.style['text-shadow'] = 'calc(-1 * 3px) calc(-2 * 3px) calc(3 * 3px) rgb(255, 255, 255)'
 PASS testDiv.style['text-shadow'] is "rgb(255, 255, 255) calc(-3px) calc(-6px) calc(9px)"
 PASS window.getComputedStyle(testDiv).getPropertyValue('text-shadow') is "rgb(255, 255, 255) -3px -6px 9px"
+
 testDiv.style['text-shadow'] = 'calc(1 * 3px) calc(2 * 3px) calc(-3 * 3px) rgb(255, 255, 255)'
-PASS testDiv.style['text-shadow'] is "rgb(255, 255, 255) calc(-3px) calc(-6px) calc(9px)"
-PASS window.getComputedStyle(testDiv).getPropertyValue('text-shadow') is "rgb(255, 255, 255) -3px -6px 9px"
+PASS testDiv.style['text-shadow'] is "rgb(255, 255, 255) calc(3px) calc(6px) calc(-9px)"
+PASS window.getComputedStyle(testDiv).getPropertyValue('text-shadow') is "rgb(255, 255, 255) 3px 6px 0px"
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 149e03d..ae7a9d5 100644 (file)
@@ -7,21 +7,26 @@ description("Tests assigning a calculated value to 'text-shadow' CSS property.")
 
 var testDiv = document.getElementById("testDiv");
 
+debug('');
 shouldBeEmptyString("testDiv.style['text-shadow']");
+testDiv.style['text-shadow'] = 'none';
 evalAndLog("testDiv.style['text-shadow'] = 'calc(1 * 3px) calc(2 * 3px) calc(3 * 3px) rgb(255, 255, 255)'");
 shouldBeEqualToString("testDiv.style['text-shadow']", "rgb(255, 255, 255) calc(3px) calc(6px) calc(9px)");
 shouldBeEqualToString("window.getComputedStyle(testDiv).getPropertyValue('text-shadow')", "rgb(255, 255, 255) 3px 6px 9px");
 
 // Negative h-shadow and v-shadow are allowed.
+debug('');
+testDiv.style['text-shadow'] = 'none';
 evalAndLog("testDiv.style['text-shadow'] = 'calc(-1 * 3px) calc(-2 * 3px) calc(3 * 3px) rgb(255, 255, 255)'");
 shouldBeEqualToString("testDiv.style['text-shadow']", "rgb(255, 255, 255) calc(-3px) calc(-6px) calc(9px)");
 shouldBeEqualToString("window.getComputedStyle(testDiv).getPropertyValue('text-shadow')", "rgb(255, 255, 255) -3px -6px 9px")
 
 // Negative blur-radius is not allowed so it should become 0.
+debug('');
+testDiv.style['text-shadow'] = 'none';
 evalAndLog("testDiv.style['text-shadow'] = 'calc(1 * 3px) calc(2 * 3px) calc(-3 * 3px) rgb(255, 255, 255)'");
-// text-shadow should not be updated.
-shouldBeEqualToString("testDiv.style['text-shadow']", "rgb(255, 255, 255) calc(-3px) calc(-6px) calc(9px)");
-shouldBeEqualToString("window.getComputedStyle(testDiv).getPropertyValue('text-shadow')", "rgb(255, 255, 255) -3px -6px 9px")
+shouldBeEqualToString("testDiv.style['text-shadow']", "rgb(255, 255, 255) calc(3px) calc(6px) calc(-9px)");
+shouldBeEqualToString("window.getComputedStyle(testDiv).getPropertyValue('text-shadow')", "rgb(255, 255, 255) 3px 6px 0px")
 
 </script>
 <script src="../../resources/js-test-post.js"></script>
index 429fd02..a124243 100644 (file)
@@ -10,7 +10,7 @@ PASS getCSSText("-webkit-shape-image-threshold", "-0.1") is "-0.1"
 PASS getCSSText("-webkit-shape-image-threshold", "1.1") is "1.1"
 PASS getCSSText("-webkit-shape-image-threshold", "identifier") is ""
 PASS getCSSText("-webkit-shape-image-threshold", "'string'") is ""
-PASS getCSSText("-webkit-shape-image-threshold", "calc(0.1 + 0.4)") is "0.5"
+PASS getCSSText("-webkit-shape-image-threshold", "calc(0.1 + 0.4)") is "calc(0.5)"
 PASS getComputedStyleValue("-webkit-shape-image-threshold", "0") is "0"
 PASS getComputedStyleValue("-webkit-shape-image-threshold", "0.5") is "0.5"
 PASS getComputedStyleValue("-webkit-shape-image-threshold", "1") is "1"
index ad4207e..d5dbbfd 100644 (file)
@@ -20,7 +20,7 @@ applyToEachArglist(
      ["-webkit-shape-image-threshold", "1.1", "1.1"],
      ["-webkit-shape-image-threshold", "identifier", ""],
      ["-webkit-shape-image-threshold", "\'string\'", ""],
-     ["-webkit-shape-image-threshold", "calc(0.1 + 0.4)", "0.5"]
+     ["-webkit-shape-image-threshold", "calc(0.1 + 0.4)", "calc(0.5)"]
     ]
 );
 
index e6d7e1d..e50c4c8 100644 (file)
@@ -1,3 +1,16 @@
+2019-12-02  Simon Fraser  <simon.fraser@apple.com>
+
+        Don't do range checking for calc() at parse time
+        https://bugs.webkit.org/show_bug.cgi?id=204737
+
+        Reviewed by Antti Koivisto.
+
+        * web-platform-tests/css/css-text-decor/parsing/text-shadow-computed-expected.txt:
+        * web-platform-tests/css/css-values/calc-numbers-expected.txt:
+        * web-platform-tests/css/css-values/getComputedStyle-border-radius-001-expected.txt:
+        * web-platform-tests/css/css-values/getComputedStyle-border-radius-003-expected.txt:
+        * web-platform-tests/css/css-values/minmax-number-serialize-expected.txt:
+
 2019-12-01  Antoine Quint  <graouts@apple.com>
 
         [Web Animations] Rebaseline some WPT expectations
index d9a0570..546cf9d 100644 (file)
@@ -3,7 +3,7 @@ PASS Property text-shadow value 'none' computes to 'none'
 PASS Property text-shadow value '10px 20px' computes to 'rgb(0, 0, 255) 10px 20px 0px' 
 PASS Property text-shadow value 'red 10px 20px 30px' computes to 'rgb(255, 0, 0) 10px 20px 30px' 
 PASS Property text-shadow value 'calc(0.5em + 10px) calc(0.5em + 10px) calc(0.5em + 10px)' computes to 'rgb(0, 0, 255) 30px 30px 30px' 
-FAIL Property text-shadow value 'calc(-0.5em + 10px) calc(-0.5em + 10px) calc(-0.5em + 10px)' computes to 'rgb(0, 0, 255) -10px -10px 0px' assert_equals: expected "rgb(0, 0, 255) -10px -10px 0px" but got "rgb(0, 0, 255) -10px -10px -10px"
+PASS Property text-shadow value 'calc(-0.5em + 10px) calc(-0.5em + 10px) calc(-0.5em + 10px)' computes to 'rgb(0, 0, 255) -10px -10px 0px' 
 PASS Property text-shadow value '10px 20px, 30px 40px' computes to 'rgb(0, 0, 255) 10px 20px 0px, rgb(0, 0, 255) 30px 40px 0px' 
 PASS Property text-shadow value 'lime 10px 20px 30px, red 40px 50px' computes to 'rgb(0, 255, 0) 10px 20px 30px, rgb(255, 0, 0) 40px 50px 0px' 
 
index 406c668..a0f138c 100644 (file)
@@ -1,6 +1,6 @@
 
 PASS testing tab-size: calc(2 * 3) 
-FAIL testing tab-size: calc(2 * -4) assert_equals: calc(2 * -4) should compute to 0 expected "0" but got "12345"
+PASS testing tab-size: calc(2 * -4) 
 PASS testing opacity: calc(2 / 4) 
 PASS testing tab-size: calc(2 / 4) 
 FAIL testing opacity: calc(2 / 4) * 1px assert_equals: calc(2 / 4) * 1px should compute to 0.9 expected "0.9" but got "0.8999999761581421"
index f76909c..201b5ab 100644 (file)
@@ -1,7 +1,7 @@
 
-FAIL testing border-top-left-radius: calc(10px + 25%) calc(20px + 25%) assert_equals: expected "calc(25% + 10px) calc(25% + 20px)" but got "10px 20px"
-FAIL testing border-top-right-radius: calc(1em + 25%) assert_equals: expected "calc(25% + 16px)" but got "16px"
+FAIL testing border-top-left-radius: calc(10px + 25%) calc(20px + 25%) assert_equals: expected "calc(25% + 10px) calc(25% + 20px)" but got "calc(10px + 25%) calc(20px + 25%)"
+FAIL testing border-top-right-radius: calc(1em + 25%) assert_equals: expected "calc(25% + 16px)" but got "calc(16px + 25%)"
 PASS testing border-bottom-right-radius: calc(25%) 
 PASS testing border-bottom-left-radius: calc(25px); 
-FAIL testing border-radius shorthand assert_equals: expected "calc(25% + 10px) calc(25% + 16px) 25% 25px / calc(25% + 20px) calc(25% + 16px) 25% 25px" but got "10px 16px 25% 25px / 20px 16px 25% 25px"
+FAIL testing border-radius shorthand assert_equals: expected "calc(25% + 10px) calc(25% + 16px) 25% 25px / calc(25% + 20px) calc(25% + 16px) 25% 25px" but got "calc(10px + 25%) calc(16px + 25%) 25% 25px / calc(20px + 25%) calc(16px + 25%) 25% 25px"
 
index 941a588..b10db50 100644 (file)
@@ -1,3 +1,3 @@
 
-FAIL testing border-radius shorthand assert_equals: expected "calc(1% + 1px) calc(2% + 2px) calc(3% + 3px) calc(4% + 4px) / calc(5% + 5px) calc(6% + 6px) calc(7% + 7px) calc(8% + 8px)" but got "1px 2px 3px 4px / 5px 6px 7px 8px"
+FAIL testing border-radius shorthand assert_equals: expected "calc(1% + 1px) calc(2% + 2px) calc(3% + 3px) calc(4% + 4px) / calc(5% + 5px) calc(6% + 6px) calc(7% + 7px) calc(8% + 8px)" but got "calc(1px + 1%) calc(2px + 2%) calc(3px + 3%) calc(4px + 4%) / calc(5px + 5%) calc(6px + 6%) calc(7px + 7%) calc(8px + 8%)"
 
index 04c470e..a28a755 100644 (file)
@@ -1,14 +1,14 @@
 
-FAIL e.style['opacity'] = "min(1)" should set the property value assert_equals: serialization should be canonical expected "min(1)" but got "1"
-FAIL e.style['opacity'] = "max(1)" should set the property value assert_equals: serialization should be canonical expected "max(1)" but got "1"
-FAIL e.style['opacity'] = "min(1, 2, 3)" should set the property value assert_equals: serialization should be canonical expected "min(1, 2, 3)" but got "1"
-FAIL e.style['opacity'] = "min(3, 2, 1)" should set the property value assert_equals: serialization should be canonical expected "min(3, 2, 1)" but got "1"
-FAIL e.style['opacity'] = "max(1, 2, 3)" should set the property value assert_equals: serialization should be canonical expected "max(1, 2, 3)" but got "3"
-FAIL e.style['opacity'] = "max(3, 2, 1)" should set the property value assert_equals: serialization should be canonical expected "max(3, 2, 1)" but got "3"
-FAIL e.style['opacity'] = "calc(min(1) + min(2))" should set the property value assert_equals: serialization should be canonical expected "calc(min(1) + min(2))" but got "3"
-FAIL e.style['opacity'] = "calc(max(1) + max(2))" should set the property value assert_equals: serialization should be canonical expected "calc(max(1) + max(2))" but got "3"
-FAIL e.style['opacity'] = "calc(1 + min(1))" should set the property value assert_equals: serialization should be canonical expected "calc(1 + min(1))" but got "2"
-FAIL e.style['opacity'] = "calc(min(1) + 1)" should set the property value assert_equals: serialization should be canonical expected "calc(1 + min(1))" but got "2"
-FAIL e.style['opacity'] = "calc(1 + max(1))" should set the property value assert_equals: serialization should be canonical expected "calc(1 + max(1))" but got "2"
-FAIL e.style['opacity'] = "calc(max(1) + 1)" should set the property value assert_equals: serialization should be canonical expected "calc(1 + max(1))" but got "2"
+FAIL e.style['opacity'] = "min(1)" should set the property value assert_equals: serialization should be canonical expected "min(1)" but got "calc(min(1))"
+FAIL e.style['opacity'] = "max(1)" should set the property value assert_equals: serialization should be canonical expected "max(1)" but got "calc(max(1))"
+FAIL e.style['opacity'] = "min(1, 2, 3)" should set the property value assert_equals: serialization should be canonical expected "min(1, 2, 3)" but got "calc(min(1, 2, 3))"
+FAIL e.style['opacity'] = "min(3, 2, 1)" should set the property value assert_equals: serialization should be canonical expected "min(3, 2, 1)" but got "calc(min(3, 2, 1))"
+FAIL e.style['opacity'] = "max(1, 2, 3)" should set the property value assert_equals: serialization should be canonical expected "max(1, 2, 3)" but got "calc(max(1, 2, 3))"
+FAIL e.style['opacity'] = "max(3, 2, 1)" should set the property value assert_equals: serialization should be canonical expected "max(3, 2, 1)" but got "calc(max(3, 2, 1))"
+FAIL e.style['opacity'] = "calc(min(1) + min(2))" should set the property value assert_equals: serialization should be canonical expected "calc(min(1) + min(2))" but got "calc(3)"
+FAIL e.style['opacity'] = "calc(max(1) + max(2))" should set the property value assert_equals: serialization should be canonical expected "calc(max(1) + max(2))" but got "calc(3)"
+FAIL e.style['opacity'] = "calc(1 + min(1))" should set the property value assert_equals: serialization should be canonical expected "calc(1 + min(1))" but got "calc(2)"
+FAIL e.style['opacity'] = "calc(min(1) + 1)" should set the property value assert_equals: serialization should be canonical expected "calc(1 + min(1))" but got "calc(2)"
+FAIL e.style['opacity'] = "calc(1 + max(1))" should set the property value assert_equals: serialization should be canonical expected "calc(1 + max(1))" but got "calc(2)"
+FAIL e.style['opacity'] = "calc(max(1) + 1)" should set the property value assert_equals: serialization should be canonical expected "calc(1 + max(1))" but got "calc(2)"
 
index bee18bc..54b3b6f 100644 (file)
@@ -1,3 +1,67 @@
+2019-12-02  Simon Fraser  <simon.fraser@apple.com>
+
+        Don't do range checking for calc() at parse time
+        https://bugs.webkit.org/show_bug.cgi?id=204737
+
+        Reviewed by Antti Koivisto.
+        
+        As specified in https://drafts.csswg.org/css-values-4/#calc-range, we should not do range checking for calc
+        at parse time; the specified value can be a calc which results in a negative values; values are clamped
+        at used value time (i.e. when building RenderStyle).
+        
+        So CSSCalculationValue doesn't need to have isInt(), isPositive(), isNegative(), and CSSCalcExpressionNode
+        doesn't have to keep track of whether its value is an integer.
+        
+        Also do some cleanup of CSSPrimitiveValue, using categories to answer more isFoo type questions, and adding
+        isZero(), isPositive() and isNegative() that return Optional<bool> for cases where parse-time range checking
+        occurs.
+
+        Tested by existing tests.
+
+        * css/CSSCalculationValue.cpp:
+        (WebCore::CSSCalcOperationNode::createSimplified):
+        (WebCore::CSSCalcExpressionNodeParser::parseValue):
+        (WebCore::createBlendHalf):
+        (WebCore::createCSS):
+        (WebCore::CSSCalcValue::isCalcFunction):
+        (WebCore::isIntegerResult): Deleted.
+        * css/CSSCalculationValue.h:
+        (WebCore::CSSCalcExpressionNode::equals const):
+        (WebCore::CSSCalcExpressionNode::category const):
+        (WebCore::CSSCalcExpressionNode::CSSCalcExpressionNode):
+        (WebCore::CSSCalcExpressionNode::isInteger const): Deleted.
+        * css/CSSComputedStyleDeclaration.cpp:
+        (WebCore::zoomAdjustedPixelValueForLength):
+        (WebCore::percentageOrZoomAdjustedValue):
+        (WebCore::autoOrZoomAdjustedValue):
+        * css/CSSPrimitiveValue.cpp:
+        (WebCore::CSSPrimitiveValue::conversionToCanonicalUnitsScaleFactor): I missed the Q unit here when I added it.
+        (WebCore::CSSPrimitiveValue::isZero const):
+        (WebCore::CSSPrimitiveValue::isPositive const):
+        (WebCore::CSSPrimitiveValue::isNegative const):
+        (WebCore::CSSPrimitiveValue::canonicalUnitTypeForCategory): Deleted.
+        * css/CSSPrimitiveValue.h:
+        (WebCore::CSSPrimitiveValue::isAngle const): Deleted.
+        * css/CSSUnits.cpp:
+        (WebCore::unitCategory):
+        (WebCore::canonicalUnitTypeForCategory):
+        (WebCore::canonicalUnitType):
+        * css/CSSUnits.h:
+        * css/parser/CSSPropertyParser.cpp:
+        (WebCore::consumeColumnWidth):
+        (WebCore::consumePerspective):
+        (WebCore::consumeWebkitAspectRatio):
+        * css/parser/CSSPropertyParserHelpers.cpp:
+        (WebCore::CSSPropertyParserHelpers::CalcParser::CalcParser):
+        (WebCore::CSSPropertyParserHelpers::CalcParser::consumeInteger):
+        (WebCore::CSSPropertyParserHelpers::consumeNumber):
+        (WebCore::CSSPropertyParserHelpers::consumeSingleShadow): This parsing is a little tricky. The blur radius value is optional,
+        but we need to distinguish between failing to parse it, and its value being negative, so an explicit check for calc is the easiest way
+        to handlel that.
+        (WebCore::CSSPropertyParserHelpers::CalcParser::consumePositiveIntegerRaw): Deleted. It was unused.
+        (WebCore::CSSPropertyParserHelpers::consumePositiveIntegerRaw): Deleted.
+        * css/parser/CSSPropertyParserHelpers.h:
+
 2019-12-02  Rob Buis  <rbuis@igalia.com>
 
         [GTK] Fix some warnings
index b3048bf..f99289d 100644 (file)
@@ -180,16 +180,16 @@ static String prettyPrintNodes(const Vector<Ref<CSSCalcExpressionNode>>& nodes)
 class CSSCalcPrimitiveValueNode final : public CSSCalcExpressionNode {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    static Ref<CSSCalcPrimitiveValueNode> create(Ref<CSSPrimitiveValue>&& value, bool isInteger)
+    static Ref<CSSCalcPrimitiveValueNode> create(Ref<CSSPrimitiveValue>&& value)
     {
-        return adoptRef(*new CSSCalcPrimitiveValueNode(WTFMove(value), isInteger));
+        return adoptRef(*new CSSCalcPrimitiveValueNode(WTFMove(value)));
     }
 
-    static RefPtr<CSSCalcPrimitiveValueNode> create(double value, CSSUnitType type, bool isInteger)
+    static RefPtr<CSSCalcPrimitiveValueNode> create(double value, CSSUnitType type)
     {
         if (!std::isfinite(value))
             return nullptr;
-        return adoptRef(new CSSCalcPrimitiveValueNode(CSSPrimitiveValue::create(value, type), isInteger));
+        return adoptRef(new CSSCalcPrimitiveValueNode(CSSPrimitiveValue::create(value, type)));
     }
 
 private:
@@ -221,8 +221,8 @@ private:
     void dump(TextStream&) const final;
 
 private:
-    explicit CSSCalcPrimitiveValueNode(Ref<CSSPrimitiveValue>&& value, bool isInteger)
-        : CSSCalcExpressionNode(calcUnitCategory(value->primitiveType()), isInteger)
+    explicit CSSCalcPrimitiveValueNode(Ref<CSSPrimitiveValue>&& value)
+        : CSSCalcExpressionNode(calcUnitCategory(value->primitiveType()))
         , m_value(WTFMove(value))
     {
     }
@@ -373,28 +373,6 @@ static CalculationCategory resolvedTypeForMinOrMax(CalculationCategory category,
     return CalculationCategory::Other;
 }
 
-static inline bool isIntegerResult(CalcOperator op, const CSSCalcExpressionNode& leftSide, const CSSCalcExpressionNode& rightSide)
-{
-    // Performs W3C spec's type checking for calc integers.
-    // http://www.w3.org/TR/css3-values/#calc-type-checking
-    return op != CalcOperator::Divide && leftSide.isInteger() && rightSide.isInteger();
-}
-
-static inline bool isIntegerResult(CalcOperator op, const Vector<Ref<CSSCalcExpressionNode>>& nodes)
-{
-    // Performs W3C spec's type checking for calc integers.
-    // http://www.w3.org/TR/css3-values/#calc-type-checking
-    if (op == CalcOperator::Divide)
-        return false;
-
-    for (auto& node : nodes) {
-        if (!node->isInteger())
-            return false;
-    }
-
-    return true;
-}
-
 static bool isSamePair(CalculationCategory a, CalculationCategory b, CalculationCategory x, CalculationCategory y)
 {
     return (a == x && b == y) || (a == y && b == x);
@@ -409,7 +387,7 @@ public:
 
 private:
     CSSCalcOperationNode(CalculationCategory category, CalcOperator op, Ref<CSSCalcExpressionNode>&& leftSide, Ref<CSSCalcExpressionNode>&& rightSide)
-        : CSSCalcExpressionNode(category, isIntegerResult(op, leftSide.get(), rightSide.get()))
+        : CSSCalcExpressionNode(category)
         , m_operator(op)
     {
         m_children.reserveInitialCapacity(2);
@@ -418,7 +396,7 @@ private:
     }
 
     CSSCalcOperationNode(CalculationCategory category, CalcOperator op, Vector<Ref<CSSCalcExpressionNode>>&& children)
-        : CSSCalcExpressionNode(category, isIntegerResult(op, children))
+        : CSSCalcExpressionNode(category)
         , m_operator(op)
         , m_children(WTFMove(children))
     {
@@ -527,12 +505,10 @@ RefPtr<CSSCalcExpressionNode> CSSCalcOperationNode::createSimplified(CalcOperato
     ASSERT(leftCategory < CalculationCategory::Other);
     ASSERT(rightCategory < CalculationCategory::Other);
 
-    bool isInteger = isIntegerResult(op, *leftSide, *rightSide);
-
     // Simplify numbers.
     if (leftCategory == CalculationCategory::Number && rightCategory == CalculationCategory::Number) {
         CSSUnitType evaluationType = CSSUnitType::CSS_NUMBER;
-        return CSSCalcPrimitiveValueNode::create(evaluateOperator(op, { leftSide->doubleValue(), rightSide->doubleValue() }), evaluationType, isInteger);
+        return CSSCalcPrimitiveValueNode::create(evaluateOperator(op, { leftSide->doubleValue(), rightSide->doubleValue() }), evaluationType);
     }
 
     // Simplify addition and subtraction between same types.
@@ -542,14 +518,14 @@ RefPtr<CSSCalcExpressionNode> CSSCalcOperationNode::createSimplified(CalcOperato
             if (hasDoubleValue(leftType)) {
                 CSSUnitType rightType = rightSide->primitiveType();
                 if (leftType == rightType)
-                    return CSSCalcPrimitiveValueNode::create(evaluateOperator(op, { leftSide->doubleValue(), rightSide->doubleValue() }), leftType, isInteger);
+                    return CSSCalcPrimitiveValueNode::create(evaluateOperator(op, { leftSide->doubleValue(), rightSide->doubleValue() }), leftType);
                 CSSUnitCategory leftUnitCategory = unitCategory(leftType);
                 if (leftUnitCategory != CSSUnitCategory::Other && leftUnitCategory == unitCategory(rightType)) {
-                    CSSUnitType canonicalType = CSSPrimitiveValue::canonicalUnitTypeForCategory(leftUnitCategory);
+                    CSSUnitType canonicalType = canonicalUnitTypeForCategory(leftUnitCategory);
                     if (canonicalType != CSSUnitType::CSS_UNKNOWN) {
                         double leftValue = leftSide->doubleValue() * CSSPrimitiveValue::conversionToCanonicalUnitsScaleFactor(leftType);
                         double rightValue = rightSide->doubleValue() * CSSPrimitiveValue::conversionToCanonicalUnitsScaleFactor(rightType);
-                        return CSSCalcPrimitiveValueNode::create(evaluateOperator(op, { leftValue, rightValue }), canonicalType, isInteger);
+                        return CSSCalcPrimitiveValueNode::create(evaluateOperator(op, { leftValue, rightValue }), canonicalType);
                     }
                 }
             }
@@ -572,7 +548,7 @@ RefPtr<CSSCalcExpressionNode> CSSCalcOperationNode::createSimplified(CalcOperato
 
         auto otherType = otherSide.primitiveType();
         if (hasDoubleValue(otherType))
-            return CSSCalcPrimitiveValueNode::create(evaluateOperator(op, { otherSide.doubleValue(), number }), otherType, isInteger);
+            return CSSCalcPrimitiveValueNode::create(evaluateOperator(op, { otherSide.doubleValue(), number }), otherType);
     }
 
     return create(op, leftSide.releaseNonNull(), rightSide.releaseNonNull());
@@ -827,8 +803,7 @@ bool CSSCalcExpressionNodeParser::parseValue(CSSParserTokenRange& tokens, Value*
     if (calcUnitCategory(type) == CalculationCategory::Other)
         return false;
     
-    bool isInteger = token.numericValueType() == IntegerValueType || (token.numericValueType() == NumberValueType && token.numericValue() == trunc(token.numericValue()));
-    result->value = CSSCalcPrimitiveValueNode::create(CSSPrimitiveValue::create(token.numericValue(), type), isInteger);
+    result->value = CSSCalcPrimitiveValueNode::create(CSSPrimitiveValue::create(token.numericValue(), type));
     
     return true;
 }
@@ -970,7 +945,7 @@ bool CSSCalcExpressionNodeParser::parseValueExpression(CSSParserTokenRange& toke
 static inline RefPtr<CSSCalcOperationNode> createBlendHalf(const Length& length, const RenderStyle& style, float progress)
 {
     return CSSCalcOperationNode::create(CalcOperator::Multiply, createCSS(length, style),
-        CSSCalcPrimitiveValueNode::create(CSSPrimitiveValue::create(progress, CSSUnitType::CSS_NUMBER), !progress || progress == 1));
+        CSSCalcPrimitiveValueNode::create(CSSPrimitiveValue::create(progress, CSSUnitType::CSS_NUMBER)));
 }
 
 static RefPtr<CSSCalcExpressionNode> createCSS(const CalcExpressionNode& node, const RenderStyle& style)
@@ -978,7 +953,7 @@ static RefPtr<CSSCalcExpressionNode> createCSS(const CalcExpressionNode& node, c
     switch (node.type()) {
     case CalcExpressionNodeType::Number: {
         float value = downcast<CalcExpressionNumber>(node).value();
-        return CSSCalcPrimitiveValueNode::create(CSSPrimitiveValue::create(value, CSSUnitType::CSS_NUMBER), value == std::trunc(value));
+        return CSSCalcPrimitiveValueNode::create(CSSPrimitiveValue::create(value, CSSUnitType::CSS_NUMBER));
     }
     case CalcExpressionNodeType::Length:
         return createCSS(downcast<CalcExpressionLength>(node).length(), style);
@@ -1020,7 +995,7 @@ static RefPtr<CSSCalcExpressionNode> createCSS(const Length& length, const Rende
     switch (length.type()) {
     case Percent:
     case Fixed:
-        return CSSCalcPrimitiveValueNode::create(CSSPrimitiveValue::create(length, style), length.value() == trunc(length.value()));
+        return CSSCalcPrimitiveValueNode::create(CSSPrimitiveValue::create(length, style));
     case Calculated:
         return createCSS(length.calculationValue().expression(), style);
     case Auto:
@@ -1065,6 +1040,20 @@ double CSSCalcValue::computeLengthPx(const CSSToLengthConversionData& conversion
     return clampToPermittedRange(m_expression->computeLengthPx(conversionData));
 }
 
+bool CSSCalcValue::isCalcFunction(CSSValueID functionId)
+{
+    switch (functionId) {
+    case CSSValueCalc:
+    case CSSValueWebkitCalc:
+    case CSSValueMin:
+    case CSSValueMax:
+        return true;
+    default:
+        return false;
+    }
+    return false;
+}
+
 void CSSCalcValue::dump(TextStream& ts) const
 {
     ts << indent << "(" << "CSSCalcValue";
index 5287bd8..7d09f7e 100644 (file)
@@ -70,7 +70,7 @@ public:
     virtual double doubleValue() const = 0;
     virtual double computeLengthPx(const CSSToLengthConversionData&) const = 0;
     virtual String customCSSText() const = 0;
-    virtual bool equals(const CSSCalcExpressionNode& other) const { return m_category == other.m_category && m_isInteger == other.m_isInteger; }
+    virtual bool equals(const CSSCalcExpressionNode& other) const { return m_category == other.m_category; }
     virtual Type type() const = 0;
     virtual CSSUnitType primitiveType() const = 0;
 
@@ -78,20 +78,17 @@ public:
     virtual void collectDirectRootComputationalDependencies(HashSet<CSSPropertyID>&) const = 0;
 
     CalculationCategory category() const { return m_category; }
-    bool isInteger() const { return m_isInteger; }
 
     virtual void dump(TextStream&) const = 0;
 
 protected:
-    CSSCalcExpressionNode(CalculationCategory category, bool isInteger)
+    CSSCalcExpressionNode(CalculationCategory category)
         : m_category(category)
-        , m_isInteger(isInteger)
     {
     }
 
 private:
     CalculationCategory m_category;
-    bool m_isInteger;
 };
 
 class CSSCalcValue final : public CSSValue {
@@ -101,10 +98,7 @@ public:
     static RefPtr<CSSCalcValue> create(const CalculationValue&, const RenderStyle&);
 
     CalculationCategory category() const { return m_expression->category(); }
-    bool isInt() const { return m_expression->isInteger(); }
     double doubleValue() const;
-    bool isPositive() const { return m_expression->doubleValue() > 0; }
-    bool isNegative() const { return m_expression->doubleValue() < 0; }
     double computeLengthPx(const CSSToLengthConversionData&) const;
     CSSUnitType primitiveType() const { return m_expression->primitiveType(); }
 
index 73596ee..1c4e36f 100644 (file)
@@ -257,7 +257,7 @@ inline static Ref<CSSPrimitiveValue> zoomAdjustedNumberValue(double value, const
     return CSSValuePool::singleton().createValue(value / style.effectiveZoom(), CSSUnitType::CSS_NUMBER);
 }
 
-static Ref<CSSValue> zoomAdjustedPixelValueForLength(const Length& length, const RenderStyle& style)
+static Ref<CSSPrimitiveValue> zoomAdjustedPixelValueForLength(const Length& length, const RenderStyle& style)
 {
     if (length.isFixed())
         return zoomAdjustedPixelValue(length.value(), style);
@@ -454,7 +454,7 @@ static Ref<CSSPrimitiveValue> percentageOrZoomAdjustedValue(Length length, const
     if (length.isPercent())
         return CSSValuePool::singleton().createValue(length.percent(), CSSUnitType::CSS_PERCENTAGE);
     
-    return zoomAdjustedPixelValue(valueForLength(length, 0), style);
+    return zoomAdjustedPixelValueForLength(length, style);
 }
 
 static Ref<CSSPrimitiveValue> autoOrZoomAdjustedValue(Length length, const RenderStyle& style)
@@ -462,7 +462,7 @@ static Ref<CSSPrimitiveValue> autoOrZoomAdjustedValue(Length length, const Rende
     if (length.isAuto())
         return CSSValuePool::singleton().createIdentifierValue(CSSValueAuto);
 
-    return zoomAdjustedPixelValue(valueForLength(length, 0), style);
+    return zoomAdjustedPixelValueForLength(length, style);
 }
 
 static Ref<CSSValueList> borderRadiusCornerValues(const LengthSize& radius, const RenderStyle& style)
index 7cacbb3..b328b70 100644 (file)
@@ -714,6 +714,9 @@ double CSSPrimitiveValue::conversionToCanonicalUnitsScaleFactor(CSSUnitType unit
     case CSSUnitType::CSS_MM:
         factor = cssPixelsPerInch / mmPerInch;
         break;
+    case CSSUnitType::CSS_Q:
+        factor = cssPixelsPerInch / QPerInch;
+        break;
     case CSSUnitType::CSS_IN:
         factor = cssPixelsPerInch;
         break;
@@ -764,31 +767,25 @@ double CSSPrimitiveValue::doubleValue() const
     return primitiveUnitType() != CSSUnitType::CSS_CALC ? m_value.num : m_value.calc->doubleValue();
 }
 
-CSSUnitType CSSPrimitiveValue::canonicalUnitTypeForCategory(CSSUnitCategory category)
+Optional<bool> CSSPrimitiveValue::isZero() const
 {
-    // The canonical unit type is chosen according to the way CSSParser::validUnit() chooses the default unit
-    // in each category (based on unitflags).
-    // Canonical units are specified in <https://drafts.csswg.org/css-values-4>.
-    switch (category) {
-    case CSSUnitCategory::Number:
-        return CSSUnitType::CSS_NUMBER;
-    case CSSUnitCategory::Length:
-        return CSSUnitType::CSS_PX;
-    case CSSUnitCategory::Percent:
-        return CSSUnitType::CSS_UNKNOWN; // Cannot convert between numbers and percent.
-    case CSSUnitCategory::Time:
-        return CSSUnitType::CSS_MS;
-    case CSSUnitCategory::Angle:
-        return CSSUnitType::CSS_DEG;
-    case CSSUnitCategory::Frequency:
-        return CSSUnitType::CSS_HZ;
-#if ENABLE(CSS_IMAGE_RESOLUTION) || ENABLE(RESOLUTION_MEDIA_QUERY)
-    case CSSUnitCategory::Resolution:
-        return CSSUnitType::CSS_DPPX;
-#endif
-    default:
-        return CSSUnitType::CSS_UNKNOWN;
-    }
+    if (primitiveUnitType() == CSSUnitType::CSS_CALC)
+        return WTF::nullopt;
+    return !m_value.num;
+}
+
+Optional<bool> CSSPrimitiveValue::isPositive() const
+{
+    if (primitiveUnitType() == CSSUnitType::CSS_CALC)
+        return WTF::nullopt;
+    return m_value.num > 0;
+}
+
+Optional<bool> CSSPrimitiveValue::isNegative() const
+{
+    if (primitiveUnitType() == CSSUnitType::CSS_CALC)
+        return WTF::nullopt;
+    return m_value.num < 0;
 }
 
 Optional<double> CSSPrimitiveValue::doubleValueInternal(CSSUnitType requestedUnitType) const
index 68cbb18..2241b70 100644 (file)
@@ -81,7 +81,8 @@ public:
     static bool isResolution(CSSUnitType);
     static bool isViewportPercentageLength(CSSUnitType type) { return type >= CSSUnitType::CSS_VW && type <= CSSUnitType::CSS_VMAX; }
 
-    bool isAngle() const;
+    // FIXME: Some of these use primitiveUnitType() and some use primitiveType(). Those that use primitiveUnitType() are broken with calc().
+    bool isAngle() const { return unitCategory(primitiveType()) == CSSUnitCategory::Angle; }
     bool isAttr() const { return primitiveUnitType() == CSSUnitType::CSS_ATTR; }
     bool isCounter() const { return primitiveUnitType() == CSSUnitType::CSS_COUNTER; }
     bool isFontIndependentLength() const { return primitiveUnitType() >= CSSUnitType::CSS_PX && primitiveUnitType() <= CSSUnitType::CSS_PC; }
@@ -98,7 +99,8 @@ public:
     bool isShape() const { return primitiveUnitType() == CSSUnitType::CSS_SHAPE; }
     bool isString() const { return primitiveUnitType() == CSSUnitType::CSS_STRING; }
     bool isFontFamily() const { return primitiveUnitType() == CSSUnitType::CSS_FONT_FAMILY; }
-    bool isTime() const { return primitiveUnitType() == CSSUnitType::CSS_S || primitiveUnitType() == CSSUnitType::CSS_MS; }
+    bool isTime() const { return unitCategory(primitiveUnitType()) == CSSUnitCategory::Time; }
+    bool isFrequency() const { return unitCategory(primitiveType()) == CSSUnitCategory::Frequency; }
     bool isURI() const { return primitiveUnitType() == CSSUnitType::CSS_URI; }
     bool isCalculated() const { return primitiveUnitType() == CSSUnitType::CSS_CALC; }
     bool isCalculatedPercentageWithNumber() const { return primitiveType() == CSSUnitType::CSS_CALC_PERCENTAGE_WITH_NUMBER; }
@@ -106,7 +108,7 @@ public:
     bool isDotsPerInch() const { return primitiveType() == CSSUnitType::CSS_DPI; }
     bool isDotsPerPixel() const { return primitiveType() == CSSUnitType::CSS_DPPX; }
     bool isDotsPerCentimeter() const { return primitiveType() == CSSUnitType::CSS_DPCM; }
-    bool isResolution() const { return isResolution(primitiveType()); }
+    bool isResolution() const { return unitCategory(primitiveType()) == CSSUnitCategory::Resolution; }
     bool isViewportPercentageLength() const { return isViewportPercentageLength(primitiveUnitType()); }
     bool isViewportPercentageWidth() const { return primitiveUnitType() == CSSUnitType::CSS_VW; }
     bool isViewportPercentageHeight() const { return primitiveUnitType() == CSSUnitType::CSS_VH; }
@@ -155,8 +157,14 @@ public:
     bool convertingToLengthRequiresNonNullStyle(int lengthConversion) const;
 
     double doubleValue(CSSUnitType) const;
+    // It's usually wrong to call this; it can trigger type conversion in calc without sufficient context to resolve relative length units.
     double doubleValue() const;
 
+    // These return nullopt for calc, for which range checking is not done at parse time: <https://www.w3.org/TR/css3-values/#calc-range>.
+    Optional<bool> isZero() const;
+    Optional<bool> isPositive() const;
+    Optional<bool> isNegative() const;
+
     template<typename T> inline T value(CSSUnitType type) const { return clampTo<T>(doubleValue(type)); }
     template<typename T> inline T value() const { return clampTo<T>(doubleValue()); }
 
@@ -188,7 +196,6 @@ public:
 
     bool equals(const CSSPrimitiveValue&) const;
 
-    static CSSUnitType canonicalUnitTypeForCategory(CSSUnitCategory);
     static double conversionToCanonicalUnitsScaleFactor(CSSUnitType);
 
     static double computeNonCalcLengthDouble(const CSSToLengthConversionData&, CSSUnitType, double value);
@@ -256,15 +263,6 @@ private:
     } m_value;
 };
 
-inline bool CSSPrimitiveValue::isAngle() const
-{
-    auto primitiveType = this->primitiveType();
-    return primitiveType == CSSUnitType::CSS_DEG
-        || primitiveType == CSSUnitType::CSS_RAD
-        || primitiveType == CSSUnitType::CSS_GRAD
-        || primitiveType == CSSUnitType::CSS_TURN;
-}
-
 inline bool CSSPrimitiveValue::isFontRelativeLength(CSSUnitType type)
 {
     return type == CSSUnitType::CSS_EMS
index 890b33f..4b7474d 100644 (file)
@@ -49,15 +49,42 @@ CSSUnitCategory unitCategory(CSSUnitType type)
     case CSSUnitType::CSS_HZ:
     case CSSUnitType::CSS_KHZ:
         return CSSUnitCategory::Frequency;
-#if ENABLE(CSS_IMAGE_RESOLUTION) || ENABLE(RESOLUTION_MEDIA_QUERY)
     case CSSUnitType::CSS_DPPX:
     case CSSUnitType::CSS_DPI:
     case CSSUnitType::CSS_DPCM:
         return CSSUnitCategory::Resolution;
-#endif
     default:
         return CSSUnitCategory::Other;
     }
 }
 
+CSSUnitType canonicalUnitTypeForCategory(CSSUnitCategory category)
+{
+    switch (category) {
+    case CSSUnitCategory::Number:
+        return CSSUnitType::CSS_NUMBER;
+    case CSSUnitCategory::Length:
+        return CSSUnitType::CSS_PX;
+    case CSSUnitCategory::Percent:
+        return CSSUnitType::CSS_UNKNOWN; // Cannot convert between numbers and percent.
+    case CSSUnitCategory::Time:
+        return CSSUnitType::CSS_MS;
+    case CSSUnitCategory::Angle:
+        return CSSUnitType::CSS_DEG;
+    case CSSUnitCategory::Frequency:
+        return CSSUnitType::CSS_HZ;
+#if ENABLE(CSS_IMAGE_RESOLUTION) || ENABLE(RESOLUTION_MEDIA_QUERY)
+    case CSSUnitCategory::Resolution:
+        return CSSUnitType::CSS_DPPX;
+#endif
+    default:
+        return CSSUnitType::CSS_UNKNOWN;
+    }
+}
+
+CSSUnitType canonicalUnitType(CSSUnitType unitType)
+{
+    return canonicalUnitTypeForCategory(unitCategory(unitType));
+}
+
 } // namespace WebCore
index a518bca..9a93bab 100644 (file)
@@ -98,13 +98,13 @@ enum class CSSUnitCategory : uint8_t {
     Angle,
     Time,
     Frequency,
-#if ENABLE(CSS_IMAGE_RESOLUTION) || ENABLE(RESOLUTION_MEDIA_QUERY)
     Resolution,
-#endif
     Other
 };
 
 CSSUnitCategory unitCategory(CSSUnitType);
+CSSUnitType canonicalUnitTypeForCategory(CSSUnitCategory);
+CSSUnitType canonicalUnitType(CSSUnitType);
 
 } // namespace WebCore
 
index e7a2b44..ebd0eaf 100644 (file)
@@ -1380,8 +1380,9 @@ static RefPtr<CSSValue> consumeColumnWidth(CSSParserTokenRange& range)
     // Always parse lengths in strict mode here, since it would be ambiguous otherwise when used in
     // the 'columns' shorthand property.
     RefPtr<CSSPrimitiveValue> columnWidth = consumeLength(range, HTMLStandardMode, ValueRangeNonNegative);
-    if (!columnWidth || (!columnWidth->isCalculated() && !columnWidth->doubleValue()) || (columnWidth->cssCalcValue() && !columnWidth->cssCalcValue()->doubleValue()))
+    if (!columnWidth || columnWidth->isZero().valueOr(false))
         return nullptr;
+
     return columnWidth;
 }
 
@@ -2280,8 +2281,13 @@ static RefPtr<CSSPrimitiveValue> consumePerspective(CSSParserTokenRange& range,
             return nullptr;
         parsedValue = CSSPrimitiveValue::create(perspective, CSSUnitType::CSS_PX);
     }
-    if (parsedValue && (parsedValue->isCalculated() || parsedValue->doubleValue() > 0))
+
+    if (!parsedValue)
+        return nullptr;
+
+    if (parsedValue->isPositive().valueOr(true))
         return parsedValue;
+
     return nullptr;
 }
 
@@ -3687,12 +3693,13 @@ static RefPtr<CSSValue> consumeWebkitAspectRatio(CSSParserTokenRange& range)
         return consumeIdent<CSSValueAuto, CSSValueFromDimensions, CSSValueFromIntrinsic>(range);
     
     RefPtr<CSSPrimitiveValue> leftValue = consumeNumber(range, ValueRangeNonNegative);
-    if (!leftValue || !leftValue->floatValue() || range.atEnd() || !consumeSlashIncludingWhitespace(range))
+    if (!leftValue || leftValue->isZero().valueOr(false) || range.atEnd() || !consumeSlashIncludingWhitespace(range))
         return nullptr;
+
     RefPtr<CSSPrimitiveValue> rightValue = consumeNumber(range, ValueRangeNonNegative);
-    if (!rightValue || !rightValue->floatValue())
+    if (!rightValue || rightValue->isZero().valueOr(false))
         return nullptr;
-    
+
     return CSSAspectRatioValue::create(leftValue->floatValue(), rightValue->floatValue());
 }
 
index 21a2b30..1e0922b 100644 (file)
@@ -86,7 +86,7 @@ public:
     {
         const CSSParserToken& token = range.peek();
         auto functionId = token.functionId();
-        if (functionId == CSSValueCalc || functionId == CSSValueWebkitCalc || functionId == CSSValueMin || functionId == CSSValueMax)
+        if (CSSCalcValue::isCalcFunction(functionId))
             m_calcValue = CSSCalcValue::create(functionId, consumeFunction(m_range), destinationCategory, valueRange);
     }
 
@@ -105,7 +105,6 @@ public:
         if (!m_calcValue)
             return nullptr;
         m_sourceRange = m_range;
-
         double value = std::max(m_calcValue->doubleValue(), minimumValue);
         value = std::round(value);
         return CSSValuePool::singleton().createValue(value, CSSUnitType::CSS_NUMBER);
@@ -128,17 +127,6 @@ public:
         return true;
     }
     
-    bool consumePositiveIntegerRaw(int& result)
-    {
-        if (!m_calcValue || m_calcValue->category() != CalculationCategory::Number || !m_calcValue->isInt())
-            return false;
-        result = static_cast<int>(m_calcValue->doubleValue());
-        if (result < 1)
-            return false;
-        m_sourceRange = m_range;
-        return true;
-    }
-
 private:
     CSSParserTokenRange& m_sourceRange;
     CSSParserTokenRange m_range;
@@ -172,23 +160,6 @@ RefPtr<CSSPrimitiveValue> consumePositiveInteger(CSSParserTokenRange& range)
     return consumeInteger(range, 1);
 }
 
-bool consumePositiveIntegerRaw(CSSParserTokenRange& range, int& result)
-{
-    const CSSParserToken& token = range.peek();
-    if (token.type() == NumberToken) {
-        if (token.numericValueType() == NumberValueType || token.numericValue() < 1)
-            return false;
-        result = range.consumeIncludingWhitespace().numericValue();
-        return true;
-    }
-
-    if (token.type() != FunctionToken)
-        return false;
-
-    CalcParser calcParser(range, CalculationCategory::Number);
-    return calcParser.consumePositiveIntegerRaw(result);
-}
-    
 bool consumeNumberRaw(CSSParserTokenRange& range, double& result)
 {
     const CSSParserToken& token = range.peek();
@@ -217,13 +188,11 @@ RefPtr<CSSPrimitiveValue> consumeNumber(CSSParserTokenRange& range, ValueRange v
     if (token.type() != FunctionToken)
         return nullptr;
 
-    CalcParser calcParser(range, CalculationCategory::Number, ValueRangeAll);
-    if (const CSSCalcValue* calculation = calcParser.value()) {
-        // FIXME: Calcs should not be subject to parse time range checks.
-        // spec: https://drafts.csswg.org/css-values-3/#calc-range
-        if (calculation->category() != CalculationCategory::Number || (valueRange == ValueRangeNonNegative && calculation->isNegative()))
+    CalcParser calcParser(range, CalculationCategory::Number, valueRange);
+    if (const CSSCalcValue* calcValue = calcParser.value()) {
+        if (calcValue->category() != CalculationCategory::Number)
             return nullptr;
-        return calcParser.consumeNumber();
+        return calcParser.consumeValue();
     }
 
     return nullptr;
@@ -1708,16 +1677,20 @@ RefPtr<CSSShadowValue> consumeSingleShadow(CSSParserTokenRange& range, CSSParser
     if (!verticalOffset)
         return nullptr;
 
-    auto blurRadius = consumeLength(range, cssParserMode, ValueRangeAll);
+    RefPtr<CSSPrimitiveValue> blurRadius;
     RefPtr<CSSPrimitiveValue> spreadDistance;
-    if (blurRadius) {
-        // Blur radius must be non-negative.
-        if (blurRadius->doubleValue() < 0)
+
+    const CSSParserToken& token = range.peek();
+    // The explicit check for calc() is unfortunate. This is ensuring that we only fail parsing if there is a length, but it fails the range check.
+    if (token.type() == DimensionToken || token.type() == NumberToken || (token.type() == FunctionToken && CSSCalcValue::isCalcFunction(token.functionId()))) {
+        blurRadius = consumeLength(range, cssParserMode, ValueRangeNonNegative);
+        if (!blurRadius)
             return nullptr;
-        if (allowSpread)
-            spreadDistance = consumeLength(range, cssParserMode, ValueRangeAll);
     }
 
+    if (blurRadius && allowSpread)
+        spreadDistance = consumeLength(range, cssParserMode, ValueRangeAll);
+
     if (!range.atEnd()) {
         if (!color)
             color = consumeColor(range, cssParserMode);
index 76745a8..156ee88 100644 (file)
@@ -57,7 +57,6 @@ enum class UnitlessQuirk {
 };
 
 RefPtr<CSSPrimitiveValue> consumeInteger(CSSParserTokenRange&, double minimumValue = -std::numeric_limits<double>::max());
-bool consumePositiveIntegerRaw(CSSParserTokenRange&, int& result);
 RefPtr<CSSPrimitiveValue> consumePositiveInteger(CSSParserTokenRange&);
 bool consumeNumberRaw(CSSParserTokenRange&, double& result);
 RefPtr<CSSPrimitiveValue> consumeNumber(CSSParserTokenRange&, ValueRange);