From 45f854296527b3106c1e823e3f5cf1146359c3c0 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 30 Mar 2023 12:08:41 +0200 Subject: [PATCH] LibWeb: Actually visit rules and media queries in imported style sheets Due to CSSImportRule::has_import_result() being backwards, we never actually entered imported style sheets when traversing style rules or media queries. With this fixed, we no longer need the "collect style sheets" step in StyleComputer, as normal for_each_effective_style_rule() will now actually find all the rules. :^) --- .../css-imported-sheet-with-media-rule.txt | 7 +++++++ .../input/css-imported-sheet-with-media-rule.css | 5 +++++ .../css-imported-sheet-with-media-rule.html | 7 +++++++ Userland/Libraries/LibWeb/CSS/CSSImportRule.h | 1 - Userland/Libraries/LibWeb/CSS/CSSRuleList.cpp | 4 ++-- Userland/Libraries/LibWeb/CSS/StyleComputer.cpp | 16 ---------------- 6 files changed, 21 insertions(+), 19 deletions(-) create mode 100644 Tests/LibWeb/Layout/expected/css-imported-sheet-with-media-rule.txt create mode 100644 Tests/LibWeb/Layout/input/css-imported-sheet-with-media-rule.css create mode 100644 Tests/LibWeb/Layout/input/css-imported-sheet-with-media-rule.html diff --git a/Tests/LibWeb/Layout/expected/css-imported-sheet-with-media-rule.txt b/Tests/LibWeb/Layout/expected/css-imported-sheet-with-media-rule.txt new file mode 100644 index 00000000000..1769f5d7969 --- /dev/null +++ b/Tests/LibWeb/Layout/expected/css-imported-sheet-with-media-rule.txt @@ -0,0 +1,7 @@ +Viewport <#document> at (0,0) content-size 800x600 children: not-inline + BlockContainer at (0,0) content-size 800x125.179687 children: not-inline + BlockContainer at (8,8) content-size 784x109.179687 children: inline + line 0 width: 275.292968, height: 109.179687, bottom: 109.179687, baseline: 84.570312 + frag 0 from TextNode start: 0, length: 5, rect: [8,8 275.292968x109.179687] + "Crazy" + TextNode <#text> diff --git a/Tests/LibWeb/Layout/input/css-imported-sheet-with-media-rule.css b/Tests/LibWeb/Layout/input/css-imported-sheet-with-media-rule.css new file mode 100644 index 00000000000..fe93702aae8 --- /dev/null +++ b/Tests/LibWeb/Layout/input/css-imported-sheet-with-media-rule.css @@ -0,0 +1,5 @@ +@media screen { + body { + font-size: 100px; + } +} diff --git a/Tests/LibWeb/Layout/input/css-imported-sheet-with-media-rule.html b/Tests/LibWeb/Layout/input/css-imported-sheet-with-media-rule.html new file mode 100644 index 00000000000..7cf047ffe4f --- /dev/null +++ b/Tests/LibWeb/Layout/input/css-imported-sheet-with-media-rule.html @@ -0,0 +1,7 @@ + +Crazy diff --git a/Userland/Libraries/LibWeb/CSS/CSSImportRule.h b/Userland/Libraries/LibWeb/CSS/CSSImportRule.h index caf436c90a3..03df9a6e965 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSImportRule.h +++ b/Userland/Libraries/LibWeb/CSS/CSSImportRule.h @@ -29,7 +29,6 @@ public: // FIXME: This should return only the specified part of the url. eg, "stuff/foo.css", not "https://example.com/stuff/foo.css". DeprecatedString href() const { return m_url.to_deprecated_string(); } - bool has_import_result() const { return !m_style_sheet; } CSSStyleSheet* loaded_style_sheet() { return m_style_sheet; } CSSStyleSheet const* loaded_style_sheet() const { return m_style_sheet; } CSSStyleSheet* style_sheet_for_bindings() { return m_style_sheet; } diff --git a/Userland/Libraries/LibWeb/CSS/CSSRuleList.cpp b/Userland/Libraries/LibWeb/CSS/CSSRuleList.cpp index 8b26140d2bf..27c3e5335f7 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSRuleList.cpp +++ b/Userland/Libraries/LibWeb/CSS/CSSRuleList.cpp @@ -128,7 +128,7 @@ void CSSRuleList::for_each_effective_style_rule(Function(*rule); - if (import_rule.has_import_result() && import_rule.loaded_style_sheet()) + if (import_rule.loaded_style_sheet()) import_rule.loaded_style_sheet()->for_each_effective_style_rule(callback); break; } @@ -155,7 +155,7 @@ bool CSSRuleList::evaluate_media_queries(HTML::Window const& window) break; case CSSRule::Type::Import: { auto& import_rule = verify_cast(*rule); - if (import_rule.has_import_result() && import_rule.loaded_style_sheet() && import_rule.loaded_style_sheet()->evaluate_media_queries(window)) + if (import_rule.loaded_style_sheet() && import_rule.loaded_style_sheet()->evaluate_media_queries(window)) any_media_queries_changed_match_state = true; break; } diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp index dcc9cabff33..8e05bf58db8 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -162,19 +162,6 @@ static CSSStyleSheet& quirks_mode_stylesheet(DOM::Document const& document) return *sheet; } -static void collect_style_sheets(CSSStyleSheet const& sheet, Vector>& sheets) -{ - sheets.append(sheet); - for (auto const& rule : sheet.rules()) { - if (rule->type() == CSSRule::Type::Import) { - auto const& import_rule = static_cast(*rule); - if (auto const* imported_sheet = import_rule.loaded_style_sheet()) { - collect_style_sheets(*imported_sheet, sheets); - } - } - } -} - template void StyleComputer::for_each_stylesheet(CascadeOrigin cascade_origin, Callback callback) const { @@ -184,10 +171,7 @@ void StyleComputer::for_each_stylesheet(CascadeOrigin cascade_origin, Callback c callback(quirks_mode_stylesheet(document())); } if (cascade_origin == CascadeOrigin::Author) { - Vector> sheets; for (auto const& sheet : document().style_sheets().sheets()) - collect_style_sheets(sheet, sheets); - for (auto const& sheet : sheets) callback(*sheet); } }