LibWeb/CSS: Maintain original form of media-feature ranges

Previously, for `foo < 30px` ranges, we'd flip them and store them as
`30px > foo` instead. That worked fine, but means the serialization is
wrong. So instead, keep them in their original form.

I experimented with giving Range two optional sub-structs instead of 4
optional members, thinking it would be smaller - but it's actually
larger, because the two Optional<Comparison>s fit snugly together. So,
the slightly-goofy all-Optionals remains.

This gets us 2 WPT passes that I'm aware of.
This commit is contained in:
Sam Atkins 2025-05-22 12:50:03 +01:00
commit 9fe8445946
Notes: github-actions[bot] 2025-05-23 09:19:11 +00:00
5 changed files with 28 additions and 32 deletions

View file

@ -82,10 +82,14 @@ String MediaFeature::to_string() const
return MUST(String::formatted("max-{}: {}", string_from_media_feature_id(m_id), value().to_string())); return MUST(String::formatted("max-{}: {}", string_from_media_feature_id(m_id), value().to_string()));
case Type::Range: { case Type::Range: {
auto& range = this->range(); auto& range = this->range();
if (!range.right_comparison.has_value()) StringBuilder builder;
return MUST(String::formatted("{} {} {}", range.left_value.to_string(), comparison_string(range.left_comparison), string_from_media_feature_id(m_id))); if (range.left_comparison.has_value())
builder.appendff("{} {} ", range.left_value->to_string(), comparison_string(*range.left_comparison));
builder.append(string_from_media_feature_id(m_id));
if (range.right_comparison.has_value())
builder.appendff(" {} {}", comparison_string(*range.right_comparison), range.right_value->to_string());
return MUST(String::formatted("{} {} {} {} {}", range.left_value.to_string(), comparison_string(range.left_comparison), string_from_media_feature_id(m_id), comparison_string(*range.right_comparison), range.right_value->to_string())); return builder.to_string_without_validation();
} }
} }
@ -137,12 +141,15 @@ MatchResult MediaFeature::evaluate(HTML::Window const* window) const
case Type::Range: { case Type::Range: {
auto const& range = this->range(); auto const& range = this->range();
if (auto const left_result = compare(*window, range.left_value, range.left_comparison, queried_value); left_result != MatchResult::True) if (range.left_comparison.has_value()) {
return left_result; if (auto const left_result = compare(*window, *range.left_value, *range.left_comparison, queried_value); left_result != MatchResult::True)
return left_result;
}
if (range.right_comparison.has_value()) if (range.right_comparison.has_value()) {
if (auto const right_result = compare(*window, queried_value, *range.right_comparison, *range.right_value); right_result != MatchResult::True) if (auto const right_result = compare(*window, queried_value, *range.right_comparison, *range.right_value); right_result != MatchResult::True)
return right_result; return right_result;
}
return MatchResult::True; return MatchResult::True;
} }

View file

@ -132,7 +132,6 @@ public:
return adopt_own(*new MediaFeature(Type::MaxValue, id, move(value))); return adopt_own(*new MediaFeature(Type::MaxValue, id, move(value)));
} }
// Corresponds to `<mf-range>` grammar, with a single comparison
static NonnullOwnPtr<MediaFeature> half_range(MediaFeatureValue value, Comparison comparison, MediaFeatureID id) static NonnullOwnPtr<MediaFeature> half_range(MediaFeatureValue value, Comparison comparison, MediaFeatureID id)
{ {
return adopt_own(*new MediaFeature(Type::Range, id, return adopt_own(*new MediaFeature(Type::Range, id,
@ -141,6 +140,14 @@ public:
.left_comparison = comparison, .left_comparison = comparison,
})); }));
} }
static NonnullOwnPtr<MediaFeature> half_range(MediaFeatureID id, Comparison comparison, MediaFeatureValue value)
{
return adopt_own(*new MediaFeature(Type::Range, id,
Range {
.right_comparison = comparison,
.right_value = move(value),
}));
}
// Corresponds to `<mf-range>` grammar, with two comparisons // Corresponds to `<mf-range>` grammar, with two comparisons
static NonnullOwnPtr<MediaFeature> range(MediaFeatureValue left_value, Comparison left_comparison, MediaFeatureID id, Comparison right_comparison, MediaFeatureValue right_value) static NonnullOwnPtr<MediaFeature> range(MediaFeatureValue left_value, Comparison left_comparison, MediaFeatureID id, Comparison right_comparison, MediaFeatureValue right_value)
@ -168,8 +175,8 @@ private:
}; };
struct Range { struct Range {
MediaFeatureValue left_value; Optional<MediaFeatureValue> left_value {};
Comparison left_comparison; Optional<Comparison> left_comparison {};
Optional<Comparison> right_comparison {}; Optional<Comparison> right_comparison {};
Optional<MediaFeatureValue> right_value {}; Optional<MediaFeatureValue> right_value {};
}; };

