<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[178156] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/178156">178156</a></dd>
<dt>Author</dt> <dd>cdumez@apple.com</dd>
<dt>Date</dt> <dd>2015-01-08 21:29:09 -0800 (Thu, 08 Jan 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>ASSERTION FAILED: !valueWithCalculation.calculation() in WebCore::CSSParser::validateCalculationUnit
https://bugs.webkit.org/show_bug.cgi?id=140251
Reviewed by Darin Adler.
Source/WebCore:
Using a calculated value for text-shadow's blur-radius was hitting an
assertion in CSSParser::validateCalculationUnit() because validUnit()
is called twice, first with 'FLength' unit, then more stricly with
'FLength|FNonNeg' if parsing the blur-radius as it cannot be negative
as per the specification:
- http://dev.w3.org/csswg/css-text-decor-3/#text-shadow-property
- http://dev.w3.org/csswg/css-backgrounds-3/#shadow
On the second call, the ValueWithCalculation's m_calculation member
was already initialized and the code did not handle this. This patch
updates validateCalculationUnit() to teach it to reuse the previously
parsed calculation in this case. All it needs to do is to update the
existing CSSCalcValue's range to allow negative values or not.
When writing the layout test for this, I also noticed that the CSS
parser was not rejecting negative calculated values for blur-radius
(only negative non-calculated ones). This is because
validateCalculationUnit() was ignoring FNonNeg if the calculated
value is a Length. This patch also addresses the issue.
Test: fast/css/text-shadow-calc-value.html
* css/CSSCalculationValue.h:
(WebCore::CSSCalcValue::setPermittedValueRange):
Add a setter to update the CSSCalculationValue's permitted value range
so that the CSS parser does not need to fully reparse the calculation
only to update the permitted value range.
* css/CSSParser.cpp:
(WebCore::CSSParser::validateCalculationUnit):
- Teach the code to reuse the previously parsed calculation value.
- Do the FNonNeg check for Length calculations as well.
LayoutTests:
Add a layout test to check that using calculated values for
'text-shadow' CSS doesn't crash and works as intended. Also check
that the CSS parser is correctly validating the blur-radius, which
is supposed to be non-negative, as per the specification:
- http://dev.w3.org/csswg/css-text-decor-3/#text-shadow-property
- http://dev.w3.org/csswg/css-backgrounds-3/#shadow
* fast/css/text-shadow-calc-value-expected.txt: Added.
* fast/css/text-shadow-calc-value.html: Added.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorecssCSSCalculationValueh">trunk/Source/WebCore/css/CSSCalculationValue.h</a></li>
<li><a href="#trunkSourceWebCorecssCSSParsercpp">trunk/Source/WebCore/css/CSSParser.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastcsstextshadowcalcvalueexpectedtxt">trunk/LayoutTests/fast/css/text-shadow-calc-value-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastcsstextshadowcalcvaluehtml">trunk/LayoutTests/fast/css/text-shadow-calc-value.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (178155 => 178156)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2015-01-09 05:18:40 UTC (rev 178155)
+++ trunk/LayoutTests/ChangeLog 2015-01-09 05:29:09 UTC (rev 178156)
</span><span class="lines">@@ -1,3 +1,20 @@
</span><ins>+2015-01-08 Chris Dumez <cdumez@apple.com>
+
+ ASSERTION FAILED: !valueWithCalculation.calculation() in WebCore::CSSParser::validateCalculationUnit
+ https://bugs.webkit.org/show_bug.cgi?id=140251
+
+ Reviewed by Darin Adler.
+
+ Add a layout test to check that using calculated values for
+ 'text-shadow' CSS doesn't crash and works as intended. Also check
+ that the CSS parser is correctly validating the blur-radius, which
+ is supposed to be non-negative, as per the specification:
+ - http://dev.w3.org/csswg/css-text-decor-3/#text-shadow-property
+ - http://dev.w3.org/csswg/css-backgrounds-3/#shadow
+
+ * fast/css/text-shadow-calc-value-expected.txt: Added.
+ * fast/css/text-shadow-calc-value.html: Added.
+
</ins><span class="cx"> 2015-01-08 Benjamin Poulain <bpoulain@apple.com>
</span><span class="cx">
</span><span class="cx"> Make better use of the stack when compiling selectors
</span></span></pre></div>
<a id="trunkLayoutTestsfastcsstextshadowcalcvalueexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css/text-shadow-calc-value-expected.txt (0 => 178156)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/text-shadow-calc-value-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css/text-shadow-calc-value-expected.txt 2015-01-09 05:29:09 UTC (rev 178156)
</span><span class="lines">@@ -0,0 +1,19 @@
</span><ins>+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 previousStyle
+PASS window.getComputedStyle(testDiv).getPropertyValue('text-shadow') is previousComputedStyle
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcsstextshadowcalcvaluehtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css/text-shadow-calc-value.html (0 => 178156)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/text-shadow-calc-value.html (rev 0)
+++ trunk/LayoutTests/fast/css/text-shadow-calc-value.html 2015-01-09 05:29:09 UTC (rev 178156)
</span><span class="lines">@@ -0,0 +1,31 @@
</span><ins>+<!DOCTYPE html>
+<body>
+<script src="../../resources/js-test-pre.js"></script>
+<div id="testDiv" style="position: absolute;"></div>
+<script>
+description("Tests assigning a calculated value to 'text-shadow' CSS property.");
+
+var testDiv = document.getElementById("testDiv");
+
+shouldBeEmptyString("testDiv.style['text-shadow']");
+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.
+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")
+
+var previousStyle = testDiv.style['text-shadow'];
+var previousComputedStyle = window.getComputedStyle(testDiv).getPropertyValue('text-shadow');
+
+// Negative blur-radius is not allowed.
+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")
+
+</script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (178155 => 178156)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-01-09 05:18:40 UTC (rev 178155)
+++ trunk/Source/WebCore/ChangeLog 2015-01-09 05:29:09 UTC (rev 178156)
</span><span class="lines">@@ -1,3 +1,43 @@
</span><ins>+2015-01-08 Chris Dumez <cdumez@apple.com>
+
+ ASSERTION FAILED: !valueWithCalculation.calculation() in WebCore::CSSParser::validateCalculationUnit
+ https://bugs.webkit.org/show_bug.cgi?id=140251
+
+ Reviewed by Darin Adler.
+
+ Using a calculated value for text-shadow's blur-radius was hitting an
+ assertion in CSSParser::validateCalculationUnit() because validUnit()
+ is called twice, first with 'FLength' unit, then more stricly with
+ 'FLength|FNonNeg' if parsing the blur-radius as it cannot be negative
+ as per the specification:
+ - http://dev.w3.org/csswg/css-text-decor-3/#text-shadow-property
+ - http://dev.w3.org/csswg/css-backgrounds-3/#shadow
+
+ On the second call, the ValueWithCalculation's m_calculation member
+ was already initialized and the code did not handle this. This patch
+ updates validateCalculationUnit() to teach it to reuse the previously
+ parsed calculation in this case. All it needs to do is to update the
+ existing CSSCalcValue's range to allow negative values or not.
+
+ When writing the layout test for this, I also noticed that the CSS
+ parser was not rejecting negative calculated values for blur-radius
+ (only negative non-calculated ones). This is because
+ validateCalculationUnit() was ignoring FNonNeg if the calculated
+ value is a Length. This patch also addresses the issue.
+
+ Test: fast/css/text-shadow-calc-value.html
+
+ * css/CSSCalculationValue.h:
+ (WebCore::CSSCalcValue::setPermittedValueRange):
+ Add a setter to update the CSSCalculationValue's permitted value range
+ so that the CSS parser does not need to fully reparse the calculation
+ only to update the permitted value range.
+
+ * css/CSSParser.cpp:
+ (WebCore::CSSParser::validateCalculationUnit):
+ - Teach the code to reuse the previously parsed calculation value.
+ - Do the FNonNeg check for Length calculations as well.
+
</ins><span class="cx"> 2015-01-08 Darin Adler <darin@apple.com>
</span><span class="cx">
</span><span class="cx"> Modernize and streamline HTMLTokenizer
</span></span></pre></div>
<a id="trunkSourceWebCorecssCSSCalculationValueh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/css/CSSCalculationValue.h (178155 => 178156)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/css/CSSCalculationValue.h 2015-01-09 05:18:40 UTC (rev 178155)
+++ trunk/Source/WebCore/css/CSSCalculationValue.h 2015-01-09 05:29:09 UTC (rev 178156)
</span><span class="lines">@@ -97,6 +97,7 @@
</span><span class="cx"> double computeLengthPx(const CSSToLengthConversionData&) const;
</span><span class="cx">
</span><span class="cx"> Ref<CalculationValue> createCalculationValue(const CSSToLengthConversionData&) const;
</span><ins>+ void setPermittedValueRange(CalculationPermittedValueRange);
</ins><span class="cx">
</span><span class="cx"> String customCSSText() const;
</span><span class="cx"> bool equals(const CSSCalcValue&) const;
</span><span class="lines">@@ -107,7 +108,7 @@
</span><span class="cx"> double clampToPermittedRange(double) const;
</span><span class="cx">
</span><span class="cx"> const Ref<CSSCalcExpressionNode> m_expression;
</span><del>- const bool m_shouldClampToNonNegative;
</del><ins>+ bool m_shouldClampToNonNegative;
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> inline CSSCalcValue::CSSCalcValue(Ref<CSSCalcExpressionNode>&& expression, bool shouldClampToNonNegative)
</span><span class="lines">@@ -123,6 +124,11 @@
</span><span class="cx"> m_shouldClampToNonNegative ? CalculationRangeNonNegative : CalculationRangeAll);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+inline void CSSCalcValue::setPermittedValueRange(CalculationPermittedValueRange range)
+{
+ m_shouldClampToNonNegative = range != CalculationRangeAll;
+}
+
</ins><span class="cx"> } // namespace WebCore
</span><span class="cx">
</span><span class="cx"> SPECIALIZE_TYPE_TRAITS_CSS_VALUE(CSSCalcValue, isCalcValue())
</span></span></pre></div>
<a id="trunkSourceWebCorecssCSSParsercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/css/CSSParser.cpp (178155 => 178156)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/css/CSSParser.cpp 2015-01-09 05:18:40 UTC (rev 178155)
+++ trunk/Source/WebCore/css/CSSParser.cpp 2015-01-09 05:29:09 UTC (rev 178156)
</span><span class="lines">@@ -1586,10 +1586,17 @@
</span><span class="cx"> {
</span><span class="cx"> bool mustBeNonNegative = unitFlags & FNonNeg;
</span><span class="cx">
</span><del>- ASSERT(!valueWithCalculation.calculation());
- RefPtr<CSSCalcValue> calculation = parseCalculation(valueWithCalculation, mustBeNonNegative ? CalculationRangeNonNegative : CalculationRangeAll);
- if (!calculation)
- return false;
</del><ins>+ RefPtr<CSSCalcValue> calculation;
+ if (valueWithCalculation.calculation()) {
+ // The calculation value was already parsed so we reuse it. However, we may need to update its range.
+ calculation = valueWithCalculation.calculation();
+ calculation->setPermittedValueRange(mustBeNonNegative ? CalculationRangeNonNegative : CalculationRangeAll);
+ } else {
+ valueWithCalculation.setCalculation(parseCalculation(valueWithCalculation, mustBeNonNegative ? CalculationRangeNonNegative : CalculationRangeAll));
+ calculation = valueWithCalculation.calculation();
+ if (!calculation)
+ return false;
+ }
</ins><span class="cx">
</span><span class="cx"> bool isValid = false;
</span><span class="cx"> switch (calculation->category()) {
</span><span class="lines">@@ -1604,6 +1611,8 @@
</span><span class="cx"> break;
</span><span class="cx"> case CalcLength:
</span><span class="cx"> isValid = (unitFlags & FLength);
</span><ins>+ if (isValid && mustBeNonNegative && calculation->isNegative())
+ isValid = false;
</ins><span class="cx"> break;
</span><span class="cx"> case CalcPercent:
</span><span class="cx"> isValid = (unitFlags & FPercent);
</span><span class="lines">@@ -1628,8 +1637,7 @@
</span><span class="cx"> case CalcOther:
</span><span class="cx"> break;
</span><span class="cx"> }
</span><del>- if (isValid)
- valueWithCalculation.setCalculation(WTF::move(calculation));
</del><ins>+
</ins><span class="cx"> return isValid;
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>