From 2a0f6fd23e326d0937900f2499e65a0fb50fb4f3 Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Wed, 11 Sep 2024 16:41:18 +0100 Subject: [PATCH] LibWeb: Bring parse_as_sizes_attribute() up to date with the spec The following spec algorithms had changed since we implemented them: - "parse a sizes attribute" - "update the source set" - "create a source set" This commit brings them up to date, as well as adding some additional logging when parsing the sizes attribute fails in some way. --- .../Libraries/LibWeb/CSS/Parser/Parser.cpp | 76 ++++++++++++++----- Userland/Libraries/LibWeb/CSS/Parser/Parser.h | 2 +- .../LibWeb/HTML/HTMLImageElement.cpp | 17 +++-- Userland/Libraries/LibWeb/HTML/SourceSet.cpp | 14 ++-- Userland/Libraries/LibWeb/HTML/SourceSet.h | 4 +- 5 files changed, 80 insertions(+), 33 deletions(-) diff --git a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp index 4e744c5cd37..76f2d06220c 100644 --- a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp +++ b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp @@ -39,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -86,6 +87,7 @@ #include #include #include +#include #include #include @@ -7930,29 +7932,43 @@ private: }; // https://html.spec.whatwg.org/multipage/images.html#parsing-a-sizes-attribute -LengthOrCalculated Parser::Parser::parse_as_sizes_attribute() +LengthOrCalculated Parser::Parser::parse_as_sizes_attribute([[maybe_unused]] DOM::Element const& element, HTML::HTMLImageElement const* img) { + // When asked to parse a sizes attribute from an element element, with an img element or null img: + // 1. Let unparsed sizes list be the result of parsing a comma-separated list of component values // from the value of element's sizes attribute (or the empty string, if the attribute is absent). + // NOTE: The sizes attribute has already been tokenized into m_token_stream by this point. 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(); + }; + + 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)) + tokens.take_last(); + }; + // 3. For each unparsed size in unparsed sizes list: - for (auto& unparsed_size : unparsed_sizes_list) { + for (auto i = 0u; i < unparsed_sizes_list.size(); i++) { + auto& unparsed_size = unparsed_sizes_list[i]; + // 1. Remove all consecutive s from the end of unparsed size. // If unparsed size is now empty, that is a parse error; continue. - while (!unparsed_size.is_empty() && unparsed_size.last().is_token() && unparsed_size.last().token().is(Token::Type::Whitespace)) - unparsed_size.take_last(); + remove_all_consecutive_whitespace_tokens_from_the_end_of(unparsed_size); if (unparsed_size.is_empty()) { log_parse_error(); + dbgln_if(CSS_PARSER_DEBUG, "-> Failed in step 3.1; all whitespace"); continue; } // 2. If the last component value in unparsed size is a valid non-negative , - // let size be its value and remove the component value from unparsed size. - // FIXME: Any CSS function other than the math functions is invalid. + // then set size to its value and remove the component value from unparsed size. + // Any CSS function other than the math functions is invalid. // Otherwise, there is a parse error; continue. auto last_value_stream = TokenStream::of_single_token(unparsed_size.last()); if (auto source_size_value = parse_source_size_value(last_value_stream); source_size_value.has_value()) { @@ -7960,33 +7976,55 @@ LengthOrCalculated Parser::Parser::parse_as_sizes_attribute() unparsed_size.take_last(); } else { log_parse_error(); + dbgln_if(CSS_PARSER_DEBUG, "-> Failed in step 3.2; couldn't parse {} as a ", unparsed_size.last().to_debug_string()); continue; } - // 3. Remove all consecutive s from the end of unparsed size. - while (!unparsed_size.is_empty() && unparsed_size.last().is_token() && unparsed_size.last().token().is(Token::Type::Whitespace)) - unparsed_size.take_last(); + // 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()) { + // 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( + img->width(), img->height(), + img->natural_width(), img->natural_height(), img->intrinsic_aspect_ratio(), + // NOTE: https://html.spec.whatwg.org/multipage/rendering.html#img-contain-size + CSSPixelSize { 300, 150 }); + size = Length::make_px(concrete_size.width()); + } - // If unparsed size is now empty, then return size. - if (unparsed_size.is_empty()) - return size.value(); + // 4. Remove all consecutive s from the end of unparsed size. + // If unparsed size is now empty: + remove_all_consecutive_whitespace_tokens_from_the_end_of(unparsed_size); + if (unparsed_size.is_empty()) { + // 1. If this was not the last item in unparsed sizes list, that is a parse error. + if (i != unparsed_sizes_list.size() - 1) { + log_parse_error(); + dbgln_if(CSS_PARSER_DEBUG, "-> Failed in step 3.4.1; is unparsed size #{}, count {}", i, unparsed_sizes_list.size()); + } - // FIXME: If this was not the keyword auto and it was not the last item in unparsed sizes list, that is a parse error. + // 2. If size is not auto, then return size. Otherwise, continue. + if (!size_is_auto()) + return size.release_value(); + continue; + } - // 4. Parse the remaining component values in unparsed size as a . + // 5. Parse the remaining component values in unparsed size as a . // If it does not parse correctly, or it does parse correctly but the evaluates to false, continue. TokenStream token_stream { unparsed_size }; auto media_condition = parse_media_condition(token_stream, MediaCondition::AllowOr::Yes); - auto context_window = m_context.window(); - if (context_window && media_condition && media_condition->evaluate(*context_window) == MatchResult::True) { - return size.value(); + auto const* context_window = m_context.window(); + if (!media_condition || (context_window && media_condition->evaluate(*context_window) == MatchResult::False)) { + continue; } - // 5. If size is not auto, then return size. - if (size.value().is_calculated() || !size.value().value().is_auto()) + // 5. If size is not auto, then return size. Otherwise, continue. + if (!size_is_auto()) return size.value(); } + // 4. Return 100vw. return Length(100, Length::Type::Vw); } diff --git a/Userland/Libraries/LibWeb/CSS/Parser/Parser.h b/Userland/Libraries/LibWeb/CSS/Parser/Parser.h index e6f30c7774f..545bfe6fe29 100644 --- a/Userland/Libraries/LibWeb/CSS/Parser/Parser.h +++ b/Userland/Libraries/LibWeb/CSS/Parser/Parser.h @@ -75,7 +75,7 @@ public: static NonnullRefPtr resolve_unresolved_style_value(ParsingContext const&, DOM::Element&, Optional, PropertyID, UnresolvedStyleValue const&); - [[nodiscard]] LengthOrCalculated parse_as_sizes_attribute(); + [[nodiscard]] LengthOrCalculated parse_as_sizes_attribute(DOM::Element const& element, HTML::HTMLImageElement const* img = nullptr); private: Parser(ParsingContext const&, Vector); diff --git a/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp index 1f67d45bdab..5f21e9ad28f 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp @@ -990,7 +990,12 @@ static void update_the_source_set(DOM::Element& element) }); } - // 4. For each child in elements: + // 4. Let img be el if el is an img element, otherwise null. + HTMLImageElement* img = nullptr; + if (is(element)) + img = static_cast(&element); + + // 5. For each child in elements: for (auto child : elements) { // 1. If child is el: if (child == &element) { @@ -1039,11 +1044,13 @@ static void update_the_source_set(DOM::Element& element) default_source = href_value.release_value(); } - // 10. Let el's source set be the result of creating a source set given default source, srcset, and sizes. + // 10. Let el's source set be the result of creating a source set given default source, srcset, sizes, and img. if (is(element)) - static_cast(element).set_source_set(SourceSet::create(element, default_source, srcset, sizes)); + static_cast(element).set_source_set(SourceSet::create(element, default_source, srcset, sizes, img)); else if (is(element)) TODO(); + + // 11. Return. return; } // 2. If child is not a source element, then continue. @@ -1070,8 +1077,8 @@ static void update_the_source_set(DOM::Element& element) } } - // 7. Parse child's sizes attribute, and let source set's source size be the returned value. - source_set.m_source_size = parse_a_sizes_attribute(element.document(), child->get_attribute_value(HTML::AttributeNames::sizes)); + // 7. Parse child's sizes attribute with img, and let source set's source size be the returned value. + source_set.m_source_size = parse_a_sizes_attribute(element, child->get_attribute_value(HTML::AttributeNames::sizes), img); // 8. If child has a type attribute, and its value is an unknown or unsupported MIME type, continue to the next child. if (child->has_attribute(HTML::AttributeNames::type)) { diff --git a/Userland/Libraries/LibWeb/HTML/SourceSet.cpp b/Userland/Libraries/LibWeb/HTML/SourceSet.cpp index da53794453c..e0d9e349d2e 100644 --- a/Userland/Libraries/LibWeb/HTML/SourceSet.cpp +++ b/Userland/Libraries/LibWeb/HTML/SourceSet.cpp @@ -339,15 +339,17 @@ descriptor_parser: } // https://html.spec.whatwg.org/multipage/images.html#parse-a-sizes-attribute -CSS::LengthOrCalculated parse_a_sizes_attribute(DOM::Document const& document, StringView sizes) +CSS::LengthOrCalculated parse_a_sizes_attribute(DOM::Element const& element, StringView sizes, HTML::HTMLImageElement const* img) { - auto css_parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext { document }, sizes); - return css_parser.parse_as_sizes_attribute(); + auto css_parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext { element.document() }, sizes); + return css_parser.parse_as_sizes_attribute(element, img); } // https://html.spec.whatwg.org/multipage/images.html#create-a-source-set -SourceSet SourceSet::create(DOM::Element const& element, String default_source, String srcset, String sizes) +SourceSet SourceSet::create(DOM::Element const& element, String const& default_source, String const& srcset, String const& sizes, HTML::HTMLImageElement const* img) { + // When asked to create a source set given a string default source, a string srcset, a string sizes, and an element or null img: + // 1. Let source set be an empty source set. SourceSet source_set; @@ -355,8 +357,8 @@ SourceSet SourceSet::create(DOM::Element const& element, String default_source, if (!srcset.is_empty()) source_set = parse_a_srcset_attribute(srcset); - // 3. Let source size be the result of parsing sizes. - source_set.m_source_size = parse_a_sizes_attribute(element.document(), sizes); + // 3. Let source size be the result of parsing sizes with img. + source_set.m_source_size = parse_a_sizes_attribute(element, sizes, img); // 4. If default source is not the empty string and source set does not contain an image source // with a pixel density descriptor value of 1, and no image source with a width descriptor, diff --git a/Userland/Libraries/LibWeb/HTML/SourceSet.h b/Userland/Libraries/LibWeb/HTML/SourceSet.h index e4a1fe6885c..5031f7c74d9 100644 --- a/Userland/Libraries/LibWeb/HTML/SourceSet.h +++ b/Userland/Libraries/LibWeb/HTML/SourceSet.h @@ -33,7 +33,7 @@ struct ImageSourceAndPixelDensity { // https://html.spec.whatwg.org/multipage/images.html#source-set struct SourceSet { - static SourceSet create(DOM::Element const&, String default_source, String srcset, String sizes); + static SourceSet create(DOM::Element const& element, String const& default_source, String const& srcset, String const& sizes, HTML::HTMLImageElement const* img = nullptr); [[nodiscard]] bool is_empty() const; @@ -50,6 +50,6 @@ struct SourceSet { }; SourceSet parse_a_srcset_attribute(StringView); -[[nodiscard]] CSS::LengthOrCalculated parse_a_sizes_attribute(DOM::Document const&, StringView); +[[nodiscard]] CSS::LengthOrCalculated parse_a_sizes_attribute(DOM::Element const& element, StringView sizes, HTML::HTMLImageElement const* img = nullptr); }