Browse Source

LibWeb: Insert default font in font list before emoji font

This fixes a bug where, if a non-existent font family is specified in
CSS, whitespaces would be rendered using the emoji font, while letters
would use the default font. This issue occurred because the font was
resolved separately for each code point. Since the emoji font was listed
before the default font, it was chosen for whitespace characters due to
its inclusion of whitespace glyphs (at least in the Apple Color Emoji
font on macOS). This change resolves the issue by placing the default
font before the emoji font in the list.
Aliaksandr Kalenik 5 months ago
parent
commit
c7d6a7aafb

+ 10 - 8
Libraries/LibWeb/CSS/StyleComputer.cpp

@@ -1775,9 +1775,7 @@ RefPtr<Gfx::FontCascadeList const> StyleComputer::compute_font_for_style_values(
     auto* parent_element = element_to_inherit_style_from(element, pseudo_element);
 
     auto width = font_stretch.to_font_width();
-
     auto weight = font_weight.to_font_weight();
-    bool bold = weight > Gfx::FontWeight::Regular;
 
     // FIXME: Should be based on "user's default font size"
     CSSPixels font_size_in_px = 16;
@@ -1915,9 +1913,6 @@ RefPtr<Gfx::FontCascadeList const> StyleComputer::compute_font_for_style_values(
 
     // FIXME: Implement the full font-matching algorithm: https://www.w3.org/TR/css-fonts-4/#font-matching-algorithm
 
-    // Note: This is modified by the find_font() lambda
-    bool monospace = false;
-
     float const font_size_in_pt = font_size_in_px * 0.75f;
 
     auto find_font = [&](FlyString const& family) -> RefPtr<Gfx::FontCascadeList const> {
@@ -1954,7 +1949,6 @@ RefPtr<Gfx::FontCascadeList const> StyleComputer::compute_font_for_style_values(
         case Keyword::Monospace:
         case Keyword::UiMonospace:
             generic_font = Platform::GenericFont::Monospace;
-            monospace = true;
             break;
         case Keyword::Serif:
             generic_font = Platform::GenericFont::Serif;
@@ -2009,12 +2003,20 @@ RefPtr<Gfx::FontCascadeList const> StyleComputer::compute_font_for_style_values(
             font_list->extend(*other_font_list);
     }
 
+    auto default_font = Platform::FontPlugin::the().default_font(font_size_in_pt);
+    if (font_list->is_empty()) {
+        // This is needed to make sure we check default font before reaching to emojis.
+        font_list->add(*default_font);
+    }
+
     if (auto emoji_font = Platform::FontPlugin::the().default_emoji_font(font_size_in_pt); emoji_font) {
         font_list->add(*emoji_font);
     }
 
-    auto found_font = ComputedProperties::font_fallback(monospace, bold, font_size_in_pt);
-    font_list->set_last_resort_font(found_font->with_size(font_size_in_pt));
+    // The default font is already included in the font list, but we explicitly set it
+    // as the last-resort font. This ensures that if none of the specified fonts contain
+    // the requested code point, there is still a font available to provide a fallback glyph.
+    font_list->set_last_resort_font(*default_font);
 
     return font_list;
 }

+ 11 - 0
Tests/LibWeb/Layout/expected/non-existing-font-family.txt

@@ -0,0 +1,11 @@
+Viewport <#document> at (0,0) content-size 800x600 children: not-inline
+  BlockContainer <html> at (0,0) content-size 800x600 [BFC] children: not-inline
+    BlockContainer <body> at (8,8) content-size 784x17 children: inline
+      frag 0 from TextNode start: 0, length: 77, rect: [8,8 647.140625x17] baseline: 13.296875
+          "the same font is supposed to be used for all characters including whitespaces"
+      TextNode <#text>
+
+ViewportPaintable (Viewport<#document>) [0,0 800x600]
+  PaintableWithLines (BlockContainer<HTML>) [0,0 800x600]
+    PaintableWithLines (BlockContainer<BODY>) [8,8 784x17]
+      TextPaintable (TextNode<#text>)

+ 5 - 0
Tests/LibWeb/Layout/input/non-existing-font-family.html

@@ -0,0 +1,5 @@
+<style>
+* {
+    font-family: "some fake font name";
+}
+</style>the same font is supposed to be used for all characters including whitespaces