Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LibWeb: Store CSS color name in CSSRGB #2551

Merged
merged 1 commit into from
Nov 25, 2024

Conversation

milotier
Copy link
Contributor

When serializing an sRGB color value that originated from a named color, it should simply return the color name. This requires storing the color name (if it has one).

This fixes 2 WPT subtests of https://wpt.fyi/results/domparsing/style_attribute_html.html?label=master&product=ladybird.

This is the easiest way I could find to store the original color name, though there is also the option of storing an enum.

@milotier milotier requested a review from AtkinsSJ as a code owner November 24, 2024 18:27
Copy link
Member

@tcl3 tcl3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change fixes a number of WPT tests in /css/cssom. The current change also causes some WPT regressions. I believe the majority of those are caused by the behavior of getComputedStyle() not being correct.

Libraries/LibWeb/CSS/StyleValues/CSSRGB.cpp Outdated Show resolved Hide resolved
Libraries/LibWeb/CSS/StyleValues/CSSColorValue.cpp Outdated Show resolved Hide resolved
@milotier milotier force-pushed the wpt-style-attribute branch 2 times, most recently from 1280920 to 45b7fd8 Compare November 25, 2024 02:20
When serializing an sRGB color value that originated from a named color,
it should return the color name converted to ASCII lowercase. This
requires storing the color name (if it has one).

This change also requires explicitly removing the color names when
computing style, because computed color values do not retain their name.
It also requires removing a caching optimization in create_from_color(),
because adding the name means that the cached value might be wrong.

This fixes some WPT subtests, and also required updating some of our own
tests.
@milotier milotier force-pushed the wpt-style-attribute branch from 45b7fd8 to 70e9cc2 Compare November 25, 2024 08:23
@milotier
Copy link
Contributor Author

The rest of the "failing" tests were actually correct, the expected output just had to be updated.

@awesomekling awesomekling merged commit 6bb8bf1 into LadybirdBrowser:master Nov 25, 2024
6 checks passed
@milotier milotier deleted the wpt-style-attribute branch November 25, 2024 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants