Commit 0992c24a authored by Xiaocheng Hu's avatar Xiaocheng Hu Committed by Commit Bot

Handle non-negativeness of blur radius properly

Spec on blur radius says [1]:

"... Specifies the blur radius. Negative values are not allowed."

Current implementation performs parse time range check, which is
incorrect as calc() doesn't always have a double value. And spec
requires clamping instead of parse time check for calc() [2]:

"Parse-time range-checking of values is not performed within math
functions, and therefore out-of-range values do not cause the
declaration to become invalid. However, the value resulting from
an expression must be clamped to the range allowed in the target
context."

This patch changes it to pass |kValueRangeNonNegative| to
|ConsumeLength| as it handles non-negative calc() more properly.

This is also preparation for adding DCHECK into GetDoubleValue() to
avoid calling it on calc() without a double value.

[1] https://drafts.csswg.org/css-backgrounds-3/#shadow-blur-radius
[2] https://drafts.csswg.org/css-values-4/#calc-range

Bug: 979895
Change-Id: Ia8c36641f4ec4111ecf5a94a6ffc6eaacf22049c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1691747Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#675815}
parent 0e860e70
......@@ -1206,12 +1206,9 @@ CSSShadowValue* ParseSingleShadow(CSSParserTokenRange& range,
return nullptr;
CSSPrimitiveValue* blur_radius = css_property_parser_helpers::ConsumeLength(
range, css_parser_mode, kValueRangeAll);
range, css_parser_mode, kValueRangeNonNegative);
CSSPrimitiveValue* spread_distance = nullptr;
if (blur_radius) {
// Blur radius must be non-negative.
if (blur_radius->GetDoubleValue() < 0)
return nullptr;
if (inset_and_spread == AllowInsetAndSpread::kAllow) {
spread_distance = css_property_parser_helpers::ConsumeLength(
range, css_parser_mode, kValueRangeAll);
......
......@@ -8,6 +8,7 @@
<link rel="help" href="https://drafts.csswg.org/css-backgrounds/#box-shadow">
<meta name="assert" content="box-shadow supports only the grammar 'none | <shadow>#'.">
<meta name="assert" content="Lengths must stay adjacent." />
<meta name="assert" content="<blur-radius> must be non-negative." />
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/css/support/parsing-testcommon.js"></script>
......@@ -57,6 +58,9 @@ test_invalid_value("box-shadow", "inset 4px -4px red 0");
test_invalid_value("box-shadow", "4px -4px red 0 inset");
test_invalid_value("box-shadow", "red 4px -4px inset 0");
test_invalid_value("box-shadow", "4px -4px inset 0 red");
// <blur-radius> must be non-negative
test_invalid_value("box-shadow", "1px 1px -1px");
</script>
</body>
</html>
......@@ -57,6 +57,11 @@ test_valid_value("box-shadow", "4px -4px 0 0 green inset", "green 4px -4px 0px 0
test_valid_value("box-shadow", "4px -4px 0 0 inset green", "green 4px -4px 0px 0px inset");
test_valid_value("box-shadow", "green 4px -4px 0 0 inset", "green 4px -4px 0px 0px inset");
test_valid_value("box-shadow", "inset 4px -4px 0 0 green", "green 4px -4px 0px 0px inset");
// No parse-time range-checking for <blur-radius> given as a math function
// https://drafts.csswg.org/css-values-4/#calc-range
test_valid_value("box-shadow", "1px 1px calc(-1px)", "1px 1px calc(-1px)");
test_valid_value("box-shadow", "1px 1px calc(1em - 2px)", "1px 1px calc(1em - 2px)");
</script>
</body>
</html>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment