mirror of
https://github.com/LadybirdBrowser/ladybird.git
synced 2025-01-23 09:46:04 -05:00
LibWeb: Store CSS color name in CSSRGB
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.
This commit is contained in:
Milo van der Tier
Andreas Kling
parent
ff311c1560
commit
6bb8bf189f
Notes:
github-actions[bot]
2024-11-25 10:52:53 +00:00
Author: https://github.com/milotier Commit: https://github.com/LadybirdBrowser/ladybird/commit/6bb8bf189f2 Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/2551 Reviewed-by: https://github.com/tcl3
16 changed files with 133 additions and 69 deletions
|
|
@ -3413,7 +3413,7 @@ RefPtr<CSSStyleValue> Parser::parse_color_value(TokenStream<ComponentValue>& tok
|
|||
auto color = Color::from_string(ident);
|
||||
if (color.has_value()) {
|
||||
transaction.commit();
|
||||
return CSSColorValue::create_from_color(color.release_value());
|
||||
return CSSColorValue::create_from_color(color.release_value(), ident);
|
||||
}
|
||||
// Otherwise, fall through to the hashless-hex-color case
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2425,6 +2425,17 @@ Optional<StyleProperties> StyleComputer::compute_style_impl(DOM::Element& elemen
|
|||
start_needed_transitions(*previous_style, style, element, pseudo_element);
|
||||
}
|
||||
|
||||
// Remove color names from CSS color values. This is needed because computed values cannot be named colors.
|
||||
for (auto i = to_underlying(CSS::first_property_id); i <= to_underlying(CSS::last_property_id); ++i) {
|
||||
auto property_id = (CSS::PropertyID)i;
|
||||
auto* property = style.maybe_null_property(property_id);
|
||||
if (property && property->is_color()) {
|
||||
auto& color_value = property->as_color();
|
||||
if (color_value.color_type() == CSSColorValue::ColorType::RGB)
|
||||
style.set_property(property_id, CSSColorValue::create_from_color(color_value.to_color({})));
|
||||
}
|
||||
}
|
||||
|
||||
return style;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -17,32 +17,14 @@
|
|||
|
||||
namespace Web::CSS {
|
||||
|
||||
ValueComparingNonnullRefPtr<CSSColorValue> CSSColorValue::create_from_color(Color color)
|
||||
ValueComparingNonnullRefPtr<CSSColorValue> CSSColorValue::create_from_color(Color color, Optional<FlyString> name)
|
||||
{
|
||||
auto make_rgb_color = [](Color const& color) {
|
||||
return CSSRGB::create(
|
||||
NumberStyleValue::create(color.red()),
|
||||
NumberStyleValue::create(color.green()),
|
||||
NumberStyleValue::create(color.blue()),
|
||||
NumberStyleValue::create(color.alpha() / 255.0));
|
||||
};
|
||||
|
||||
if (color.value() == 0) {
|
||||
static auto transparent = make_rgb_color(color);
|
||||
return transparent;
|
||||
}
|
||||
|
||||
if (color == Color::from_rgb(0x000000)) {
|
||||
static auto black = make_rgb_color(color);
|
||||
return black;
|
||||
}
|
||||
|
||||
if (color == Color::from_rgb(0xffffff)) {
|
||||
static auto white = make_rgb_color(color);
|
||||
return white;
|
||||
}
|
||||
|
||||
return make_rgb_color(color);
|
||||
NumberStyleValue::create(color.alpha() / 255.0),
|
||||
name);
|
||||
}
|
||||
|
||||
Optional<double> CSSColorValue::resolve_hue(CSSStyleValue const& style_value)
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@
|
|||
|
||||
#pragma once
|
||||
|
||||
#include <AK/FlyString.h>
|
||||
#include <LibGfx/Color.h>
|
||||
#include <LibWeb/CSS/CSSStyleValue.h>
|
||||
|
||||
|
|
@ -17,7 +18,7 @@ namespace Web::CSS {
|
|||
// https://drafts.css-houdini.org/css-typed-om-1/#csscolorvalue
|
||||
class CSSColorValue : public CSSStyleValue {
|
||||
public:
|
||||
static ValueComparingNonnullRefPtr<CSSColorValue> create_from_color(Color color);
|
||||
static ValueComparingNonnullRefPtr<CSSColorValue> create_from_color(Color color, Optional<FlyString> name = {});
|
||||
virtual ~CSSColorValue() override = default;
|
||||
|
||||
virtual bool has_color() const override { return true; }
|
||||
|
|
|
|||
|
|
@ -71,6 +71,8 @@ bool CSSRGB::equals(CSSStyleValue const& other) const
|
|||
String CSSRGB::to_string() const
|
||||
{
|
||||
// FIXME: Do this properly, taking unresolved calculated values into account.
|
||||
if (m_properties.name.has_value())
|
||||
return m_properties.name.value().to_string().to_ascii_lowercase();
|
||||
return serialize_a_srgb_value(to_color({}));
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -14,13 +14,13 @@ namespace Web::CSS {
|
|||
// https://drafts.css-houdini.org/css-typed-om-1/#cssrgb
|
||||
class CSSRGB final : public CSSColorValue {
|
||||
public:
|
||||
static ValueComparingNonnullRefPtr<CSSRGB> create(ValueComparingNonnullRefPtr<CSSStyleValue> r, ValueComparingNonnullRefPtr<CSSStyleValue> g, ValueComparingNonnullRefPtr<CSSStyleValue> b, ValueComparingRefPtr<CSSStyleValue> alpha = {})
|
||||
static ValueComparingNonnullRefPtr<CSSRGB> create(ValueComparingNonnullRefPtr<CSSStyleValue> r, ValueComparingNonnullRefPtr<CSSStyleValue> g, ValueComparingNonnullRefPtr<CSSStyleValue> b, ValueComparingRefPtr<CSSStyleValue> alpha = {}, Optional<FlyString> name = {})
|
||||
{
|
||||
// alpha defaults to 1
|
||||
if (!alpha)
|
||||
return adopt_ref(*new (nothrow) CSSRGB(move(r), move(g), move(b), NumberStyleValue::create(1)));
|
||||
return adopt_ref(*new (nothrow) CSSRGB(move(r), move(g), move(b), NumberStyleValue::create(1), name));
|
||||
|
||||
return adopt_ref(*new (nothrow) CSSRGB(move(r), move(g), move(b), alpha.release_nonnull()));
|
||||
return adopt_ref(*new (nothrow) CSSRGB(move(r), move(g), move(b), alpha.release_nonnull(), name));
|
||||
}
|
||||
virtual ~CSSRGB() override = default;
|
||||
|
||||
|
|
@ -36,9 +36,9 @@ public:
|
|||
virtual bool equals(CSSStyleValue const& other) const override;
|
||||
|
||||
private:
|
||||
CSSRGB(ValueComparingNonnullRefPtr<CSSStyleValue> r, ValueComparingNonnullRefPtr<CSSStyleValue> g, ValueComparingNonnullRefPtr<CSSStyleValue> b, ValueComparingNonnullRefPtr<CSSStyleValue> alpha)
|
||||
CSSRGB(ValueComparingNonnullRefPtr<CSSStyleValue> r, ValueComparingNonnullRefPtr<CSSStyleValue> g, ValueComparingNonnullRefPtr<CSSStyleValue> b, ValueComparingNonnullRefPtr<CSSStyleValue> alpha, Optional<FlyString> name = {})
|
||||
: CSSColorValue(ColorType::RGB)
|
||||
, m_properties { .r = move(r), .g = move(g), .b = move(b), .alpha = move(alpha) }
|
||||
, m_properties { .r = move(r), .g = move(g), .b = move(b), .alpha = move(alpha), .name = name }
|
||||
{
|
||||
}
|
||||
|
||||
|
|
@ -47,6 +47,7 @@ private:
|
|||
ValueComparingNonnullRefPtr<CSSStyleValue> g;
|
||||
ValueComparingNonnullRefPtr<CSSStyleValue> b;
|
||||
ValueComparingNonnullRefPtr<CSSStyleValue> alpha;
|
||||
Optional<FlyString> name;
|
||||
bool operator==(Properties const&) const = default;
|
||||
} m_properties;
|
||||
};
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
style.cssText = background-color: rgb(255, 255, 0); background-image: none; background-position-x: left 0%; background-position-y: top 0%; background-size: auto auto; background-repeat: repeat repeat; background-attachment: scroll; background-origin: padding-box; background-clip: border-box;
|
||||
style.cssText = background-color: yellow; background-image: none; background-position-x: left 0%; background-position-y: top 0%; background-size: auto auto; background-repeat: repeat repeat; background-attachment: scroll; background-origin: padding-box; background-clip: border-box;
|
||||
style.length = 9
|
||||
style[] =
|
||||
1. background-color
|
||||
|
|
|
|||
|
|
@ -1 +1 @@
|
|||
test { --color: red; color: rgb(255, 0, 0); }
|
||||
test { --color: red; color: red; }
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
fooRule: [object CSSKeyframesRule] ~ @keyframes "foo" { 0% { color: rgb(0, 0, 0); } 100% { color: rgb(255, 255, 255); } }
|
||||
fooRule: [object CSSKeyframesRule] ~ @keyframes "foo" { 0% { color: black; } 100% { color: white; } }
|
||||
fooRule.cssRules: [object CSSRuleList]
|
||||
fooRule.cssRules[0]: [object CSSKeyframeRule] ~ 0% { color: rgb(0, 0, 0); }
|
||||
fooRule.cssRules[0]: [object CSSKeyframeRule] ~ 0% { color: black; }
|
||||
fooRule.cssRules[0].parentRule: [object CSSKeyframesRule]
|
||||
fooRule.cssRules[0].parentRule: [object CSSKeyframesRule] ~ @keyframes "foo" { 0% { color: rgb(0, 0, 0); } 100% { color: rgb(255, 255, 255); } }
|
||||
fooRule.cssRules[0].style.parentRule: [object CSSKeyframeRule] ~ 0% { color: rgb(0, 0, 0); }
|
||||
fooRule.cssRules[0].parentRule: [object CSSKeyframesRule] ~ @keyframes "foo" { 0% { color: black; } 100% { color: white; } }
|
||||
fooRule.cssRules[0].style.parentRule: [object CSSKeyframeRule] ~ 0% { color: black; }
|
||||
fooRule.cssRules[0].style.parentRule === fooRule.cssRules[0]: true
|
||||
|
|
|
|||
|
|
@ -28,7 +28,7 @@ Setting flex: ''; becomes...
|
|||
Setting border: '1px solid red'; becomes...
|
||||
border-width: '1px 1px 1px 1px'
|
||||
border-style: 'solid solid solid solid'
|
||||
border-color: 'rgb(255, 0, 0) rgb(255, 0, 0) rgb(255, 0, 0) rgb(255, 0, 0)'
|
||||
border-color: 'red red red red'
|
||||
border: ''
|
||||
e.style.length: 12
|
||||
> [0] border-top-width
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
spanRule: [object CSSStyleRule] ~ span { color: rgb(128, 0, 128); }
|
||||
spanRule.style: [object CSSStyleDeclaration] ~ span { color: rgb(128, 0, 128); }
|
||||
spanRule.style.parentRule: [object CSSStyleRule] ~ span { color: rgb(128, 0, 128); }
|
||||
spanRule: [object CSSStyleRule] ~ span { color: purple; }
|
||||
spanRule.style: [object CSSStyleDeclaration] ~ span { color: purple; }
|
||||
spanRule.style.parentRule: [object CSSStyleRule] ~ span { color: purple; }
|
||||
spanRule.style.parentRule === spanRule: true
|
||||
|
|
|
|||
|
|
@ -6,19 +6,19 @@ Rerun
|
|||
|
||||
Found 13 tests
|
||||
|
||||
1 Pass
|
||||
12 Fail
|
||||
11 Pass
|
||||
2 Fail
|
||||
Details
|
||||
Result Test Name MessageFail CSSStyleRule is a CSSGroupingRule
|
||||
Fail Simple CSSOM manipulation of subrules
|
||||
Fail Simple CSSOM manipulation of subrules 1
|
||||
Fail Simple CSSOM manipulation of subrules 2
|
||||
Fail Simple CSSOM manipulation of subrules 3
|
||||
Fail Simple CSSOM manipulation of subrules 4
|
||||
Fail Simple CSSOM manipulation of subrules 5
|
||||
Fail Simple CSSOM manipulation of subrules 6
|
||||
Fail Simple CSSOM manipulation of subrules 7
|
||||
Pass Simple CSSOM manipulation of subrules
|
||||
Pass Simple CSSOM manipulation of subrules 1
|
||||
Pass Simple CSSOM manipulation of subrules 2
|
||||
Pass Simple CSSOM manipulation of subrules 3
|
||||
Pass Simple CSSOM manipulation of subrules 4
|
||||
Pass Simple CSSOM manipulation of subrules 5
|
||||
Pass Simple CSSOM manipulation of subrules 6
|
||||
Pass Simple CSSOM manipulation of subrules 7
|
||||
Fail Simple CSSOM manipulation of subrules 8
|
||||
Fail Simple CSSOM manipulation of subrules 9
|
||||
Fail Simple CSSOM manipulation of subrules 10
|
||||
Pass Simple CSSOM manipulation of subrules 9
|
||||
Pass Simple CSSOM manipulation of subrules 10
|
||||
Pass Mutating the selectorText of outer rule invalidates inner rules
|
||||
|
|
@ -6,7 +6,8 @@ Rerun
|
|||
|
||||
Found 2 tests
|
||||
|
||||
2 Fail
|
||||
1 Pass
|
||||
1 Fail
|
||||
Details
|
||||
Result Test Name MessageFail Simple CSSOM manipulation of subrules
|
||||
Result Test Name MessagePass Simple CSSOM manipulation of subrules
|
||||
Fail Simple CSSOM manipulation of subrules 1
|
||||
|
|
@ -6,20 +6,19 @@ Rerun
|
|||
|
||||
Found 15 tests
|
||||
|
||||
4 Pass
|
||||
11 Fail
|
||||
15 Pass
|
||||
Details
|
||||
Result Test Name MessageFail Declarations are serialized on one line, rules on two.
|
||||
Fail Mixed declarations/rules are on two lines.
|
||||
Fail Implicit rule is serialized
|
||||
Fail Implicit rule not removed
|
||||
Fail Implicit + empty hover rule
|
||||
Fail Implicit like rule not in first position
|
||||
Fail Two implicit-like rules
|
||||
Fail Implicit like rule after decls
|
||||
Fail Implicit like rule after decls, missing closing braces
|
||||
Fail Implicit like rule with other selectors
|
||||
Fail Implicit-like rule in style rule
|
||||
Result Test Name MessagePass Declarations are serialized on one line, rules on two.
|
||||
Pass Mixed declarations/rules are on two lines.
|
||||
Pass Implicit rule is serialized
|
||||
Pass Implicit rule not removed
|
||||
Pass Implicit + empty hover rule
|
||||
Pass Implicit like rule not in first position
|
||||
Pass Two implicit-like rules
|
||||
Pass Implicit like rule after decls
|
||||
Pass Implicit like rule after decls, missing closing braces
|
||||
Pass Implicit like rule with other selectors
|
||||
Pass Implicit-like rule in style rule
|
||||
Pass Empty conditional rule
|
||||
Pass Empty style rule
|
||||
Pass Empty conditional inside style rule
|
||||
|
|
|
|||
|
|
@ -0,0 +1,15 @@
|
|||
Summary
|
||||
|
||||
Harness status: OK
|
||||
|
||||
Rerun
|
||||
|
||||
Found 4 tests
|
||||
|
||||
2 Pass
|
||||
2 Fail
|
||||
Details
|
||||
Result Test Name MessagePass Parsing of initial style attribute
|
||||
Fail Parsing of invalid style attribute
|
||||
Fail Parsing of style attribute
|
||||
Pass Update style.backgroundColor
|
||||
|
|
@ -0,0 +1,52 @@
|
|||
<!doctype html>
|
||||
<meta charset=utf-8>
|
||||
<title>Style attribute in HTML</title>
|
||||
<script src=../resources/testharness.js></script>
|
||||
<script src=../resources/testharnessreport.js></script>
|
||||
<script>
|
||||
|
||||
var div;
|
||||
setup(function() {
|
||||
var input = '<div style="color: red">Foo</div>';
|
||||
var doc = (new DOMParser()).parseFromString(input, 'text/html');
|
||||
div = doc.querySelector('div');
|
||||
});
|
||||
|
||||
test(function() {
|
||||
var style = div.style;
|
||||
assert_equals(style.cssText, 'color: red;');
|
||||
assert_equals(style.color, 'red');
|
||||
assert_equals(div.getAttribute("style"), 'color: red',
|
||||
'Value of style attribute should match the string value that was set');
|
||||
}, 'Parsing of initial style attribute');
|
||||
|
||||
test(function() {
|
||||
var style = div.style;
|
||||
div.setAttribute('style', 'color:: invalid');
|
||||
assert_equals(style.cssText, '');
|
||||
assert_equals(style.color, '');
|
||||
assert_equals(div.getAttribute('style'), 'color:: invalid',
|
||||
'Value of style attribute should match the string value that was set');
|
||||
}, 'Parsing of invalid style attribute');
|
||||
|
||||
test(function() {
|
||||
var style = div.style;
|
||||
div.setAttribute('style', 'color: green');
|
||||
assert_equals(style.cssText, 'color: green;');
|
||||
assert_equals(style.color, 'green');
|
||||
assert_equals(div.getAttribute('style'), 'color: green',
|
||||
'Value of style attribute should match the string value that was set');
|
||||
}, 'Parsing of style attribute');
|
||||
|
||||
test(function() {
|
||||
var style = div.style;
|
||||
style.backgroundColor = 'blue';
|
||||
assert_equals(style.cssText, 'color: green; background-color: blue;',
|
||||
'Should not drop the existing style');
|
||||
assert_equals(style.color, 'green',
|
||||
'Should not drop the existing style');
|
||||
assert_equals(div.getAttribute('style'), 'color: green; background-color: blue;',
|
||||
'Should update style attribute');
|
||||
}, 'Update style.backgroundColor');
|
||||
|
||||
</script>
|
||||
Loading…
Reference in a new issue