Skip to content

Commit

Permalink
Don't parse color functions with missing parameters.
Browse files Browse the repository at this point in the history 
From ch..@w3.org:
"""
This used to be correct per spec, long ago when color() also did
custom color spaces and it was reasonable to omit some components and have them auto-filled with zero.

Now custom color spaces are in CSS Color 5, and clearly distinguished
with a dashed-ident; while predefined color spaces are in CSS Color 4
and are wither an RGB space or an XYZ space. Both take 3 components, as
the grammar makes clear.

There was some leftover prose about variable number of components, not up to date with the grammar change, which I just corrected.
"""

Bug: 1410200
Change-Id: I6e1e7d49b91d91a1c61be95a837555f861a5478c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4198199
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Aaron Krajeski <aaronhk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1098869}
  • Loading branch information
@mysteryDate @marcoscaceres
mysteryDate authored and marcoscaceres committed Mar 28, 2023
1 parent 3fef9e4 commit 764dd5b
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 27 deletions.
15 changes: 0 additions & 15 deletions css/css-color/parsing/color-computed-color-function.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,6 @@
test_computed_value("color", `color(${colorSpace} 0 0 0 / -10%)`, `color(${colorSpace} 0 0 0 / 0)`);
test_computed_value("color", `color(${colorSpace} 0 0 0 / 110%)`, `color(${colorSpace} 0 0 0)`);
test_computed_value("color", `color(${colorSpace} 0 0 0 / 300%)`, `color(${colorSpace} 0 0 0)`);
test_computed_value("color", `color(${colorSpace} 50% -200)`, `color(${colorSpace} 0.5 -200 0)`);
test_computed_value("color", `color(${colorSpace} 50%)`, `color(${colorSpace} 0.5 0 0)`);
test_computed_value("color", `color(${colorSpace})`, `color(${colorSpace} 0 0 0)`);
test_computed_value("color", `color(${colorSpace} 50% -200 / 0.5)`, `color(${colorSpace} 0.5 -200 0 / 0.5)`);
test_computed_value("color", `color(${colorSpace} 50% / 0.5)`, `color(${colorSpace} 0.5 0 0 / 0.5)`);
test_computed_value("color", `color(${colorSpace} / 0.5)`, `color(${colorSpace} 0 0 0 / 0.5)`);
test_computed_value("color", `color(${colorSpace} 200 200 200)`, `color(${colorSpace} 200 200 200)`);
test_computed_value("color", `color(${colorSpace} 200 200 200 / 200)`, `color(${colorSpace} 200 200 200)`);
test_computed_value("color", `color(${colorSpace} -200 -200 -200)`, `color(${colorSpace} -200 -200 -200)`);
Expand Down Expand Up @@ -71,12 +65,6 @@
test_computed_value("color", `color(${colorSpace} 0 0 0 / -10%)`, `color(${resultColorSpace} 0 0 0 / 0)`);
test_computed_value("color", `color(${colorSpace} 0 0 0 / 110%)`, `color(${resultColorSpace} 0 0 0)`);
test_computed_value("color", `color(${colorSpace} 0 0 0 / 300%)`, `color(${resultColorSpace} 0 0 0)`);
test_computed_value("color", `color(${colorSpace} 1 1)`, `color(${resultColorSpace} 1 1 0)`);
test_computed_value("color", `color(${colorSpace} 1)`, `color(${resultColorSpace} 1 0 0)`);
test_computed_value("color", `color(${colorSpace})`, `color(${resultColorSpace} 0 0 0)`);
test_computed_value("color", `color(${colorSpace} 1 1 / .5)`, `color(${resultColorSpace} 1 1 0 / 0.5)`);
test_computed_value("color", `color(${colorSpace} 1 / 0.5)`, `color(${resultColorSpace} 1 0 0 / 0.5)`);
test_computed_value("color", `color(${colorSpace} / 50%)`, `color(${resultColorSpace} 0 0 0 / 0.5)`);
test_computed_value("color", `color(${colorSpace} calc(0.5 + 1) calc(0.5 - 1) calc(0.5) / calc(-0.5 + 1))`, `color(${resultColorSpace} 1.5 -0.5 0.5 / 0.5)`);

test_computed_value("color", `color(${colorSpace} none none none / none)`, `color(${resultColorSpace} none none none / none)`);
Expand Down Expand Up @@ -276,9 +264,6 @@
test_computed_value("color", "color(srgb 1 1 1 / 0)", "color(srgb 1 1 1 / 0)", "[sRGB white with 0 alpha]");
test_computed_value("color", "color(srgb 1 1 1 / 50%)", "color(srgb 1 1 1 / 0.5)", "[sRGB white with 50% alpha]");
test_computed_value("color", "color(srgb 1 1 1 / 0%)", "color(srgb 1 1 1 / 0)", "[sRGB white with 0% alpha]");
test_computed_value("color", "color(srgb 1 1)", "color(srgb 1 1 0)", "[One missing component is 0]");
test_computed_value("color", "color(srgb 1)", "color(srgb 1 0 0)", "[Two missing components are 0]");
test_computed_value("color", "color(srgb)", "color(srgb 0 0 0)", "[All components missing]");
test_computed_value("color", "color(display-p3 0.6 0.7 0.8)", "color(display-p3 0.6 0.7 0.8)", "[Display P3 color]");
test_computed_value("color", "color(dIspLaY-P3 0.6 0.7 0.8)", "color(display-p3 0.6 0.7 0.8)", "[Different case for Display P3]");

