From ab9aa9da0d6a2c8dd3150ce822bf039fea481103 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Wed, 2 Nov 2022 20:19:03 +0100 Subject: [PATCH] LibWeb: Do CSS var() expansion in a separate pass By expanding all the var() values first, we allow var() to occur anywhere, even as arguments to CSS functions. --- .../Libraries/LibWeb/CSS/StyleComputer.cpp | 136 +++++++++++------- Userland/Libraries/LibWeb/CSS/StyleComputer.h | 3 +- 2 files changed, 83 insertions(+), 56 deletions(-) diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp index 36691a0f3b..c3ee482b1e 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -556,12 +556,8 @@ static RefPtr get_custom_property(DOM::Element const& element, FlySt return nullptr; } -bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView property_name, HashMap>& dependencies, Parser::TokenStream& source, Vector& dest) const +bool StyleComputer::expand_variables(DOM::Element& element, StringView property_name, HashMap>& dependencies, Parser::TokenStream& source, Vector& dest) const { - // FIXME: Do this better! - // We build a copy of the tree of ComponentValues, with all var()s and attr()s replaced with their contents. - // This is a very naive solution, and we could do better if the CSS Parser could accept tokens one at a time. - // Arbitrary large value chosen to avoid the billion-laughs attack. // https://www.w3.org/TR/css-variables-1/#long-variables const size_t MAX_VALUE_COUNT = 16384; @@ -578,53 +574,77 @@ bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView p return new_node; }; + while (source.has_next_token()) { + auto const& value = source.next_token(); + if (!value.is_function()) { + dest.empend(value); + continue; + } + if (!value.function().name().equals_ignoring_case("var"sv)) { + auto const& source_function = value.function(); + Vector function_values; + Parser::TokenStream source_function_contents { source_function.values() }; + if (!expand_variables(element, property_name, dependencies, source_function_contents, function_values)) + return false; + NonnullRefPtr function = Parser::Function::create(source_function.name(), move(function_values)); + dest.empend(function); + continue; + } + + Parser::TokenStream var_contents { value.function().values() }; + var_contents.skip_whitespace(); + if (!var_contents.has_next_token()) + return false; + + auto const& custom_property_name_token = var_contents.next_token(); + if (!custom_property_name_token.is(Parser::Token::Type::Ident)) + return false; + auto custom_property_name = custom_property_name_token.token().ident(); + if (!custom_property_name.starts_with("--"sv)) + return false; + + // Detect dependency cycles. https://www.w3.org/TR/css-variables-1/#cycles + // We do not do this by the spec, since we are not keeping a graph of var dependencies around, + // but rebuilding it every time. + if (custom_property_name == property_name) + return false; + auto parent = get_dependency_node(property_name); + auto child = get_dependency_node(custom_property_name); + parent->add_child(child); + if (parent->has_cycles()) + return false; + + if (auto custom_property_value = get_custom_property(element, custom_property_name)) { + VERIFY(custom_property_value->is_unresolved()); + Parser::TokenStream custom_property_tokens { custom_property_value->as_unresolved().values() }; + if (!expand_variables(element, custom_property_name, dependencies, custom_property_tokens, dest)) + return false; + continue; + } + + // Use the provided fallback value, if any. + var_contents.skip_whitespace(); + if (var_contents.has_next_token()) { + auto const& comma_token = var_contents.next_token(); + if (!comma_token.is(Parser::Token::Type::Comma)) + return false; + var_contents.skip_whitespace(); + if (!expand_variables(element, property_name, dependencies, var_contents, dest)) + return false; + } + } + return true; +} + +bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView property_name, Parser::TokenStream& source, Vector& dest) const +{ + // FIXME: Do this better! + // We build a copy of the tree of ComponentValues, with all var()s and attr()s replaced with their contents. + // This is a very naive solution, and we could do better if the CSS Parser could accept tokens one at a time. + while (source.has_next_token()) { auto const& value = source.next_token(); if (value.is_function()) { - if (value.function().name().equals_ignoring_case("var"sv)) { - Parser::TokenStream var_contents { value.function().values() }; - var_contents.skip_whitespace(); - if (!var_contents.has_next_token()) - return false; - - auto const& custom_property_name_token = var_contents.next_token(); - if (!custom_property_name_token.is(Parser::Token::Type::Ident)) - return false; - auto custom_property_name = custom_property_name_token.token().ident(); - if (!custom_property_name.starts_with("--"sv)) - return false; - - // Detect dependency cycles. https://www.w3.org/TR/css-variables-1/#cycles - // We do not do this by the spec, since we are not keeping a graph of var dependencies around, - // but rebuilding it every time. - if (custom_property_name == property_name) - return false; - auto parent = get_dependency_node(property_name); - auto child = get_dependency_node(custom_property_name); - parent->add_child(child); - if (parent->has_cycles()) - return false; - - if (auto custom_property_value = get_custom_property(element, custom_property_name)) { - VERIFY(custom_property_value->is_unresolved()); - Parser::TokenStream custom_property_tokens { custom_property_value->as_unresolved().values() }; - if (!expand_unresolved_values(element, custom_property_name, dependencies, custom_property_tokens, dest)) - return false; - continue; - } - - // Use the provided fallback value, if any. - var_contents.skip_whitespace(); - if (var_contents.has_next_token()) { - auto const& comma_token = var_contents.next_token(); - if (!comma_token.is(Parser::Token::Type::Comma)) - return false; - var_contents.skip_whitespace(); - if (!expand_unresolved_values(element, property_name, dependencies, var_contents, dest)) - return false; - } - continue; - } if (value.function().name().equals_ignoring_case("attr"sv)) { // https://drafts.csswg.org/css-values-5/#attr-substitution Parser::TokenStream attr_contents { value.function().values() }; @@ -653,7 +673,7 @@ bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView p if (!comma_token.is(Parser::Token::Type::Comma)) return false; attr_contents.skip_whitespace(); - if (!expand_unresolved_values(element, property_name, dependencies, attr_contents, dest)) + if (!expand_unresolved_values(element, property_name, attr_contents, dest)) return false; continue; } @@ -665,7 +685,7 @@ bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView p auto const& source_function = value.function(); Vector function_values; Parser::TokenStream source_function_contents { source_function.values() }; - if (!expand_unresolved_values(element, property_name, dependencies, source_function_contents, function_values)) + if (!expand_unresolved_values(element, property_name, source_function_contents, function_values)) return false; NonnullRefPtr function = Parser::Function::create(source_function.name(), move(function_values)); dest.empend(function); @@ -675,7 +695,7 @@ bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView p auto const& source_block = value.block(); Parser::TokenStream source_block_values { source_block.values() }; Vector block_values; - if (!expand_unresolved_values(element, property_name, dependencies, source_block_values, block_values)) + if (!expand_unresolved_values(element, property_name, source_block_values, block_values)) return false; NonnullRefPtr block = Parser::Block::create(source_block.token(), move(block_values)); dest.empend(move(block)); @@ -693,10 +713,16 @@ RefPtr StyleComputer::resolve_unresolved_style_value(DOM::Element& e // to produce a different StyleValue from it. VERIFY(unresolved.contains_var_or_attr()); - Vector expanded_values; + Parser::TokenStream unresolved_values_without_variables_expanded { unresolved.values() }; + Vector values_with_variables_expanded; + HashMap> dependencies; - Parser::TokenStream unresolved_values { unresolved.values() }; - if (!expand_unresolved_values(element, string_from_property_id(property_id), dependencies, unresolved_values, expanded_values)) + if (!expand_variables(element, string_from_property_id(property_id), dependencies, unresolved_values_without_variables_expanded, values_with_variables_expanded)) + return {}; + + Parser::TokenStream unresolved_values_with_variables_expanded { values_with_variables_expanded }; + Vector expanded_values; + if (!expand_unresolved_values(element, string_from_property_id(property_id), unresolved_values_with_variables_expanded, expanded_values)) return {}; if (auto parsed_value = Parser::Parser::parse_css_value({}, Parser::ParsingContext { document() }, property_id, expanded_values)) diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.h b/Userland/Libraries/LibWeb/CSS/StyleComputer.h index 8a22f750d3..5a010af73f 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.h +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.h @@ -87,7 +87,8 @@ private: void compute_defaulted_property_value(StyleProperties&, DOM::Element const*, CSS::PropertyID, Optional) const; RefPtr resolve_unresolved_style_value(DOM::Element&, PropertyID, UnresolvedStyleValue const&) const; - bool expand_unresolved_values(DOM::Element&, StringView property_name, HashMap>& dependencies, Parser::TokenStream& source, Vector& dest) const; + bool expand_variables(DOM::Element&, StringView property_name, HashMap>& dependencies, Parser::TokenStream& source, Vector& dest) const; + bool expand_unresolved_values(DOM::Element&, StringView property_name, Parser::TokenStream& source, Vector& dest) const; template void for_each_stylesheet(CascadeOrigin, Callback) const;