- Revision
- 183765
- Author
- [email protected]
- Date
- 2015-05-04 13:27:50 -0700 (Mon, 04 May 2015)
Log Message
REGRESSION (r178156): CSS Parser incorrectly rejects valid calc() in padding-right property
https://bugs.webkit.org/show_bug.cgi?id=144584
<rdar://problem/20796829>
Reviewed by Darin Adler.
Source/WebCore:
The CSS parser was rejecting calculated values at parsing time if it
considered the value was negative and the CSS property did not allow
negative values. However, doing so at this point will not always work
because we don't necessarily know the font-size yet (for e.g. for
calc(0.5em - 2px). Also, rejecting negative calculated values is not
the right behavior as the the specification. The specification says
we should clamp:
http://dev.w3.org/csswg/css-values-3/#calc-range
This patch updates validateCalculationUnit() to stop marking the value
as invalid if it is negative. Instead, let the CSSCalcValue's permitted
range clamp the value as needed.
This bug was causing the bottom graphic on aldentrio.com to not be
rendered properly.
Test: fast/css/negative-calc-values.html
fast/css/padding-calc-value.html
* css/CSSParser.cpp:
(WebCore::CSSParser::validateCalculationUnit):
LayoutTests:
* fast/css/negative-calc-values-expected.txt: Added.
* fast/css/negative-calc-values.html: Added.
Add a layout test that assigns negative calc() values to properties
whose values cannot be negative to verify that values are clamped as
per the specification:
http://dev.w3.org/csswg/css-values-3/#calc-range
* fast/css/padding-calc-value-expected.txt: Added.
* fast/css/padding-calc-value.html: Added.
Add a layout test to test that using calc(.5em - 2px) for padding-right
CSS property works as intended. It used to be resolved as 0px instead
of "2*font-size - 2px".
* fast/css/text-shadow-calc-value-expected.txt:
* fast/css/text-shadow-calc-value.html:
Update test to match what the specification says:
http://dev.w3.org/csswg/css-values-3/#calc-range
"width: calc(5px - 10px);" is equivalent to "width: 0px;" since widths
smaller than 0px are not allowed.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (183764 => 183765)
--- trunk/LayoutTests/ChangeLog 2015-05-04 20:22:55 UTC (rev 183764)
+++ trunk/LayoutTests/ChangeLog 2015-05-04 20:27:50 UTC (rev 183765)
@@ -1,3 +1,31 @@
+2015-05-04 Chris Dumez <[email protected]>
+
+ REGRESSION (r178156): CSS Parser incorrectly rejects valid calc() in padding-right property
+ https://bugs.webkit.org/show_bug.cgi?id=144584
+ <rdar://problem/20796829>
+
+ Reviewed by Darin Adler.
+
+ * fast/css/negative-calc-values-expected.txt: Added.
+ * fast/css/negative-calc-values.html: Added.
+ Add a layout test that assigns negative calc() values to properties
+ whose values cannot be negative to verify that values are clamped as
+ per the specification:
+ http://dev.w3.org/csswg/css-values-3/#calc-range
+
+ * fast/css/padding-calc-value-expected.txt: Added.
+ * fast/css/padding-calc-value.html: Added.
+ Add a layout test to test that using calc(.5em - 2px) for padding-right
+ CSS property works as intended. It used to be resolved as 0px instead
+ of "2*font-size - 2px".
+
+ * fast/css/text-shadow-calc-value-expected.txt:
+ * fast/css/text-shadow-calc-value.html:
+ Update test to match what the specification says:
+ http://dev.w3.org/csswg/css-values-3/#calc-range
+ "width: calc(5px - 10px);" is equivalent to "width: 0px;" since widths
+ smaller than 0px are not allowed.
+
2015-05-04 Joseph Pecoraro <[email protected]>
Unreviewed gardening. Fix lint error on mac-wk1.
Added: trunk/LayoutTests/fast/css/negative-calc-values-expected.txt (0 => 183765)
--- trunk/LayoutTests/fast/css/negative-calc-values-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css/negative-calc-values-expected.txt 2015-05-04 20:27:50 UTC (rev 183765)
@@ -0,0 +1,29 @@
+Tests that negative calc() values are properly clamped when needed.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS testDiv.style['width'] is ""
+testDiv.style['width'] = '100px'
+testDiv.style['width'] = 'calc(5px - 10px)'
+PASS testDiv.style['width'] is "calc(-5px)"
+PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "0px"
+testDiv.style['width'] = '100px'
+testDiv.style['width'] = 'calc(10% - 100px)'
+PASS testDiv.style['width'] is "calc(10% - 100px)"
+PASS window.getComputedStyle(testDiv).getPropertyValue('width') is "0px"
+testDiv.style['line-height'] = '10%'
+testDiv.style['line-height'] = 'calc(10% - 20%)'
+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 "calc(-2)"
+PASS window.getComputedStyle(testPre).getPropertyValue('tab-size') is "0"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
+
+
+
Added: trunk/LayoutTests/fast/css/negative-calc-values.html (0 => 183765)
--- trunk/LayoutTests/fast/css/negative-calc-values.html (rev 0)
+++ trunk/LayoutTests/fast/css/negative-calc-values.html 2015-05-04 20:27:50 UTC (rev 183765)
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<body>
+<script src=""
+<div id="testDiv" style="position: absolute;"></div>
+<pre id="testPre"><pre>
+<script>
+description("Tests that negative calc() values are properly clamped when needed.");
+
+var testDiv = document.getElementById("testDiv");
+var testPre = document.getElementById("testPre");
+
+shouldBeEmptyString("testDiv.style['width']");
+
+evalAndLog("testDiv.style['width'] = '100px'");
+evalAndLog("testDiv.style['width'] = 'calc(5px - 10px)'");
+shouldBeEqualToString("testDiv.style['width']", "calc(-5px)");
+shouldBeEqualToString("window.getComputedStyle(testDiv).getPropertyValue('width')", "0px");
+
+evalAndLog("testDiv.style['width'] = '100px'");
+evalAndLog("testDiv.style['width'] = 'calc(10% - 100px)'");
+shouldBeEqualToString("testDiv.style['width']", "calc(10% - 100px)");
+shouldBeEqualToString("window.getComputedStyle(testDiv).getPropertyValue('width')", "0px");
+
+evalAndLog("testDiv.style['line-height'] = '10%'");
+evalAndLog("testDiv.style['line-height'] = 'calc(10% - 20%)'");
+shouldBeEqualToString("testDiv.style['line-height']", "calc(-10%)");
+shouldBeEqualToString("window.getComputedStyle(testDiv).getPropertyValue('line-height')", "0px");
+
+evalAndLog("testPre.style['tab-size'] = '8'");
+evalAndLog("testPre.style['tab-size'] = 'calc(2 - 4)'");
+shouldBeEqualToString("testPre.style['tab-size']", "calc(-2)");
+shouldBeEqualToString("window.getComputedStyle(testPre).getPropertyValue('tab-size')", "0");
+
+</script>
+<script src=""
+</body>
Added: trunk/LayoutTests/fast/css/padding-calc-value-expected.txt (0 => 183765)
--- trunk/LayoutTests/fast/css/padding-calc-value-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css/padding-calc-value-expected.txt 2015-05-04 20:27:50 UTC (rev 183765)
@@ -0,0 +1,10 @@
+Tests assigning a calculated value to 'padding' CSS property.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS window.getComputedStyle(testDiv).getPropertyValue('padding-right') is "11px"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/css/padding-calc-value.html (0 => 183765)
--- trunk/LayoutTests/fast/css/padding-calc-value.html (rev 0)
+++ trunk/LayoutTests/fast/css/padding-calc-value.html 2015-05-04 20:27:50 UTC (rev 183765)
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<body>
+<script src=""
+
+<div id="testDiv" style="padding: 0 calc(.5em - 2px) 0 0; font-size: 26px;">
+</div>
+
+<script>
+description("Tests assigning a calculated value to 'padding' CSS property.");
+
+var testDiv = document.getElementById("testDiv");
+shouldBeEqualToString("window.getComputedStyle(testDiv).getPropertyValue('padding-right')", "11px");
+
+</script>
+<script src=""
+</body>
Modified: trunk/LayoutTests/fast/css/text-shadow-calc-value-expected.txt (183764 => 183765)
--- trunk/LayoutTests/fast/css/text-shadow-calc-value-expected.txt 2015-05-04 20:22:55 UTC (rev 183764)
+++ trunk/LayoutTests/fast/css/text-shadow-calc-value-expected.txt 2015-05-04 20:27:50 UTC (rev 183765)
@@ -11,8 +11,8 @@
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 previousStyle
-PASS window.getComputedStyle(testDiv).getPropertyValue('text-shadow') is previousComputedStyle
+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
Modified: trunk/LayoutTests/fast/css/text-shadow-calc-value.html (183764 => 183765)
--- trunk/LayoutTests/fast/css/text-shadow-calc-value.html 2015-05-04 20:22:55 UTC (rev 183764)
+++ trunk/LayoutTests/fast/css/text-shadow-calc-value.html 2015-05-04 20:27:50 UTC (rev 183765)
@@ -17,14 +17,11 @@
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")
-var previousStyle = testDiv.style['text-shadow'];
-var previousComputedStyle = window.getComputedStyle(testDiv).getPropertyValue('text-shadow');
-
-// Negative blur-radius is not allowed.
+// Negative blur-radius is not allowed so it should become 0.
evalAndLog("testDiv.style['text-shadow'] = 'calc(1 * 3px) calc(2 * 3px) calc(-3 * 3px) rgb(255, 255, 255)'");
// text-shadow should not be updated.
-shouldBe("testDiv.style['text-shadow']", "previousStyle");
-shouldBe("window.getComputedStyle(testDiv).getPropertyValue('text-shadow')", "previousComputedStyle")
+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=""
Modified: trunk/Source/WebCore/ChangeLog (183764 => 183765)
--- trunk/Source/WebCore/ChangeLog 2015-05-04 20:22:55 UTC (rev 183764)
+++ trunk/Source/WebCore/ChangeLog 2015-05-04 20:27:50 UTC (rev 183765)
@@ -1,3 +1,33 @@
+2015-05-04 Chris Dumez <[email protected]>
+
+ REGRESSION (r178156): CSS Parser incorrectly rejects valid calc() in padding-right property
+ https://bugs.webkit.org/show_bug.cgi?id=144584
+ <rdar://problem/20796829>
+
+ Reviewed by Darin Adler.
+
+ The CSS parser was rejecting calculated values at parsing time if it
+ considered the value was negative and the CSS property did not allow
+ negative values. However, doing so at this point will not always work
+ because we don't necessarily know the font-size yet (for e.g. for
+ calc(0.5em - 2px). Also, rejecting negative calculated values is not
+ the right behavior as the the specification. The specification says
+ we should clamp:
+ http://dev.w3.org/csswg/css-values-3/#calc-range
+
+ This patch updates validateCalculationUnit() to stop marking the value
+ as invalid if it is negative. Instead, let the CSSCalcValue's permitted
+ range clamp the value as needed.
+
+ This bug was causing the bottom graphic on aldentrio.com to not be
+ rendered properly.
+
+ Test: fast/css/negative-calc-values.html
+ fast/css/padding-calc-value.html
+
+ * css/CSSParser.cpp:
+ (WebCore::CSSParser::validateCalculationUnit):
+
2015-05-04 Eric Carlson <[email protected]>
[Mac] Show wireless playback placard even when an element has custom controls
Modified: trunk/Source/WebCore/css/CSSCalculationValue.h (183764 => 183765)
--- trunk/Source/WebCore/css/CSSCalculationValue.h 2015-05-04 20:22:55 UTC (rev 183764)
+++ trunk/Source/WebCore/css/CSSCalculationValue.h 2015-05-04 20:27:50 UTC (rev 183765)
@@ -92,7 +92,6 @@
CalculationCategory category() const { return m_expression->category(); }
bool isInt() const { return m_expression->isInteger(); }
double doubleValue() const;
- bool isNegative() const { return m_expression->doubleValue() < 0; }
bool isPositive() const { return m_expression->doubleValue() > 0; }
double computeLengthPx(const CSSToLengthConversionData&) const;
unsigned short primitiveType() const { return m_expression->primitiveType(); }
Modified: trunk/Source/WebCore/css/CSSParser.cpp (183764 => 183765)
--- trunk/Source/WebCore/css/CSSParser.cpp 2015-05-04 20:22:55 UTC (rev 183764)
+++ trunk/Source/WebCore/css/CSSParser.cpp 2015-05-04 20:27:50 UTC (rev 183765)
@@ -1670,18 +1670,12 @@
isValid = true;
if (!isValid && (unitFlags & FPositiveInteger) && calculation->isInt() && calculation->isPositive())
isValid = true;
- if (isValid && mustBeNonNegative && calculation->isNegative())
- isValid = false;
break;
case CalcLength:
isValid = (unitFlags & FLength);
- if (isValid && mustBeNonNegative && calculation->isNegative())
- isValid = false;
break;
case CalcPercent:
isValid = (unitFlags & FPercent);
- if (isValid && mustBeNonNegative && calculation->isNegative())
- isValid = false;
break;
case CalcPercentLength:
isValid = (unitFlags & FPercent) && (unitFlags & FLength);
