LibWeb/CSS: Correct how we evaluate boolean media-features

The spec has a general rule for this, which is roughly that "If it's not
a falsey value, it's true". However, a couple of media-features are
always false, apparently breaking this rule. To handle that, we have an
array of false keywords in the JSON, instead of a single keyword. For
those always-false media-features, we can enter all their values into
this array.

Gets us 2 more WPT subtest passes.
This commit is contained in:
Sam Atkins 2025-05-22 17:02:44 +01:00
commit fb975cc156
Notes: github-actions[bot] 2025-05-23 09:18:59 +00:00
5 changed files with 107 additions and 14 deletions

View file

@ -216,10 +216,11 @@ They are listed in the [`@media` descriptor table](https://www.w3.org/TR/mediaqu
The definitions here are like a simplified version of the `Properties.json` definitions. The definitions here are like a simplified version of the `Properties.json` definitions.
| Field | Description | | Field | Description |
|----------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| |------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `type` | String. How the media-feature is evaluated, either `discrete` or `range`. | | `type` | String. How the media-feature is evaluated, either `discrete` or `range`. |
| `values` | Array of strings. These are directly taken from the spec, with keywords as they are, and `<>` around type names. Types may be `<boolean>`, `<integer>`, `<length>`, `<ratio>`, or `<resolution>`. | | `values` | Array of strings. These are directly taken from the spec, with keywords as they are, and `<>` around type names. Types may be `<boolean>`, `<integer>`, `<length>`, `<ratio>`, or `<resolution>`. |
| `false-keywords` | Array of strings. These are any keywords that should be considered false when the media feature is evaluated as `@media (foo)`. Generally this will be a single value, such as `"none"`. |
The generated code provides: The generated code provides:
- A `MediaFeatureValueType` enum listing the possible value types - A `MediaFeatureValueType` enum listing the possible value types
@ -229,6 +230,7 @@ The generated code provides:
- `bool media_feature_type_is_range(MediaFeatureID)` returns whether the media feature is a `range` type, as opposed to a `discrete` type - `bool media_feature_type_is_range(MediaFeatureID)` returns whether the media feature is a `range` type, as opposed to a `discrete` type
- `bool media_feature_accepts_type(MediaFeatureID, MediaFeatureValueType)` returns whether the media feature will accept values of this type - `bool media_feature_accepts_type(MediaFeatureID, MediaFeatureValueType)` returns whether the media feature will accept values of this type
- `bool media_feature_accepts_keyword(MediaFeatureID, Keyword)` returns whether the media feature accepts this keyword - `bool media_feature_accepts_keyword(MediaFeatureID, Keyword)` returns whether the media feature accepts this keyword
- `bool media_feature_keyword_is_falsey(MediaFeatureID, Keyword)` returns whether the given keyword is considered false when the media-feature is evaluated in a boolean context. (Like `@media (foo)`)
## MathFunctions.json ## MathFunctions.json

View file

@ -4,6 +4,9 @@
"values": [ "values": [
"none", "none",
"hover" "hover"
],
"false-keywords": [
"none"
] ]
}, },
"any-pointer": { "any-pointer": {
@ -12,6 +15,9 @@
"none", "none",
"coarse", "coarse",
"fine" "fine"
],
"false-keywords": [
"none"
] ]
}, },
"aspect-ratio": { "aspect-ratio": {
@ -73,6 +79,11 @@
"values": [ "values": [
"standard", "standard",
"high" "high"
],
"FIXME": "This always evaluating to false in a boolean context seems like a spec bug. https://github.com/w3c/csswg-drafts/issues/8050",
"false-keywords": [
"standard",
"high"
] ]
}, },
"environment-blending": { "environment-blending": {
@ -88,6 +99,9 @@
"values": [ "values": [
"none", "none",
"active" "active"
],
"false-keywords": [
"none"
] ]
}, },
"grid": { "grid": {
@ -113,6 +127,9 @@
"values": [ "values": [
"none", "none",
"hover" "hover"
],
"false-keywords": [
"none"
] ]
}, },
"inverted-colors": { "inverted-colors": {
@ -120,6 +137,9 @@
"values": [ "values": [
"none", "none",
"inverted" "inverted"
],
"false-keywords": [
"none"
] ]
}, },
"monochrome": { "monochrome": {
@ -133,6 +153,9 @@
"values": [ "values": [
"none", "none",
"back" "back"
],
"false-keywords": [
"none"
] ]
}, },
"orientation": { "orientation": {
@ -148,6 +171,9 @@
"none", "none",
"scroll", "scroll",
"paged" "paged"
],
"false-keywords": [
"none"
] ]
}, },
"overflow-inline": { "overflow-inline": {
@ -155,6 +181,9 @@
"values": [ "values": [
"none", "none",
"scroll" "scroll"
],
"false-keywords": [
"none"
] ]
}, },
"pointer": { "pointer": {
@ -163,6 +192,9 @@
"none", "none",
"coarse", "coarse",
"fine" "fine"
],
"false-keywords": [
"none"
] ]
}, },
"prefers-color-scheme": { "prefers-color-scheme": {
@ -179,6 +211,9 @@
"less", "less",
"more", "more",
"custom" "custom"
],
"false-keywords": [
"no-preference"
] ]
}, },
"prefers-reduced-data": { "prefers-reduced-data": {
@ -186,6 +221,9 @@
"values": [ "values": [
"no-preference", "no-preference",
"reduce" "reduce"
],
"false-keywords": [
"no-preference"
] ]
}, },
"prefers-reduced-motion": { "prefers-reduced-motion": {
@ -193,6 +231,9 @@
"values": [ "values": [
"no-preference", "no-preference",
"reduce" "reduce"
],
"false-keywords": [
"no-preference"
] ]
}, },
"prefers-reduced-transparency": { "prefers-reduced-transparency": {
@ -200,6 +241,9 @@
"values": [ "values": [
"no-preference", "no-preference",
"reduce" "reduce"
],
"false-keywords": [
"no-preference"
] ]
}, },
"resolution": { "resolution": {
@ -214,6 +258,9 @@
"values": [ "values": [
"interlace", "interlace",
"progressive" "progressive"
],
"false-keywords": [
"none"
] ]
}, },
"scripting": { "scripting": {
@ -222,6 +269,9 @@
"none", "none",
"initial-only", "initial-only",
"enabled" "enabled"
],
"false-keywords": [
"none"
] ]
}, },
"update": { "update": {
@ -230,6 +280,9 @@
"none", "none",
"slow", "slow",
"fast" "fast"
],
"false-keywords": [
"none"
] ]
}, },
"vertical-viewport-segments": { "vertical-viewport-segments": {
@ -251,6 +304,11 @@
"values": [ "values": [
"standard", "standard",
"high" "high"
],
"FIXME": "See dynamic-range",
"false-keywords": [
"standard",
"high"
] ]
}, },
"width": { "width": {

View file

@ -121,12 +121,9 @@ MatchResult MediaFeature::evaluate(HTML::Window const* window) const
if (queried_value.is_resolution()) if (queried_value.is_resolution())
return as_match_result(queried_value.resolution().resolved(calculation_context).map([](auto& it) { return it.to_dots_per_pixel(); }).value_or(0) != 0); return as_match_result(queried_value.resolution().resolved(calculation_context).map([](auto& it) { return it.to_dots_per_pixel(); }).value_or(0) != 0);
if (queried_value.is_ident()) { if (queried_value.is_ident()) {
// NOTE: It is not technically correct to always treat `no-preference` as false, but every if (media_feature_keyword_is_falsey(m_id, queried_value.ident()))
// media-feature that accepts it as a value treats it as false, so good enough. :^) return MatchResult::False;
// If other features gain this property for other keywords in the future, we can return MatchResult::True;
// add more robust handling for them then.
return as_match_result(queried_value.ident() != Keyword::None
&& queried_value.ident() != Keyword::NoPreference);
} }
return MatchResult::False; return MatchResult::False;

View file

@ -80,6 +80,8 @@ bool media_feature_type_is_range(MediaFeatureID);
bool media_feature_accepts_type(MediaFeatureID, MediaFeatureValueType); bool media_feature_accepts_type(MediaFeatureID, MediaFeatureValueType);
bool media_feature_accepts_keyword(MediaFeatureID, Keyword); bool media_feature_accepts_keyword(MediaFeatureID, Keyword);
bool media_feature_keyword_is_falsey(MediaFeatureID, Keyword);
} }
)~~~"); )~~~");
@ -290,6 +292,41 @@ bool media_feature_accepts_keyword(MediaFeatureID media_feature_id, Keyword keyw
VERIFY_NOT_REACHED(); VERIFY_NOT_REACHED();
} }
bool media_feature_keyword_is_falsey(MediaFeatureID media_feature_id, Keyword keyword)
{
switch (media_feature_id) {)~~~");
media_feature_data.for_each_member([&](auto& name, JsonValue const& feature_value) {
VERIFY(feature_value.is_object());
auto& feature = feature_value.as_object();
auto false_keywords = feature.get_array("false-keywords"sv);
if (!false_keywords.has_value() || false_keywords->is_empty())
return;
auto member_generator = generator.fork();
member_generator.set("name:titlecase", title_casify(name));
member_generator.append(R"~~~(
case MediaFeatureID::@name:titlecase@:
switch (keyword) {)~~~");
false_keywords.value().for_each([&](JsonValue const& value) {
auto value_generator = member_generator.fork();
member_generator.set("false_keyword:titlecase", title_casify(value.as_string()));
member_generator.append(R"~~~(
case Keyword::@false_keyword:titlecase@:)~~~");
});
member_generator.append(R"~~~(
return true;
default:
return false;
})~~~");
});
generator.append(R"~~~(
default:
return false;
}
}
} }
)~~~"); )~~~");

