mirrors/ladybird
1
0
Fork 0
mirror of https://github.com/LadybirdBrowser/ladybird.git synced 2025-04-21 20:15:17 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 10

LibWeb: Move style sheet parsing into create_a_css_style_sheet()

Browse source 
The spec is unclear about when exactly we should parse the style sheet.
Previously we'd do so before calling this algorithm, which was
error-prone, as seen by the bug fixed by the previous commit. The spec
for step 1 of "create a CSS style sheet" says:

1. Create a new CSS style sheet object and set its properties as
   specified.

The definitions linked are UA-defined enough that it seems reasonable to
put the parsing here. That simplifies the user code a little and makes
it harder to mess up. It does raise the question of what to do if
parsing fails. I've matched our previous behaviour by just logging and
returning in that case.

While I'm modifying this method, I've also converted the bool params to
enums so they're a little clearer to read.
This commit is contained in:
Sam Atkins 2025-04-14 13:53:11 +01:00
parent f29a95e2fe
commit 2a96a81e68
Notes: github-actions[bot] 2025-04-15 08:41:41 +00:00
4 changed files with 74 additions and 50 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

37
Libraries/LibWeb/CSS/StyleSheetList.cpp
View file

@ -1,5 +1,6 @@
/*
* Copyright (c) 2020-2024, Andreas Kling <andreas@ladybird.org>
* Copyright (c) 2025, Sam Atkins <sam@ladybird.org>
*
* SPDX-License-Identifier: BSD-2-Clause
*/
@ -60,23 +61,35 @@ void StyleSheetList::add_a_css_style_sheet(CSS::CSSStyleSheet& sheet)
}
// https://www.w3.org/TR/cssom/#create-a-css-style-sheet
void StyleSheetList::create_a_css_style_sheet(String type, DOM::Element* owner_node, String media, String title, bool alternate, bool origin_clean, Optional<::URL::URL> location, CSS::CSSStyleSheet* parent_style_sheet, CSS::CSSRule* owner_rule, CSS::CSSStyleSheet& sheet)
GC::Ptr<CSSStyleSheet> StyleSheetList::create_a_css_style_sheet(String const& css_text, String type, DOM::Element* owner_node, String media, String title, Alternate alternate, OriginClean origin_clean, Optional<::URL::URL> location, CSSStyleSheet* parent_style_sheet, CSSRule* owner_rule)
{
// 1. Create a new CSS style sheet object and set its properties as specified.
// FIXME: We receive `sheet` from the caller already. This is weird.
// AD-HOC: The spec never tells us when to parse this style sheet, but the most logical place is here.
// AD-HOC: Are we supposed to use the document's URL for the stylesheet's location during parsing? Not doing it breaks things.
auto location_url = location.value_or(document().url());
auto* sheet = parse_css_stylesheet(Parser::ParsingParams { document(), location_url }, css_text, location_url);
sheet.set_parent_css_style_sheet(parent_style_sheet);
sheet.set_owner_css_rule(owner_rule);
sheet.set_owner_node(owner_node);
sheet.set_type(move(type));
sheet.set_media(move(media));
sheet.set_title(move(title));
sheet.set_alternate(alternate);
sheet.set_origin_clean(origin_clean);
sheet.set_location(move(location));
// AD-HOC: Exit out if parsing failed.
// FIXME: What should we actually do here?
if (!sheet) {
dbgln_if(CSS_LOADER_DEBUG, "StyleSheetList::create_a_css_style_sheet(): Failed to parse stylesheet at {}", location_url);
return nullptr;
}
sheet->set_parent_css_style_sheet(parent_style_sheet);
sheet->set_owner_css_rule(owner_rule);
sheet->set_owner_node(owner_node);
sheet->set_type(move(type));
sheet->set_media(move(media));
sheet->set_title(move(title));
sheet->set_alternate(alternate == Alternate::Yes);
sheet->set_origin_clean(origin_clean == OriginClean::Yes);
sheet->set_location(move(location));
// 2. Then run the add a CSS style sheet steps for the newly created CSS style sheet.
add_a_css_style_sheet(sheet);
add_a_css_style_sheet(*sheet);
return sheet;
}
void StyleSheetList::add_sheet(CSSStyleSheet& sheet)

15
Libraries/LibWeb/CSS/StyleSheetList.h
View file

