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

LibWeb/CSS: Don't resolve @import URLs until they are used

Browse source 
The regression in the "conditional-CSSGroupingRule" test is we now fail
the "inserting an `@import`" subtests differently and the subtests
aren't independent. Specifically, we don't yet implement the checks in
`CSSRuleList::insert_a_css_rule()` that reject certain rules from being
inserted. Previously we didn't insert the `@import` rule because we
failed to parse its relative URL. Now we parse it correctly, we end up
inserting it.
This commit is contained in:
Sam Atkins 2025-04-08 18:16:38 +01:00 • committed by Tim Ledbetter
parent ca0890ce16
commit 0f42d5ec3e
Notes: github-actions[bot] 2025-04-09 17:47:10 +00:00
9 changed files with 41 additions and 46 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
Libraries/LibWeb/CSS/CSSImportRule.cpp
View file

@ -9,7 +9,6 @@
#include <AK/Debug.h>
#include <AK/ScopeGuard.h>
#include <LibTextCodec/Decoder.h>
#include <LibURL/URL.h>
#include <LibWeb/Bindings/CSSImportRulePrototype.h>
#include <LibWeb/Bindings/Intrinsics.h>
#include <LibWeb/CSS/CSSImportRule.h>
@ -17,18 +16,19 @@
#include <LibWeb/CSS/Parser/Parser.h>
#include <LibWeb/CSS/StyleComputer.h>
#include <LibWeb/DOM/Document.h>
#include <LibWeb/DOMURL/DOMURL.h>
#include <LibWeb/HTML/Window.h>
namespace Web::CSS {
GC_DEFINE_ALLOCATOR(CSSImportRule);
GC::Ref<CSSImportRule> CSSImportRule::create(JS::Realm& realm, ::URL::URL url, GC::Ptr<DOM::Document> document, RefPtr<Supports> supports, Vector<NonnullRefPtr<MediaQuery>> media_query_list)
GC::Ref<CSSImportRule> CSSImportRule::create(JS::Realm& realm, URL url, GC::Ptr<DOM::Document> document, RefPtr<Supports> supports, Vector<NonnullRefPtr<MediaQuery>> media_query_list)
{
return realm.create<CSSImportRule>(realm, move(url), document, move(supports), move(media_query_list));
}
CSSImportRule::CSSImportRule(JS::Realm& realm, ::URL::URL url, GC::Ptr<DOM::Document> document, RefPtr<Supports> supports, Vector<NonnullRefPtr<MediaQuery>> media_query_list)
CSSImportRule::CSSImportRule(JS::Realm& realm, URL url, GC::Ptr<DOM::Document> document, RefPtr<Supports> supports, Vector<NonnullRefPtr<MediaQuery>> media_query_list)
: CSSRule(realm, Type::Import)
, m_url(move(url))
, m_document(document)
@ -72,7 +72,7 @@ String CSSImportRule::serialized() const
builder.append("@import "sv);
// 2. The result of performing serialize a URL on the rule’s location.
serialize_a_url(builder, m_url.to_string());
builder.append(m_url.to_string());
// AD-HOC: Serialize the rule's supports condition if it exists.
// This isn't currently specified, but major browsers include this in their serialization of import rules
@ -106,15 +106,18 @@ void CSSImportRule::fetch()
// 3. Let parsedUrl be the result of the URL parser steps with rule’s URL and parentStylesheet’s location.
// If the algorithm returns an error, return. [CSSOM]
// FIXME: Stop producing a URL::URL when parsing the @import
auto parsed_url = url().to_string();
auto parsed_url = DOMURL::parse(href(), parent_style_sheet.location());
if (!parsed_url.has_value()) {
dbgln("Unable to parse @import url `{}` parent location `{}` as a URL.", href(), parent_style_sheet.location());
return;
}
// FIXME: Figure out the "correct" way to delay the load event.
m_document_load_event_delayer.emplace(*m_document);
// 4. Fetch a style resource from parsedUrl, with stylesheet parentStylesheet, destination "style", CORS mode "no-cors", and processResponse being the following steps given response response and byte stream, null or failure byteStream:
fetch_a_style_resource(parsed_url, parent_style_sheet, Fetch::Infrastructure::Request::Destination::Style, CorsMode::NoCors,
[strong_this = GC::Ref { *this }, parent_style_sheet = GC::Ref { parent_style_sheet }](auto response, auto maybe_byte_stream) {
fetch_a_style_resource(parsed_url->to_string(), parent_style_sheet, Fetch::Infrastructure::Request::Destination::Style, CorsMode::NoCors,
[strong_this = GC::Ref { *this }, parent_style_sheet = GC::Ref { parent_style_sheet }, parsed_url = parsed_url.value()](auto response, auto maybe_byte_stream) {
// AD-HOC: Stop delaying the load event.
ScopeGuard guard = [strong_this] {
strong_this->m_document_load_event_delayer.clear();
@ -141,19 +144,19 @@ void CSSImportRule::fetch()
auto encoding = "utf-8"sv;
auto maybe_decoder = TextCodec::decoder_for(encoding);
if (!maybe_decoder.has_value()) {
dbgln_if(CSS_LOADER_DEBUG, "CSSImportRule: Failed to decode CSS file: {} Unsupported encoding: {}", strong_this->url(), encoding);
dbgln_if(CSS_LOADER_DEBUG, "CSSImportRule: Failed to decode CSS file: {} Unsupported encoding: {}", parsed_url, encoding);
return;
}
auto& decoder = maybe_decoder.release_value();
auto decoded_or_error = TextCodec::convert_input_to_utf8_using_given_decoder_unless_there_is_a_byte_order_mark(decoder, *byte_stream);
if (decoded_or_error.is_error()) {
dbgln_if(CSS_LOADER_DEBUG, "CSSImportRule: Failed to decode CSS file: {} Encoding was: {}", strong_this->url(), encoding);
dbgln_if(CSS_LOADER_DEBUG, "CSSImportRule: Failed to decode CSS file: {} Encoding was: {}", parsed_url, encoding);
return;
}
auto decoded = decoded_or_error.release_value();
auto* imported_style_sheet = parse_css_stylesheet(Parser::ParsingParams(*strong_this->m_document, strong_this->url()), decoded, strong_this->url(), strong_this->m_media_query_list);
auto* imported_style_sheet = parse_css_stylesheet(Parser::ParsingParams(*strong_this->m_document, parsed_url), decoded, parsed_url, strong_this->m_media_query_list);
// 5. Set importedStylesheet’s origin-clean flag to parentStylesheet’s origin-clean flag.
imported_style_sheet->set_origin_clean(parent_style_sheet->is_origin_clean());

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

@ -1,6 +1,6 @@
/*
* Copyright (c) 2021, the SerenityOS developers.
* Copyright (c) 2021-2024, Sam Atkins <sam@ladybird.org>
* Copyright (c) 2021-2025, Sam Atkins <sam@ladybird.org>
* Copyright (c) 2022, Andreas Kling <andreas@ladybird.org>
*
* SPDX-License-Identifier: BSD-2-Clause
@ -8,9 +8,9 @@
#pragma once
#include <LibURL/URL.h>
#include <LibWeb/CSS/CSSRule.h>
#include <LibWeb/CSS/CSSStyleSheet.h>
#include <LibWeb/CSS/URL.h>
#include <LibWeb/DOM/DocumentLoadEventDelayer.h>
namespace Web::CSS {
@ -21,13 +21,12 @@ class CSSImportRule final
GC_DECLARE_ALLOCATOR(CSSImportRule);
public:
[[nodiscard]] static GC::Ref<CSSImportRule> create(JS::Realm&, ::URL::URL, GC::Ptr<DOM::Document>, RefPtr<Supports>, Vector<NonnullRefPtr<MediaQuery>>);
[[nodiscard]] static GC::Ref<CSSImportRule> create(JS::Realm&, URL, GC::Ptr<DOM::Document>, RefPtr<Supports>, Vector<NonnullRefPtr<MediaQuery>>);
virtual ~CSSImportRule() = default;
::URL::URL const& url() const { return m_url; }
// FIXME: This should return only the specified part of the url. eg, "stuff/foo.css", not "https://example.com/stuff/foo.css".
String href() const { return m_url.to_string(); }
URL const& url() const { return m_url; }
String href() const { return m_url.url(); }
CSSStyleSheet* loaded_style_sheet() { return m_style_sheet; }
CSSStyleSheet const* loaded_style_sheet() const { return m_style_sheet; }
@ -37,7 +36,7 @@ public:
Optional<String> supports_text() const;
private:
CSSImportRule(JS::Realm&, ::URL::URL, GC::Ptr<DOM::Document>, RefPtr<Supports>, Vector<NonnullRefPtr<MediaQuery>>);
CSSImportRule(JS::Realm&, URL, GC::Ptr<DOM::Document>, RefPtr<Supports>, Vector<NonnullRefPtr<MediaQuery>>);
virtual void initialize(JS::Realm&) override;
virtual void visit_edges(Cell::Visitor&) override;
@ -49,7 +48,7 @@ private:
void fetch();
void set_style_sheet(GC::Ref<CSSStyleSheet>);
::URL::URL m_url;
URL m_url;
GC::Ptr<DOM::Document> m_document;
RefPtr<Supports> m_supports;
Vector<NonnullRefPtr<MediaQuery>> m_media_query_list;

9
Libraries/LibWeb/CSS/Parser/RuleParsing.cpp
View file

@ -162,13 +162,6 @@ GC::Ptr<CSSImportRule> Parser::convert_to_import_rule(AtRule const& rule)
return {};
}
// FIXME: Stop completing the URL here
auto resolved_url = complete_url(url->url());
if (!resolved_url.has_value()) {
dbgln_if(CSS_PARSER_DEBUG, "Failed to parse @import rule: Unable to complete `{}` as URL.", url->url());
return {};
}
tokens.discard_whitespace();
// FIXME: Implement layer support.
RefPtr<Supports> supports {};
@ -198,7 +191,7 @@ GC::Ptr<CSSImportRule> Parser::convert_to_import_rule(AtRule const& rule)
return {};
}
return CSSImportRule::create(realm(), resolved_url.release_value(), const_cast<DOM::Document*>(m_document.ptr()), supports, move(media_query_list));
return CSSImportRule::create(realm(), url.release_value(), const_cast<DOM::Document*>(m_document.ptr()), supports, move(media_query_list));
}
Optional<FlyString> Parser::parse_layer_name(TokenStream<ComponentValue>& tokens, AllowBlankLayerName allow_blank_layer_name)

2
Libraries/LibWeb/Dump.cpp
View file

@ -790,7 +790,7 @@ void dump_font_face_rule(StringBuilder& builder, CSS::CSSFontFaceRule const& rul
void dump_import_rule(StringBuilder& builder, CSS::CSSImportRule const& rule, int indent_levels)
{
indent(builder, indent_levels);
builder.appendff(" Document URL: {}\n", rule.url());
builder.appendff(" Document URL: {}\n", rule.url().to_string());
}
void dump_layer_block_rule(StringBuilder& builder, CSS::CSSLayerBlockRule const& layer_block, int indent_levels)

2
Services/WebContent/PageClient.cpp
View file

@ -848,7 +848,7 @@ static void gather_style_sheets(Vector<Web::CSS::StyleSheetIdentifier>& results,
// We can gather this anyway, and hope it loads later
results.append({
.type = Web::CSS::StyleSheetIdentifier::Type::ImportRule,
.url = import_rule->url().to_string(),
.url = import_rule->href(),
});
}
}

4
Tests/LibWeb/Text/expected/css/CSSImportRule-supportsText.txt
View file

@ -1,4 +1,4 @@
cssText: @import url("https://something.invalid/") supports(foo: bar);
cssText: @import url("https://something.invalid") supports(foo: bar);
supportsText: foo: bar
cssText: @import url("https://something.invalid/") supports((display: block) and (foo: bar));
cssText: @import url("https://something.invalid") supports((display: block) and (foo: bar));
supportsText: (display: block) and (foo: bar)

12
Tests/LibWeb/Text/expected/wpt-import/css/css-cascade/parsing/supports-import-parsing.txt
View file

@ -2,17 +2,17 @@ Harness status: OK
Found 22 tests
5 Pass
17 Fail
9 Pass
13 Fail
Pass @import url("nonexist.css") supports(); should be an invalid import rule due to an invalid supports() declaration
Pass @import url("nonexist.css") supports(foo: bar); should be an invalid import rule due to an invalid supports() declaration
Fail @import url("nonexist.css") supports(display:block); should be a valid supports() import rule
Fail @import url("nonexist.css") supports((display:flex)); should be a valid supports() import rule
Fail @import url("nonexist.css") supports(not (display: flex)); should be a valid supports() import rule
Fail @import url("nonexist.css") supports((display: flex) and (display: block)); should be a valid supports() import rule
Fail @import url("nonexist.css") supports((display: flex) or (display: block)); should be a valid supports() import rule
Fail @import url("nonexist.css") supports((display: flex) or (foo: bar)); should be a valid supports() import rule
Fail @import url("nonexist.css") supports(display: block !important); should be a valid supports() import rule
Pass @import url("nonexist.css") supports((display: flex) and (display: block)); should be a valid supports() import rule
Pass @import url("nonexist.css") supports((display: flex) or (display: block)); should be a valid supports() import rule
Pass @import url("nonexist.css") supports((display: flex) or (foo: bar)); should be a valid supports() import rule
Pass @import url("nonexist.css") supports(display: block !important); should be a valid supports() import rule
Pass @import url("nonexist.css") layer supports(); should be an invalid import rule due to an invalid supports() declaration
Pass @import url("nonexist.css") layer supports(foo: bar); should be an invalid import rule due to an invalid supports() declaration
Fail @import url("nonexist.css") layer(A) supports((display: flex) or (foo: bar)); should be a valid supports() import rule

12
Tests/LibWeb/Text/expected/wpt-import/css/css-conditional/js/conditional-CSSGroupingRule.txt
View file

@ -2,14 +2,14 @@ Harness status: OK
Found 34 tests
30 Pass
4 Fail
26 Pass
8 Fail
Pass @media is CSSGroupingRule
Pass @media rule type
Pass Empty @media rule length
Fail insertRule of @import into @media
Pass insertRule into empty @media at bad index
Pass insertRule into @media updates length
Fail insertRule into empty @media at bad index
Fail insertRule into @media updates length
Pass insertRule of valid @media into @media
Pass insertRule of valid style rule into @media
Fail insertRule of invalid @media into @media
@ -25,8 +25,8 @@ Pass @supports is CSSGroupingRule
Pass @supports rule type
Pass Empty @supports rule length
Fail insertRule of @import into @supports
Pass insertRule into empty @supports at bad index
Pass insertRule into @supports updates length
Fail insertRule into empty @supports at bad index
Fail insertRule into @supports updates length
Pass insertRule of valid @media into @supports
Pass insertRule of valid style rule into @supports
Fail insertRule of invalid @media into @supports

6
Tests/LibWeb/Text/expected/wpt-import/css/cssom/cssimportrule.txt
View file

@ -2,12 +2,12 @@ Harness status: OK
Found 11 tests
7 Pass
4 Fail
8 Pass
3 Fail
Pass CSSRule and CSSImportRule types
Pass Type of CSSRule#type and constant values
Pass Existence and writability of CSSRule attributes
Fail Values of CSSRule attributes
Pass Values of CSSRule attributes
Pass Existence and writability of CSSImportRule attributes
Fail Values of CSSImportRule attributes
Fail CSSImportRule : MediaList mediaText attribute should be updated due to [PutForwards]