Commit 106b2819 authored by Chris Nardi's avatar Chris Nardi Committed by Commit Bot

Update parsing of RGB() and RGBA() functions to match CSS Color 4

In CSS Color 4, RGB() and RGBA() were synonymized, along with allowing
whitespace to separate the channels instead of commas, introducing
a backslash as a new way to separate the alpha parameter, and allowing
percent values for the alpha parameter. This updates our parsing
behavior to reflect these changes, and enables now-passing WPT tests.
The new spec is at https://drafts.csswg.org/css-color/#rgb-functions.

Bug: 788707
Change-Id: Ib0f29009e477fc88cd1ceda03ff0ebb947a36ea3
Reviewed-on: https://chromium-review.googlesource.com/798831
Commit-Queue: Chris Nardi <hichris123@gmail.com>
Reviewed-by: default avatarEric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521703}
parent d1e86099
......@@ -1800,21 +1800,13 @@ external/wpt/css/css-color/lch-006.html [ WontFix ]
external/wpt/css/css-color/lch-007.html [ WontFix ]
# New rgb() and rgba() syntax not supported. https://crbug.com/788707
external/wpt/css/css-color/rgb-001.html [ WontFix ]
external/wpt/css/css-color/rgb-002.html [ WontFix ]
external/wpt/css/css-color/rgb-003.html [ WontFix ]
external/wpt/css/css-color/rgb-004.html [ WontFix ]
external/wpt/css/css-color/rgb-005.html [ WontFix ]
external/wpt/css/css-color/rgb-006.html [ WontFix ]
external/wpt/css/css-color/rgb-007.html [ WontFix ]
external/wpt/css/css-color/rgb-008.html [ WontFix ]
external/wpt/css/css-color/rgba-001.html [ WontFix ]
external/wpt/css/css-color/rgba-002.html [ WontFix ]
external/wpt/css/css-color/rgba-003.html [ WontFix ]
external/wpt/css/css-color/rgba-004.html [ WontFix ]
external/wpt/css/css-color/rgba-005.html [ WontFix ]
external/wpt/css/css-color/rgba-006.html [ WontFix ]
external/wpt/css/css-color/rgba-007.html [ WontFix ]
external/wpt/css/css-color/rgba-008.html [ WontFix ]
# Failures on the initial import of css-color. Test suite had many other bugs,
......
......@@ -2791,16 +2791,7 @@ crbug.com/709227 external/wpt/offscreen-canvas/fill-and-stroke-styles/2d.fillSty
crbug.com/709227 external/wpt/offscreen-canvas/fill-and-stroke-styles/2d.fillStyle.parse.css-color-4-hsla-8.worker.html [ Failure ]
crbug.com/709227 external/wpt/offscreen-canvas/fill-and-stroke-styles/2d.fillStyle.parse.css-color-4-hsla-9.worker.html [ Failure ]
crbug.com/709227 external/wpt/offscreen-canvas/fill-and-stroke-styles/2d.fillStyle.parse.css-color-4-rgb-1.worker.html [ Failure ]
crbug.com/709227 external/wpt/offscreen-canvas/fill-and-stroke-styles/2d.fillStyle.parse.css-color-4-rgb-2.worker.html [ Failure ]
crbug.com/709227 external/wpt/offscreen-canvas/fill-and-stroke-styles/2d.fillStyle.parse.css-color-4-rgb-3.worker.html [ Failure ]
crbug.com/709227 external/wpt/offscreen-canvas/fill-and-stroke-styles/2d.fillStyle.parse.css-color-4-rgb-4.worker.html [ Failure ]
crbug.com/709227 external/wpt/offscreen-canvas/fill-and-stroke-styles/2d.fillStyle.parse.css-color-4-rgb-5.worker.html [ Failure ]
crbug.com/709227 external/wpt/offscreen-canvas/fill-and-stroke-styles/2d.fillStyle.parse.css-color-4-rgb-6.worker.html [ Failure ]
crbug.com/709227 external/wpt/offscreen-canvas/fill-and-stroke-styles/2d.fillStyle.parse.css-color-4-rgba-1.worker.html [ Failure ]
crbug.com/709227 external/wpt/offscreen-canvas/fill-and-stroke-styles/2d.fillStyle.parse.css-color-4-rgba-3.worker.html [ Failure ]
crbug.com/709227 external/wpt/offscreen-canvas/fill-and-stroke-styles/2d.fillStyle.parse.css-color-4-rgba-4.worker.html [ Failure ]
crbug.com/709227 external/wpt/offscreen-canvas/fill-and-stroke-styles/2d.fillStyle.parse.css-color-4-rgba-5.worker.html [ Failure ]
crbug.com/709227 external/wpt/offscreen-canvas/fill-and-stroke-styles/2d.fillStyle.parse.css-color-4-rgba-6.worker.html [ Failure ]
crbug.com/709227 external/wpt/offscreen-canvas/path-objects/2d.path.stroke.prune.arc.worker.html [ Failure ]
crbug.com/709227 external/wpt/offscreen-canvas/path-objects/2d.path.stroke.prune.closed.worker.html [ Failure ]
crbug.com/709227 external/wpt/offscreen-canvas/path-objects/2d.path.stroke.prune.curve.worker.html [ Failure ]
......
......@@ -62,11 +62,11 @@ PASS colorTest.parseColor("rgb(0, 0, light)") is "parse error"
PASS colorTest.parseColor("rgb()") is "parse error"
PASS colorTest.parseColor("rgb(0)") is "parse error"
PASS colorTest.parseColor("rgb(0, 0)") is "parse error"
PASS colorTest.parseColor("rgb(0, 0, 0, 0)") is "parse error"
FAIL colorTest.parseColor("rgb(0, 0, 0, 0)") should be parse error. Was rgba(0, 0, 0, 0).
PASS colorTest.parseColor("rgb(0%)") is "parse error"
PASS colorTest.parseColor("rgb(0%, 0%)") is "parse error"
PASS colorTest.parseColor("rgb(0%, 0%, 0%, 0%)") is "parse error"
PASS colorTest.parseColor("rgb(0%, 0%, 0%, 0)") is "parse error"
FAIL colorTest.parseColor("rgb(0%, 0%, 0%, 0%)") should be parse error. Was rgba(0, 0, 0, 0).
FAIL colorTest.parseColor("rgb(0%, 0%, 0%, 0)") should be parse error. Was rgba(0, 0, 0, 0).
PASS colorTest.parseColor("rgba(0, 0, 0, 0)") is "rgba(0, 0, 0, 0)"
PASS colorTest.parseColor("rgba(204, 0, 102, 0.3)") is "rgba(204, 0, 102, 0.3)"
PASS colorTest.parseColor("RGBA(255, 255, 255, 0)") is "rgba(255, 255, 255, 0)"
......@@ -86,7 +86,7 @@ PASS colorTest.parseColor("rgba(0%, 20%, 100%, 0.42)") is "rgba(0, 51, 255, 0.42
PASS colorTest.parseColor("rgba(0%, 20%, 100%, 0)") is "rgba(0, 51, 255, 0)"
PASS colorTest.parseColor("rgba(0%, 20%, 100%, -0.1)") is "rgba(0, 51, 255, 0)"
PASS colorTest.parseColor("rgba(0%, 20%, 100%, -139)") is "rgba(0, 51, 255, 0)"
PASS colorTest.parseColor("rgba(255, 255, 255, 0%)") is "parse error"
FAIL colorTest.parseColor("rgba(255, 255, 255, 0%)") should be parse error. Was rgba(255, 255, 255, 0).
PASS colorTest.parseColor("rgba(10%, 50%, 0, 1)") is "parse error"
PASS colorTest.parseColor("rgba(255, 50%, 0%, 1)") is "parse error"
PASS colorTest.parseColor("rgba(0, 0, 0 0)") is "parse error"
......@@ -94,12 +94,12 @@ PASS colorTest.parseColor("rgba(0, 0, 0, 0deg)") is "parse error"
PASS colorTest.parseColor("rgba(0, 0, 0, light)") is "parse error"
PASS colorTest.parseColor("rgba()") is "parse error"
PASS colorTest.parseColor("rgba(0)") is "parse error"
PASS colorTest.parseColor("rgba(0, 0, 0)") is "parse error"
FAIL colorTest.parseColor("rgba(0, 0, 0)") should be parse error. Was rgb(0, 0, 0).
PASS colorTest.parseColor("rgba(0, 0, 0, 0, 0)") is "parse error"
PASS colorTest.parseColor("rgba(0%)") is "parse error"
PASS colorTest.parseColor("rgba(0%, 0%)") is "parse error"
PASS colorTest.parseColor("rgba(0%, 0%, 0%)") is "parse error"
PASS colorTest.parseColor("rgba(0%, 0%, 0%, 0%)") is "parse error"
FAIL colorTest.parseColor("rgba(0%, 0%, 0%)") should be parse error. Was rgb(0, 0, 0).
FAIL colorTest.parseColor("rgba(0%, 0%, 0%, 0%)") should be parse error. Was rgba(0, 0, 0, 0).
PASS colorTest.parseColor("rgba(0%, 0%, 0%, 0%, 0%)") is "parse error"
PASS colorTest.parseColor("HSL(0, 0%, 0%)") is "rgb(0, 0, 0)"
PASS colorTest.parseColor("hsL(0, 100%, 50%)") is "rgb(255, 0, 0)"
......
This is a testharness.js-based test.
FAIL Canvas test: 2d.fillStyle.parse.css-color-4-rgb-2 assert_equals: Red channel of the pixel at (50, 25) expected 0 but got 255
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL Canvas test: 2d.fillStyle.parse.css-color-4-rgb-3 assert_equals: Red channel of the pixel at (50, 25) expected 0 but got 255
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL Canvas test: 2d.fillStyle.parse.css-color-4-rgb-4 assert_equals: Red channel of the pixel at (50, 25) expected 0 but got 255
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL Canvas test: 2d.fillStyle.parse.css-color-4-rgb-5 assert_equals: Red channel of the pixel at (50, 25) expected 0 but got 255
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL Canvas test: 2d.fillStyle.parse.css-color-4-rgb-6 assert_equals: Red channel of the pixel at (50, 25) expected 0 but got 255
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL Canvas test: 2d.fillStyle.parse.css-color-4-rgba-3 assert_equals: Red channel of the pixel at (50, 25) expected 0 but got 255
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL Canvas test: 2d.fillStyle.parse.css-color-4-rgba-4 assert_equals: Red channel of the pixel at (50, 25) expected 0 but got 255
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL Canvas test: 2d.fillStyle.parse.css-color-4-rgba-5 assert_equals: Red channel of the pixel at (50, 25) expected 0 but got 255
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL Canvas test: 2d.fillStyle.parse.css-color-4-rgba-6 assert_equals: Red channel of the pixel at (50, 25) expected 0 but got 255
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL OffscreenCanvas test: 2d.fillStyle.parse.css-color-4-rgb-2 assert_equals: Red channel of the pixel at (50, 25) expected 0 but got 255
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL OffscreenCanvas test: 2d.fillStyle.parse.css-color-4-rgb-3 assert_equals: Red channel of the pixel at (50, 25) expected 0 but got 255
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL OffscreenCanvas test: 2d.fillStyle.parse.css-color-4-rgb-4 assert_equals: Red channel of the pixel at (50, 25) expected 0 but got 255
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL OffscreenCanvas test: 2d.fillStyle.parse.css-color-4-rgb-5 assert_equals: Red channel of the pixel at (50, 25) expected 0 but got 255
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL OffscreenCanvas test: 2d.fillStyle.parse.css-color-4-rgb-6 assert_equals: Red channel of the pixel at (50, 25) expected 0 but got 255
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL OffscreenCanvas test: 2d.fillStyle.parse.css-color-4-rgba-3 assert_equals: Red channel of the pixel at (50, 25) expected 0 but got 255
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL OffscreenCanvas test: 2d.fillStyle.parse.css-color-4-rgba-4 assert_equals: Red channel of the pixel at (50, 25) expected 0 but got 255
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL OffscreenCanvas test: 2d.fillStyle.parse.css-color-4-rgba-5 assert_equals: Red channel of the pixel at (50, 25) expected 0 but got 255
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL OffscreenCanvas test: 2d.fillStyle.parse.css-color-4-rgba-6 assert_equals: Red channel of the pixel at (50, 25) expected 0 but got 255
Harness: the test ran to completion.
......@@ -15,7 +15,7 @@ test(function(){
gradient.addColorStop(0, 'rgb(257, 0)');
});
assert_throws("SYNTAX_ERR", function() {
gradient.addColorStop(0, 'rgb(257, 0, 5, 0)');
gradient.addColorStop(0, 'rgb(257, 0, 5 / 0)');
});
}, 'This test checks invalid colors on gradients.');
</script>
......@@ -6,7 +6,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
Parsed as blue: blue
Parsed as rgb(0, 255, 0): rgb(0, 255, 0)
Parsed as rgb(255, 255, 0): rgb(100%,100%,0%
Failed to parse: rgba(100%,100%,0%
Parsed as rgb(255, 255, 0): rgba(100%,100%,0%
Failed to parse: rgb(100%,100%,r)
Failed to parse: rgb (100%,100%,r)
Failed to parse: rgb(100%, 1, 1)
......
......@@ -267,6 +267,8 @@ template <typename CharacterType>
static bool ParseColorIntOrPercentage(const CharacterType*& string,
const CharacterType* end,
const char terminator,
bool& should_whitespace_terminate,
bool is_first_value,
CSSPrimitiveValue::UnitType& expect,
int& value) {
const CharacterType* current = string;
......@@ -328,8 +330,21 @@ static bool ParseColorIntOrPercentage(const CharacterType*& string,
while (current != end && IsHTMLSpace<CharacterType>(*current))
current++;
if (current == end || *current++ != terminator)
if (current == end || *current != terminator) {
if (!should_whitespace_terminate ||
!IsHTMLSpace<CharacterType>(*(current - 1))) {
return false;
}
} else if (should_whitespace_terminate && is_first_value) {
should_whitespace_terminate = false;
} else if (should_whitespace_terminate) {
return false;
}
if (!should_whitespace_terminate)
current++;
// Clamp negative values at zero.
value = negative ? 0 : static_cast<int>(local_value);
string = current;
......@@ -409,26 +424,16 @@ static inline bool ParseAlphaValue(const CharacterType*& string,
}
template <typename CharacterType>
static inline bool MightBeRGBA(const CharacterType* characters,
unsigned length) {
if (length < 5)
return false;
return characters[4] == '(' &&
IsASCIIAlphaCaselessEqual(characters[0], 'r') &&
IsASCIIAlphaCaselessEqual(characters[1], 'g') &&
IsASCIIAlphaCaselessEqual(characters[2], 'b') &&
IsASCIIAlphaCaselessEqual(characters[3], 'a');
}
template <typename CharacterType>
static inline bool MightBeRGB(const CharacterType* characters,
unsigned length) {
static inline bool MightBeRGBOrRGBA(const CharacterType* characters,
unsigned length) {
if (length < 4)
return false;
return characters[3] == '(' &&
IsASCIIAlphaCaselessEqual(characters[0], 'r') &&
return IsASCIIAlphaCaselessEqual(characters[0], 'r') &&
IsASCIIAlphaCaselessEqual(characters[1], 'g') &&
IsASCIIAlphaCaselessEqual(characters[2], 'b');
IsASCIIAlphaCaselessEqual(characters[2], 'b') &&
(characters[3] == '(' ||
(IsASCIIAlphaCaselessEqual(characters[3], 'a') &&
characters[4] == '('));
}
template <typename CharacterType>
......@@ -446,45 +451,58 @@ static bool FastParseColorInternal(RGBA32& rgb,
return true;
}
// Try rgba() syntax.
if (MightBeRGBA(characters, length)) {
const CharacterType* current = characters + 5;
// rgb() and rgba() have the same syntax
if (MightBeRGBOrRGBA(characters, length)) {
int length_to_add = IsASCIIAlphaCaselessEqual(characters[3], 'a') ? 5 : 4;
const CharacterType* current = characters + length_to_add;
const CharacterType* end = characters + length;
int red;
int green;
int blue;
int alpha;
bool should_have_alpha = false;
bool should_whitespace_terminate = true;
bool no_whitespace_check = false;
if (!ParseColorIntOrPercentage(current, end, ',', expect, red))
return false;
if (!ParseColorIntOrPercentage(current, end, ',', expect, green))
if (!ParseColorIntOrPercentage(current, end, ',',
should_whitespace_terminate,
true /* is_first_value */, expect, red))
return false;
if (!ParseColorIntOrPercentage(current, end, ',', expect, blue))
if (!ParseColorIntOrPercentage(current, end, ',',
should_whitespace_terminate,
false /* is_first_value */, expect, green))
return false;
if (!ParseAlphaValue(current, end, ')', alpha))
return false;
if (current != end)
return false;
rgb = MakeRGBA(red, green, blue, alpha);
return true;
}
if (!ParseColorIntOrPercentage(current, end, ',', no_whitespace_check,
false /* is_first_value */, expect, blue)) {
// Might have slash as separator
if (ParseColorIntOrPercentage(current, end, '/', no_whitespace_check,
false /* is_first_value */, expect, blue)) {
if (!should_whitespace_terminate)
return false;
should_have_alpha = true;
}
// Might not have alpha
else if (!ParseColorIntOrPercentage(
current, end, ')', no_whitespace_check,
false /* is_first_value */, expect, blue))
return false;
} else {
if (should_whitespace_terminate)
return false;
should_have_alpha = true;
}
// Try rgb() syntax.
if (MightBeRGB(characters, length)) {
const CharacterType* current = characters + 4;
const CharacterType* end = characters + length;
int red;
int green;
int blue;
if (!ParseColorIntOrPercentage(current, end, ',', expect, red))
return false;
if (!ParseColorIntOrPercentage(current, end, ',', expect, green))
return false;
if (!ParseColorIntOrPercentage(current, end, ')', expect, blue))
return false;
if (current != end)
return false;
rgb = MakeRGB(red, green, blue);
if (should_have_alpha) {
if (!ParseAlphaValue(current, end, ')', alpha))
return false;
if (current != end)
return false;
rgb = MakeRGBA(red, green, blue, alpha);
} else {
if (current != end)
return false;
rgb = MakeRGB(red, green, blue);
}
return true;
}
......
......@@ -118,4 +118,45 @@ TEST(CSSParserFastPathsTest, ParseColorWithLargeAlpha) {
EXPECT_EQ(Color::kBlack, ToCSSColorValue(*value).Value());
}
TEST(CSSParserFastPathsTest, ParseColorWithNewSyntax) {
CSSValue* value =
CSSParserFastPaths::ParseColor("rgba(0 0 0)", kHTMLStandardMode);
EXPECT_NE(nullptr, value);
EXPECT_TRUE(value->IsColorValue());
EXPECT_EQ(Color::kBlack, ToCSSColorValue(*value).Value());
value = CSSParserFastPaths::ParseColor("rgba(0 0 0 / 1)", kHTMLStandardMode);
EXPECT_NE(nullptr, value);
EXPECT_TRUE(value->IsColorValue());
EXPECT_EQ(Color::kBlack, ToCSSColorValue(*value).Value());
value = CSSParserFastPaths::ParseColor("rgba(0, 0, 0, 1)", kHTMLStandardMode);
EXPECT_NE(nullptr, value);
EXPECT_TRUE(value->IsColorValue());
EXPECT_EQ(Color::kBlack, ToCSSColorValue(*value).Value());
value = CSSParserFastPaths::ParseColor("RGBA(0 0 0 / 1)", kHTMLStandardMode);
EXPECT_NE(nullptr, value);
EXPECT_TRUE(value->IsColorValue());
EXPECT_EQ(Color::kBlack, ToCSSColorValue(*value).Value());
value = CSSParserFastPaths::ParseColor("RGB(0 0 0 / 1)", kHTMLStandardMode);
EXPECT_NE(nullptr, value);
EXPECT_TRUE(value->IsColorValue());
EXPECT_EQ(Color::kBlack, ToCSSColorValue(*value).Value());
value = CSSParserFastPaths::ParseColor("rgba(0 0 0 0)", kHTMLStandardMode);
EXPECT_EQ(nullptr, value);
value = CSSParserFastPaths::ParseColor("rgba(0, 0 0 1)", kHTMLStandardMode);
EXPECT_EQ(nullptr, value);
value =
CSSParserFastPaths::ParseColor("rgba(0, 0, 0 / 1)", kHTMLStandardMode);
EXPECT_EQ(nullptr, value);
value = CSSParserFastPaths::ParseColor("rgba(0 0 0, 1)", kHTMLStandardMode);
EXPECT_EQ(nullptr, value);
}
} // namespace blink
......@@ -550,9 +550,7 @@ static int ClampRGBComponent(const CSSPrimitiveValue& value) {
return clampTo<int>(result, 0, 255);
}
static bool ParseRGBParameters(CSSParserTokenRange& range,
RGBA32& result,
bool parse_alpha) {
static bool ParseRGBParameters(CSSParserTokenRange& range, RGBA32& result) {
DCHECK(range.Peek().FunctionId() == CSSValueRgb ||
range.Peek().FunctionId() == CSSValueRgba);
CSSParserTokenRange args = ConsumeFunction(range);
......@@ -564,21 +562,37 @@ static bool ParseRGBParameters(CSSParserTokenRange& range,
const bool is_percent = color_parameter->IsPercentage();
int color_array[3];
color_array[0] = ClampRGBComponent(*color_parameter);
bool requires_commas = false;
for (int i = 1; i < 3; i++) {
if (!ConsumeCommaIncludingWhitespace(args))
if (ConsumeCommaIncludingWhitespace(args)) {
if (i != 1 && !requires_commas)
return false;
requires_commas = true;
} else if (requires_commas ||
(&args.Peek() - 1)->GetType() != kWhitespaceToken) {
return false;
}
color_parameter = is_percent ? ConsumePercent(args, kValueRangeAll)
: ConsumeInteger(args);
if (!color_parameter)
return false;
color_array[i] = ClampRGBComponent(*color_parameter);
}
if (parse_alpha) {
if (!ConsumeCommaIncludingWhitespace(args))
return false;
bool comma_consumed = ConsumeCommaIncludingWhitespace(args);
bool slash_consumed = ConsumeSlashIncludingWhitespace(args);
if ((comma_consumed && !requires_commas) ||
(slash_consumed && requires_commas))
return false;
if (comma_consumed || slash_consumed) {
double alpha;
if (!ConsumeNumberRaw(args, alpha))
return false;
if (!ConsumeNumberRaw(args, alpha)) {
CSSPrimitiveValue* alpha_percent = ConsumePercent(args, kValueRangeAll);
if (!alpha_percent)
return false;
else
alpha = alpha_percent->GetDoubleValue() / 100.0f;
}
// W3 standard stipulates a 2.55 alpha value multiplication factor.
int alpha_component =
static_cast<int>(lroundf(clampTo<double>(alpha, 0.0, 1.0) * 255.0f));
......@@ -665,7 +679,7 @@ static bool ParseColorFunction(CSSParserTokenRange& range, RGBA32& result) {
return false;
CSSParserTokenRange color_range = range;
if ((function_id <= CSSValueRgba &&
!ParseRGBParameters(color_range, result, function_id == CSSValueRgba)) ||
!ParseRGBParameters(color_range, result)) ||
(function_id >= CSSValueHsl &&
!ParseHSLParameters(color_range, result, function_id == CSSValueHsla)))
return false;
......
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