@ -1,6 +1,7 @@
/*
* Copyright (c) 2020-2024, Andreas Kling <andreas@ladybird.org>
* Copyright (c) 2023, Luke Wilde <lukew@serenityos.org>
* Copyright (c) 2025, Sam Atkins <sam@ladybird.org>
*
* SPDX-License-Identifier: BSD-2-Clause
*/
@ -19,9 +20,17 @@ class StyleSheetList final : public Bindings::PlatformObject {
public:
[[nodiscard]] static GC::Ref<StyleSheetList> create(GC::Ref<DOM::Node> document_or_shadow_root);
void add_a_css_style_sheet(CSS::CSSStyleSheet&);
void remove_a_css_style_sheet(CSS::CSSStyleSheet&);
void create_a_css_style_sheet(String type, DOM::Element* owner_node, String media, String title, bool alternate, bool origin_clean, Optional<::URL::URL> location, CSS::CSSStyleSheet* parent_style_sheet, CSS::CSSRule* owner_rule, CSS::CSSStyleSheet&);
void add_a_css_style_sheet(CSSStyleSheet&);
void remove_a_css_style_sheet(CSSStyleSheet&);
enum class Alternate : u8 {
No,
Yes,
};
enum class OriginClean : u8 {
No,
Yes,
};
GC::Ptr<CSSStyleSheet> create_a_css_style_sheet(String const& css_text, String type, DOM::Element* owner_node, String media, String title, Alternate, OriginClean, Optional<::URL::URL> location, CSSStyleSheet* parent_style_sheet, CSSRule* owner_rule);
Vector<GC::Ref<CSSStyleSheet>> const& sheets() const { return m_sheets; }
Vector<GC::Ref<CSSStyleSheet>>& sheets() { return m_sheets; }

42
Libraries/LibWeb/DOM/StyleElementUtils.cpp
View file

@ -52,31 +52,41 @@ void StyleElementUtils::update_a_style_block(DOM::Element& style_element)
// FIXME: 5. If the Should element's inline behavior be blocked by Content Security Policy? algorithm returns "Blocked" when executed upon the style element, "style", and the style element's child text content, then return. [CSP]
// FIXME: This is a bit awkward, as the spec doesn't actually tell us when to parse the CSS text,
// so we just do it here and pass the parsed sheet to create_a_css_style_sheet().
// AD-HOC: Are we supposed to use the document's URL for the stylesheet's location? Not doing it breaks things.
auto* sheet = parse_css_stylesheet(CSS::Parser::ParsingParams(style_element.document()), style_element.text_content().value_or(String {}), style_element.document().url());
if (!sheet)
return;
// FIXME: This should probably be handled by StyleSheet::set_owner_node().
m_associated_css_style_sheet = sheet;
// 6. Create a CSS style sheet with the following properties...
// 6. Create a CSS style sheet with the following properties:
// type
// text/css
// owner node
// element
// media
// The media attribute of element.
// title
// The title attribute of element, if element is in a document tree, or the empty string otherwise.
// alternate flag
// Unset.
// origin-clean flag
// Set.
// location
// parent CSS style sheet
// owner CSS rule
// null
// disabled flag
// Left at its default value.
// CSS rules
// Left uninitialized.
m_style_sheet_list = style_element.document_or_shadow_root_style_sheets();
m_style_sheet_list->create_a_css_style_sheet(
m_associated_css_style_sheet = m_style_sheet_list->create_a_css_style_sheet(
style_element.text_content().value_or(String {}),
"text/css"_string,
&style_element,
style_element.attribute(HTML::AttributeNames::media).value_or({}),
style_element.in_a_document_tree()
? style_element.attribute(HTML::AttributeNames::title).value_or({})
: String {},
false,
true,
CSS::StyleSheetList::Alternate::No,
CSS::StyleSheetList::OriginClean::Yes,
{},
nullptr,
nullptr,
*sheet);
nullptr);
}
void StyleElementUtils::visit_edges(JS::Cell::Visitor& visitor)

30
Libraries/LibWeb/HTML/HTMLLinkElement.cpp
View file

@ -486,26 +486,18 @@ void HTMLLinkElement::process_stylesheet_resource(bool success, Fetch::Infrastru
dbgln("Style sheet {} claimed to be '{}' but decoding failed", response.url().value_or(URL::URL()), encoding);
dispatch_event(*DOM::Event::create(realm(), HTML::EventNames::error));
} else {
auto const decoded_string = maybe_decoded_string.release_value();
VERIFY(!response.url_list().is_empty());
auto location = response.url_list().first();
m_loaded_style_sheet = parse_css_stylesheet(CSS::Parser::ParsingParams(document(), location), decoded_string, location);
if (m_loaded_style_sheet) {
document_or_shadow_root_style_sheets().create_a_css_style_sheet(
"text/css"_string,
this,
attribute(HTML::AttributeNames::media).value_or({}),
in_a_document_tree() ? attribute(HTML::AttributeNames::title).value_or({}) : String {},
m_relationship & Relationship::Alternate && !m_explicitly_enabled,
true,
move(location),
nullptr,
nullptr,
*m_loaded_style_sheet);
} else {
dbgln_if(CSS_LOADER_DEBUG, "HTMLLinkElement: Failed to parse stylesheet: {}", resource()->url());
}
m_loaded_style_sheet = document_or_shadow_root_style_sheets().create_a_css_style_sheet(
maybe_decoded_string.release_value(),
"text/css"_string,
this,
attribute(HTML::AttributeNames::media).value_or({}),
in_a_document_tree() ? attribute(HTML::AttributeNames::title).value_or({}) : String {},
(m_relationship & Relationship::Alternate && !m_explicitly_enabled) ? CSS::StyleSheetList::Alternate::Yes : CSS::StyleSheetList::Alternate::No,
CSS::StyleSheetList::OriginClean::Yes,
response.url_list().first(),
nullptr,
nullptr);
// 2. Fire an event named load at el.
dispatch_event(*DOM::Event::create(realm(), HTML::EventNames::load));