From 3a45bba4e0317ceca65b7e693f3e9ddc6b7fea5a Mon Sep 17 00:00:00 2001 From: Mathis Wiehl Date: Sat, 18 Mar 2023 12:44:10 +0100 Subject: [PATCH] LibWeb: Load alternative font urls if others fail We don't support all parts of the font formats we assume as "supported" in the CSS parser. For example, if an open type font has a CFF table, we reject loading it. This meant that until now, when such an unsupported-supported font url was first in the list of urls, we couldn't load it at all, even when we would support a later url. To resolve that, try loading all font urls one after each other, in case we are not able to load the higher priority one. This also resolves a FIXME related to spec compliant url prioritization. Our CSS parser already filters and prioritizes font src urls in compliance with the spec. However, we still had to resort to brittle file extension matching, because some websites don't set the `format` and if the first url in a src list happened to be one we don't support, the font could not be loaded at all. This now is unnecessary because we can try and discard the urls instead. --- .../Libraries/LibWeb/CSS/Parser/Parser.cpp | 4 ++ .../Libraries/LibWeb/CSS/StyleComputer.cpp | 45 ++++++++----------- 2 files changed, 23 insertions(+), 26 deletions(-) diff --git a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp index 17fb6285ef..76716bf3d1 100644 --- a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp +++ b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp @@ -5435,6 +5435,10 @@ Vector Parser::parse_font_face_src(TokenStream // FIXME: Implement optional tech() function from CSS-Fonts-4. if (auto maybe_url = parse_url_function(first, AllowedDataUrlType::Font); maybe_url.has_value()) { auto url = maybe_url.release_value(); + if (!url.is_valid()) { + continue; + } + Optional format; source_tokens.skip_whitespace(); diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp index ff6f1bd7b5..e974f23f3a 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -44,13 +44,12 @@ StyleComputer::~StyleComputer() = default; class StyleComputer::FontLoader : public ResourceClient { public: - explicit FontLoader(StyleComputer& style_computer, FlyString family_name, AK::URL url) + explicit FontLoader(StyleComputer& style_computer, FlyString family_name, Vector urls) : m_style_computer(style_computer) , m_family_name(move(family_name)) + , m_urls(move(urls)) { - LoadRequest request; - request.set_url(move(url)); - set_resource(ResourceLoader::the().load_resource(Resource::Type::Generic, request)); + start_loading_next_url(); } virtual ~FontLoader() override { } @@ -59,7 +58,7 @@ public: { auto result = try_load_font(); if (result.is_error()) - return; + return start_loading_next_url(); m_vector_font = result.release_value(); m_style_computer.did_load_font(m_family_name); } @@ -88,6 +87,15 @@ public: } private: + void start_loading_next_url() + { + if (m_urls.is_empty()) + return; + LoadRequest request; + request.set_url(m_urls.take_first()); + set_resource(ResourceLoader::the().load_resource(Resource::Type::Generic, request)); + } + ErrorOr> try_load_font() { // FIXME: This could maybe use the format() provided in @font-face as well, since often the mime type is just application/octet-stream and we have to try every format @@ -108,6 +116,7 @@ private: StyleComputer& m_style_computer; FlyString m_family_name; RefPtr m_vector_font; + Vector m_urls; HashMap> mutable m_cached_fonts; }; @@ -1596,32 +1605,16 @@ void StyleComputer::load_fonts_from_sheet(CSSStyleSheet const& sheet) if (m_loaded_fonts.contains(font_face.font_family())) continue; - // NOTE: This is rather ad-hoc, we just look for the first valid - // source URL that's either a WOFF or OpenType file and try loading that. - // FIXME: Find out exactly which resources we need to load and how. - Optional candidate_url; + Vector urls; for (auto& source : font_face.sources()) { - if (!source.url.is_valid()) - continue; - - if (source.url.scheme() != "data") { - auto path = source.url.path(); - if (!path.ends_with(".woff"sv, AK::CaseSensitivity::CaseInsensitive) - && !path.ends_with(".ttf"sv, AK::CaseSensitivity::CaseInsensitive)) { - continue; - } - } - - candidate_url = source.url; - break; + // FIXME: These should be loaded relative to the stylesheet URL instead of the document URL. + urls.append(m_document->parse_url(source.url.to_deprecated_string())); } - if (!candidate_url.has_value()) + if (urls.is_empty()) continue; - LoadRequest request; - auto url = m_document->parse_url(candidate_url.value().to_deprecated_string()); - auto loader = make(const_cast(*this), font_face.font_family(), move(url)); + auto loader = make(const_cast(*this), font_face.font_family(), move(urls)); const_cast(*this).m_loaded_fonts.set(font_face.font_family().to_string(), move(loader)); } }