tree 6249976827284fb8401e53dc3d2b07f84277345f
parent 5f0fba3215492e89b9021bf3bfb9dac63e8fd7c9
author Aaron Krajeski <aaronhk@chromium.org> 1695389548 -0700
committer Blink WPT Bot <blink-w3c-test-autoroller@chromium.org> 1695390646 -0700

Clamp out-of-gamut legacy rgb values

Previously legacy rgb color parameters were clamped to [0, 1] (which
is serialized to [0, 255], for legacy reasons) at parse time.
Refactoring the color parser overlooked this, as serialization did
the clamping. This lead to a corner case where out-of-gamut values
could be parsed and stored, and would be serialized as normal. The
out-of-gamut stored values will interact with alpha blending, causing
alpha to seem higher than it is (see bug).

See https://chromium-review.googlesource.com/c/chromium/src/+/4879797
to confirm that the regression test is working as expected.

I removed Color::IsLegacyColor() in favor of
Color::IsLegacyColorSpace(Color::ColorSpace) because they both do
exactly the same thing and the latter is slightly more useful.

I also took out the logic to take care of non-finite values in
serialization of legacy colors as the spec is now explicit that they
should be clamped.

css1/units/color_units.html is an invalid test that was never meant to
have an image baseline. This is made clear if you read the text within
said test. The reason it is failing for this CL is because it was
expecting the very behavior that caused the bug in crbug.com/1483736.
The new test out-of-gamut-legacy-rgb.html verifies this much more
clearly. The image output also only renders half of the page. It is
merely meant to validate that different color inputs should be
rendered the same way within the page. This is already well tested in
WPT, for example:
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/css/css-color/rgb-001.html
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/css/css-color/rgb-002.html

All that stated, I think this test is slowing down development and not
providing any value, so I deleted it. The fact that it already had
four different baselines was a good sign that it was just blindly
being rebaselined.

Bug: 1483736
Change-Id: Ibb707f29638356bb0835903e6a56b73ad0969496
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4878299
Commit-Queue: Aaron Krajeski <aaronhk@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1200189}