View file

@ -274,22 +274,6 @@ OwnPtr<MediaFeature> Parser::parse_media_feature(TokenStream<ComponentValue>& to
return {}; return {};
}; };
auto flip = [](MediaFeature::Comparison comparison) {
switch (comparison) {
case MediaFeature::Comparison::Equal:
return MediaFeature::Comparison::Equal;
case MediaFeature::Comparison::LessThan:
return MediaFeature::Comparison::GreaterThan;
case MediaFeature::Comparison::LessThanOrEqual:
return MediaFeature::Comparison::GreaterThanOrEqual;
case MediaFeature::Comparison::GreaterThan:
return MediaFeature::Comparison::LessThan;
case MediaFeature::Comparison::GreaterThanOrEqual:
return MediaFeature::Comparison::LessThanOrEqual;
}
VERIFY_NOT_REACHED();
};
auto comparisons_match = [](MediaFeature::Comparison a, MediaFeature::Comparison b) -> bool { auto comparisons_match = [](MediaFeature::Comparison a, MediaFeature::Comparison b) -> bool {
switch (a) { switch (a) {
case MediaFeature::Comparison::Equal: case MediaFeature::Comparison::Equal:
@ -322,7 +306,7 @@ OwnPtr<MediaFeature> Parser::parse_media_feature(TokenStream<ComponentValue>& to
tokens.discard_whitespace(); tokens.discard_whitespace();
if (!tokens.has_next_token() && !maybe_value->is_ident()) { if (!tokens.has_next_token() && !maybe_value->is_ident()) {
transaction.commit(); transaction.commit();
return MediaFeature::half_range(maybe_value.release_value(), flip(maybe_comparison.release_value()), maybe_name->id); return MediaFeature::half_range(maybe_name->id, maybe_comparison.release_value(), maybe_value.release_value());
} }
} }
} }

View file

@ -2,7 +2,6 @@ Harness status: OK
Found 2 tests Found 2 tests
1 Pass 2 Pass
1 Fail
Pass Empty CSSNestedDeclarations do not affect outer serialization Pass Empty CSSNestedDeclarations do not affect outer serialization
Fail Empty CSSNestedDeclarations do not affect outer serialization (nested grouping rule) Pass Empty CSSNestedDeclarations do not affect outer serialization (nested grouping rule)

View file

@ -2,8 +2,7 @@ Harness status: OK
Found 26 tests Found 26 tests
25 Pass 26 Pass
1 Fail
Pass Should be known: '(prefers-contrast)' Pass Should be known: '(prefers-contrast)'
Pass Should be known: '(prefers-contrast: no-preference)' Pass Should be known: '(prefers-contrast: no-preference)'
Pass Should be known: '(prefers-contrast: more)' Pass Should be known: '(prefers-contrast: more)'
@ -17,7 +16,7 @@ Pass Should be parseable: '(prefers-contrast: forced high)'
Pass Should be unknown: '(prefers-contrast: forced high)' Pass Should be unknown: '(prefers-contrast: forced high)'
Pass Should be parseable: '(prefers-contrast: forced low)' Pass Should be parseable: '(prefers-contrast: forced low)'
Pass Should be unknown: '(prefers-contrast: forced low)' Pass Should be unknown: '(prefers-contrast: forced low)'
Fail Should be parseable: '(prefers-contrast > increase)' Pass Should be parseable: '(prefers-contrast > increase)'
Pass Should be unknown: '(prefers-contrast > increase)' Pass Should be unknown: '(prefers-contrast > increase)'
Pass Should be parseable: '(prefers-increased-contrast)' Pass Should be parseable: '(prefers-increased-contrast)'
Pass Should be unknown: '(prefers-increased-contrast)' Pass Should be unknown: '(prefers-increased-contrast)'