From 240536acaa9470db24a4c27dbb08b309444e91fb Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Wed, 27 Aug 2025 17:25:40 +0100 Subject: [PATCH] LibWeb: Remove `auto` length from SourceSet The spec is a bit awkward here: A few algorithms create an "empty" SourceSet, and then assign its source-size value a few steps later, so we have a temporary state with no length. In order to avoid complicating the types with Optional, I've chosen to just assign it to 0px. Previously we used `auto`, but `auto` is not a valid value here - it is used inside the "parse a sizes attribute" algorithm, but that always returns an actual length (or calc). --- Libraries/LibWeb/CSS/Parser/Parser.cpp | 27 +++++++++++++------------- Libraries/LibWeb/CSS/Parser/Parser.h | 2 +- Libraries/LibWeb/HTML/SourceSet.cpp | 4 +++- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/Libraries/LibWeb/CSS/Parser/Parser.cpp b/Libraries/LibWeb/CSS/Parser/Parser.cpp index c1d3fd57e16..5f426719e14 100644 --- a/Libraries/LibWeb/CSS/Parser/Parser.cpp +++ b/Libraries/LibWeb/CSS/Parser/Parser.cpp @@ -1648,14 +1648,19 @@ Optional Parser::convert_to_style_property(Declaration const& dec return StyleProperty { declaration.important, property_id.value(), value.release_value(), {} }; } -Optional Parser::parse_source_size_value(TokenStream& tokens) +Optional Parser::parse_source_size_value(TokenStream& tokens) { if (tokens.next_token().is_ident("auto"sv)) { tokens.discard_a_token(); // auto - return LengthOrCalculated { Length::make_auto() }; + return LengthOrAutoOrCalculated { LengthOrAuto::make_auto() }; } - return parse_length(tokens); + if (auto parsed = parse_length(tokens); parsed.has_value()) { + if (parsed->is_calculated()) + return LengthOrAutoOrCalculated { parsed->calculated() }; + return LengthOrAutoOrCalculated { parsed->value() }; + } + return {}; } bool Parser::context_allows_quirky_length() const @@ -1732,11 +1737,7 @@ LengthOrCalculated Parser::parse_as_sizes_attribute(DOM::Element const& element, auto unparsed_sizes_list = parse_a_comma_separated_list_of_component_values(m_token_stream); // 2. Let size be null. - Optional size; - - auto size_is_auto = [&size]() { - return !size->is_calculated() && size->value().is_auto(); - }; + Optional size; auto remove_all_consecutive_whitespace_tokens_from_the_end_of = [](auto& tokens) { while (!tokens.is_empty() && tokens.last().is_token() && tokens.last().token().is(Token::Type::Whitespace)) @@ -1781,7 +1782,7 @@ LengthOrCalculated Parser::parse_as_sizes_attribute(DOM::Element const& element, // 3. If size is auto, and img is not null, and img is being rendered, and img allows auto-sizes, // then set size to the concrete object size width of img, in CSS pixels. // FIXME: "img is being rendered" - we just see if it has a bitmap for now - if (size_is_auto() && img && img->immutable_bitmap() && img->allows_auto_sizes()) { + if (size->is_auto() && img && img->immutable_bitmap() && img->allows_auto_sizes()) { // FIXME: The spec doesn't seem to tell us how to determine the concrete size of an , so use the default sizing algorithm. // Should this use some of the methods from FormattingContext? auto concrete_size = run_default_sizing_algorithm( @@ -1807,8 +1808,8 @@ LengthOrCalculated Parser::parse_as_sizes_attribute(DOM::Element const& element, } // 2. If size is not auto, then return size. Otherwise, continue. - if (!size_is_auto()) - return size.release_value(); + if (!size->is_auto()) + return size->without_auto(); continue; } @@ -1822,8 +1823,8 @@ LengthOrCalculated Parser::parse_as_sizes_attribute(DOM::Element const& element, } // 5. If size is not auto, then return size. Otherwise, continue. - if (!size_is_auto()) - return size.value(); + if (!size->is_auto()) + return size->without_auto(); } // 4. Return 100vw. diff --git a/Libraries/LibWeb/CSS/Parser/Parser.h b/Libraries/LibWeb/CSS/Parser/Parser.h index 290bfad4011..4f2148f7a0a 100644 --- a/Libraries/LibWeb/CSS/Parser/Parser.h +++ b/Libraries/LibWeb/CSS/Parser/Parser.h @@ -287,7 +287,7 @@ private: Optional parse_time(TokenStream&); Optional parse_time_percentage(TokenStream&); - Optional parse_source_size_value(TokenStream&); + Optional parse_source_size_value(TokenStream&); Optional parse_ratio(TokenStream&); Optional parse_unicode_range(TokenStream&); Optional parse_unicode_range(StringView); diff --git a/Libraries/LibWeb/HTML/SourceSet.cpp b/Libraries/LibWeb/HTML/SourceSet.cpp index d21d2909551..aed318ef1b3 100644 --- a/Libraries/LibWeb/HTML/SourceSet.cpp +++ b/Libraries/LibWeb/HTML/SourceSet.cpp @@ -17,7 +17,9 @@ namespace Web::HTML { SourceSet::SourceSet() - : m_source_size(CSS::Length::make_auto()) + // Note: m_source_size always gets reassigned to its proper value during one of the construction algorithms. + // 0px is just a fake value here so that we don't have to muddy the type system by using Optional. + : m_source_size(CSS::Length::make_px(0)) { }