Expand Down
14 changes: 14 additions & 0 deletions css/css-color/parsing/color-invalid-color-function.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@
test_invalid_value("color", `color(${colorSpace} 0% 0 0deg)`);
test_invalid_value("color", `color(${colorSpace} 0% 0% 0deg)`);
test_invalid_value("color", `color(${colorSpace} 40% 0 0deg)`);
// Missing parameters should not parse
test_invalid_value("color", `color(${colorSpace} 50% -200)`);
test_invalid_value("color", `color(${colorSpace} 50%)`);
test_invalid_value("color", `color(${colorSpace})`);
test_invalid_value("color", `color(${colorSpace} 50% -200 / 0.5)`);
test_invalid_value("color", `color(${colorSpace} 50% / 0.5)`);
test_invalid_value("color", `color(${colorSpace} / 0.5)`);
}

for (const colorSpace of [ "xyz", "xyz-d50", "xyz-d65" ]) {
Expand All @@ -30,6 +37,13 @@
test_invalid_value("color", `color(${colorSpace} 0% 0 0deg)`);
test_invalid_value("color", `color(${colorSpace} 0% 0% 0deg)`);
test_invalid_value("color", `color(${colorSpace} 40% 0 0deg)`);
// Missing parameters should not parse
test_invalid_value("color", `color(${colorSpace} 1 1)`);
test_invalid_value("color", `color(${colorSpace} 1)`);
test_invalid_value("color", `color(${colorSpace})`);
test_invalid_value("color", `color(${colorSpace} 1 1 / .5)`);
test_invalid_value("color", `color(${colorSpace} 1 / 0.5)`);
test_invalid_value("color", `color(${colorSpace} / 50%)`);
}

test_invalid_value("color", "color()"); // Empty
Expand Down
12 changes: 0 additions & 12 deletions css/css-color/parsing/color-valid-color-function.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,6 @@
test_valid_value("color", `color(${colorSpace} 0 0 0 / -10%)`, `color(${colorSpace} 0 0 0 / 0)`);
test_valid_value("color", `color(${colorSpace} 0 0 0 / 110%)`, `color(${colorSpace} 0 0 0)`);
test_valid_value("color", `color(${colorSpace} 0 0 0 / 300%)`, `color(${colorSpace} 0 0 0)`);
test_valid_value("color", `color(${colorSpace} 50% -200)`, `color(${colorSpace} 0.5 -200 0)`);
test_valid_value("color", `color(${colorSpace} 50%)`, `color(${colorSpace} 0.5 0 0)`);
test_valid_value("color", `color(${colorSpace})`, `color(${colorSpace} 0 0 0)`);
test_valid_value("color", `color(${colorSpace} 50% -200 / 0.5)`, `color(${colorSpace} 0.5 -200 0 / 0.5)`);
test_valid_value("color", `color(${colorSpace} 50% / 0.5)`, `color(${colorSpace} 0.5 0 0 / 0.5)`);
test_valid_value("color", `color(${colorSpace} / 0.5)`, `color(${colorSpace} 0 0 0 / 0.5)`);
test_valid_value("color", `color(${colorSpace} 200 200 200)`, `color(${colorSpace} 200 200 200)`);
test_valid_value("color", `color(${colorSpace} 200 200 200 / 200)`, `color(${colorSpace} 200 200 200)`);
test_valid_value("color", `color(${colorSpace} -200 -200 -200)`, `color(${colorSpace} -200 -200 -200)`);
Expand Down Expand Up @@ -66,12 +60,6 @@
test_valid_value("color", `color(${colorSpace} 0 0 0 / -10%)`, `color(${resultColorSpace} 0 0 0 / 0)`);
test_valid_value("color", `color(${colorSpace} 0 0 0 / 110%)`, `color(${resultColorSpace} 0 0 0)`);
test_valid_value("color", `color(${colorSpace} 0 0 0 / 300%)`, `color(${resultColorSpace} 0 0 0)`);
test_valid_value("color", `color(${colorSpace} 1 1)`, `color(${resultColorSpace} 1 1 0)`);
test_valid_value("color", `color(${colorSpace} 1)`, `color(${resultColorSpace} 1 0 0)`);
test_valid_value("color", `color(${colorSpace})`, `color(${resultColorSpace} 0 0 0)`);
test_valid_value("color", `color(${colorSpace} 1 1 / .5)`, `color(${resultColorSpace} 1 1 0 / 0.5)`);
test_valid_value("color", `color(${colorSpace} 1 / 0.5)`, `color(${resultColorSpace} 1 0 0 / 0.5)`);
test_valid_value("color", `color(${colorSpace} / 50%)`, `color(${resultColorSpace} 0 0 0 / 0.5)`);
test_valid_value("color", `color(${colorSpace} calc(0.5 + 1) calc(0.5 - 1) calc(0.5) / calc(-0.5 + 1))`, `color(${resultColorSpace} 1.5 -0.5 0.5 / 0.5)`);

test_valid_value("color", `color(${colorSpace} none none none / none)`, `color(${resultColorSpace} none none none / none)`);
Expand Down

0 comments on commit 764dd5b

Please sign in to comment.