Skip to content

Commit c197ea5

Browse files
andruudChromium LUCI CQ
authored andcommitted
[functions] Validate defaults against their type
The spec was recently clarified to make @function rules with "mistyped" defaults invalid. For example, the following is now invalid, because the default given for --arg doesn't match the type given for --arg: @function --foo(--arg <color>: 10px) { /* ... */ } w3c/csswg-drafts#12243 Fixed: 423673310 Change-Id: I3239b573ece22bb48746a496758d53f57ac00b70 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6632858 Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org> Reviewed-by: Steinar H Gunderson <sesse@chromium.org> Cr-Commit-Position: refs/heads/main@{#1472448}
1 parent e906370 commit c197ea5

File tree

2 files changed

+24
-5
lines changed

2 files changed

+24
-5
lines changed

‎third_party/blink/renderer/core/css/parser/css_parser_impl.cc

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2413,8 +2413,7 @@ CSSParserImpl::ConsumeFunctionParameters(CSSParserTokenStream& stream) {
24132413
}
24142414
stream.ConsumeIncludingWhitespace();
24152415

2416-
CSSSyntaxDefinition type = ConsumeFunctionType(stream).value_or(
2417-
CSSSyntaxDefinition::CreateUniversal());
2416+
std::optional<CSSSyntaxDefinition> type = ConsumeFunctionType(stream);
24182417

24192418
CSSVariableData* default_value = nullptr;
24202419
if (stream.Peek().GetType() == kColonToken) {
@@ -2432,8 +2431,22 @@ CSSParserImpl::ConsumeFunctionParameters(CSSParserTokenStream& stream) {
24322431
/*comma_ends_declaration=*/true, important_ignored, *context_);
24332432
}
24342433

2434+
// If a type and a default are both provided, the default must
2435+
// parse successfully according to that type.
2436+
//
2437+
// https://drafts.csswg.org/css-mixins-1/#function-rule
2438+
if (type.has_value() && default_value) {
2439+
if (!default_value->NeedsVariableResolution() &&
2440+
!type->Parse(default_value->OriginalText(), *context_,
2441+
/*is_animation_tainted=*/false,
2442+
/*is_attr_tainted=*/false)) {
2443+
return std::nullopt;
2444+
}
2445+
}
2446+
24352447
parameters.push_back(StyleRuleFunction::Parameter{
2436-
parameter_name, std::move(type), default_value});
2448+
parameter_name, type.value_or(CSSSyntaxDefinition::CreateUniversal()),
2449+
default_value});
24372450
if (stream.Peek().GetType() == kRightParenthesisToken) {
24382451
// No more arguments.
24392452
break;

‎third_party/blink/web_tests/external/wpt/css/css-mixins/at-function-parsing.html

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060

6161
// Defaults.
6262
test_valid_prelude('@function --foo(--x : 10px)');
63+
test_valid_prelude('@function --foo(--x type(*): 10px)');
6364
test_valid_prelude('@function --foo(--x <length>: 10px)');
6465
test_valid_prelude('@function --foo(--x <length>: 10px, --y)');
6566
test_valid_prelude('@function --foo(--x, --y <length>: 10px)');
@@ -69,11 +70,16 @@
6970
// a default, even though there's no way to actually call --foo()
7071
// with just --y:
7172
test_valid_prelude('@function --foo(--x:1px, --y, --z:2px)');
72-
// The value does not have to match the type during @function parsing:
73-
test_valid_prelude('@function --foo(--x <angle>: 10px)');
7473

7574
test_invalid_prelude('@function --foo(--x: 10px !important)');
7675

76+
// Default type mismatch.
77+
test_invalid_prelude('@function --foo(--x <length>: 10deg)');
78+
test_invalid_prelude('@function --foo(--x <angle>: 10px)');
79+
test_invalid_prelude('@function --foo(--x <color>: 10px)');
80+
test_invalid_prelude('@function --foo(--x type(<color>+): red 5)');
81+
test_invalid_prelude('@function --foo(--x type(auto | none): thing)');
82+
7783
// Lists.
7884
test_valid_prelude('@function --foo(--x <length>#)');
7985
test_valid_prelude('@function --foo(--x <length>+)');

0 commit comments

Comments
 (0)