View file

@ -2,8 +2,7 @@ Harness status: OK
Found 23 tests Found 23 tests
21 Pass 23 Pass
2 Fail
Pass Should be known: '(dynamic-range: standard)' Pass Should be known: '(dynamic-range: standard)'
Pass Should be known: '(dynamic-range: high)' Pass Should be known: '(dynamic-range: high)'
Pass Should be known: '(video-dynamic-range: standard)' Pass Should be known: '(video-dynamic-range: standard)'
@ -22,8 +21,8 @@ Pass Should be parseable: '(video-dynamic-range: 10px)'
Pass Should be unknown: '(video-dynamic-range: 10px)' Pass Should be unknown: '(video-dynamic-range: 10px)'
Pass Should be parseable: '(video-dynamic-range: invalid)' Pass Should be parseable: '(video-dynamic-range: invalid)'
Pass Should be unknown: '(video-dynamic-range: invalid)' Pass Should be unknown: '(video-dynamic-range: invalid)'
Fail Check that dynamic-range evaluates to false in the boolean context Pass Check that dynamic-range evaluates to false in the boolean context
Fail Check that video-dynamic-range evaluates to false in the boolean context Pass Check that video-dynamic-range evaluates to false in the boolean context
Pass Check that dynamic-range always matches 'standard' Pass Check that dynamic-range always matches 'standard'
Pass Check that video-dynamic-range always matches 'standard' Pass Check that video-dynamic-range always matches 'standard'
Pass Check that video-dynamic-range is not 'invalid' Pass Check that video-dynamic-range is not 'invalid'