Commit 8901199d authored by Fredrik Söderqvist's avatar Fredrik Söderqvist Committed by Commit Bot

Align fast-path <quirky-color> behavior with non-fast-path parsing

Regular/non-fast-path parsing limits <quirky-color> to a limited number
of properties (based on the list in the quirks spec [1]).
The fast-path however did not limit based on property but only
considered the parser mode. This meant that some parsing code-paths
would allow <quirky-color> even when they shouldn't have.

[1] https://quirks.spec.whatwg.org/#the-hashless-hex-color-quirk

Bug: 1024491, 998400
Change-Id: I88acb102c434c1a73c79eeb1eb94ad06c0677b7c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2385357Reviewed-by: default avatarRune Lillesveen <futhark@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#803475}
parent d97ed1f5
...@@ -176,6 +176,22 @@ static inline bool IsColorPropertyID(CSSPropertyID property_id) { ...@@ -176,6 +176,22 @@ static inline bool IsColorPropertyID(CSSPropertyID property_id) {
} }
} }
// https://quirks.spec.whatwg.org/#the-hashless-hex-color-quirk
static inline bool ColorPropertyAllowsQuirkyColor(CSSPropertyID property_id) {
switch (property_id) {
case CSSPropertyID::kColor:
case CSSPropertyID::kBackgroundColor:
case CSSPropertyID::kBorderBottomColor:
case CSSPropertyID::kBorderLeftColor:
case CSSPropertyID::kBorderRightColor:
case CSSPropertyID::kBorderTopColor:
return true;
default:
DCHECK(IsColorPropertyID(property_id));
return false;
}
}
// Returns the number of characters which form a valid double // Returns the number of characters which form a valid double
// and are terminated by the given terminator character // and are terminated by the given terminator character
template <typename CharacterType> template <typename CharacterType>
...@@ -507,8 +523,12 @@ static bool FastParseColorInternal(RGBA32& rgb, ...@@ -507,8 +523,12 @@ static bool FastParseColorInternal(RGBA32& rgb,
return false; return false;
} }
CSSValue* CSSParserFastPaths::ParseColor(const String& string, static CSSValue* ParseColor(CSSPropertyID property_id,
CSSParserMode parser_mode) { const String& string,
CSSParserMode parser_mode) {
if (!IsColorPropertyID(property_id))
return nullptr;
DCHECK(!string.IsEmpty()); DCHECK(!string.IsEmpty());
CSSValueID value_id = CssValueKeywordID(string); CSSValueID value_id = CssValueKeywordID(string);
if (StyleColor::IsColorKeyword(value_id)) { if (StyleColor::IsColorKeyword(value_id)) {
...@@ -518,7 +538,8 @@ CSSValue* CSSParserFastPaths::ParseColor(const String& string, ...@@ -518,7 +538,8 @@ CSSValue* CSSParserFastPaths::ParseColor(const String& string,
} }
RGBA32 color; RGBA32 color;
bool quirks_mode = IsQuirksModeBehavior(parser_mode); bool quirks_mode = IsQuirksModeBehavior(parser_mode) &&
ColorPropertyAllowsQuirkyColor(property_id);
// Fast path for hex colors and rgb()/rgba() colors // Fast path for hex colors and rgb()/rgba() colors
bool parse_result = bool parse_result =
...@@ -530,6 +551,11 @@ CSSValue* CSSParserFastPaths::ParseColor(const String& string, ...@@ -530,6 +551,11 @@ CSSValue* CSSParserFastPaths::ParseColor(const String& string,
return cssvalue::CSSColorValue::Create(color); return cssvalue::CSSColorValue::Create(color);
} }
CSSValue* CSSParserFastPaths::ParseColor(const String& string,
CSSParserMode parser_mode) {
return blink::ParseColor(CSSPropertyID::kColor, string, parser_mode);
}
bool CSSParserFastPaths::IsValidKeywordPropertyAndValue( bool CSSParserFastPaths::IsValidKeywordPropertyAndValue(
CSSPropertyID property_id, CSSPropertyID property_id,
CSSValueID value_id, CSSValueID value_id,
...@@ -1369,8 +1395,8 @@ CSSValue* CSSParserFastPaths::MaybeParseValue(CSSPropertyID property_id, ...@@ -1369,8 +1395,8 @@ CSSValue* CSSParserFastPaths::MaybeParseValue(CSSPropertyID property_id,
if (CSSValue* length = if (CSSValue* length =
ParseSimpleLengthValue(property_id, string, parser_mode)) ParseSimpleLengthValue(property_id, string, parser_mode))
return length; return length;
if (IsColorPropertyID(property_id)) if (CSSValue* color = blink::ParseColor(property_id, string, parser_mode))
return ParseColor(string, parser_mode); return color;
if (CSSValue* keyword = ParseKeywordValue(property_id, string, parser_mode)) if (CSSValue* keyword = ParseKeywordValue(property_id, string, parser_mode))
return keyword; return keyword;
if (CSSValue* transform = ParseSimpleTransform(property_id, string)) if (CSSValue* transform = ParseSimpleTransform(property_id, string))
......
This is a testharness.js-based test.
PASS e.style['stop-color'] = "auto" should not set the property value
FAIL e.style['stop-color'] = "123" should not set the property value assert_equals: expected "" but got "rgb(17, 34, 51)"
PASS e.style['stop-color'] = "#12" should not set the property value
PASS e.style['stop-color'] = "#123456789" should not set the property value
PASS e.style['stop-color'] = "rgb" should not set the property value
PASS e.style['stop-color'] = "rgb(1)" should not set the property value
PASS e.style['stop-color'] = "rgb(1,2,3,4,5)" should not set the property value
PASS e.style['stop-color'] = "hsla(1,2,3,4,5)" should not set the property value
PASS e.style['stop-color'] = "rgb(10%, 20, 30%)" should not set the property value
PASS e.style['stop-color'] = "rgba(-2, 300, 400%, -0.5)" should not set the property value
Harness: the test ran to completion.
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