LibWeb: Verify whitespace font has the glyph in font_for_space()
When inline layout emits a whitespace chunk, it previously selected the surrounding text's font without checking whether that font actually contains a glyph for the whitespace codepoint. On pages that use `@font-face` rules sharded by `unicode-range` (e.g. a Roboto webfont split across one file for Cyrillic letters and another for basic Latin), the shard covering the letters is picked for an adjacent space even though the space codepoint lives in a different shard. HarfBuzz then shapes the space with a font that has no glyph for it and emits `.notdef`, rendering spaces as tofu boxes. Check `contains_glyph(space_code_point)` on each candidate in `font_for_space()` and fall through to `FontCascadeList::font_for_code_point()` for the whitespace codepoint when no surrounding font has the glyph. Fixes whitespace rendering on web.telegram.org/a.
This commit is contained in:
parent
1f46651af5
commit
0bfe4677ae
5 changed files with 48 additions and 8 deletions
|
|
@ -631,10 +631,12 @@ bool TextNode::ChunkIterator::is_at_line_break_opportunity() const
|
|||
VERIFY_NOT_REACHED();
|
||||
}
|
||||
|
||||
Gfx::Font const& TextNode::ChunkIterator::font_for_space(size_t at_index) const
|
||||
Gfx::Font const& TextNode::ChunkIterator::font_for_space(size_t at_index, u32 space_code_point) const
|
||||
{
|
||||
auto has_glyph = [&](Gfx::Font const& font) { return font.contains_glyph(space_code_point); };
|
||||
|
||||
// 1. Prefer the last non-whitespace font in this node/run.
|
||||
if (m_last_non_whitespace_font && !m_last_non_whitespace_font->is_emoji_font())
|
||||
if (m_last_non_whitespace_font && !m_last_non_whitespace_font->is_emoji_font() && has_glyph(*m_last_non_whitespace_font))
|
||||
return *m_last_non_whitespace_font;
|
||||
|
||||
// 2. Look ahead to the next non-space to infer the base font of this run.
|
||||
|
|
@ -642,7 +644,7 @@ Gfx::Font const& TextNode::ChunkIterator::font_for_space(size_t at_index) const
|
|||
auto cp = m_view.code_point_at(i);
|
||||
if (!is_interword_space(cp) && cp != '\t' && cp != '\n') {
|
||||
auto const& font = m_font_cascade_list.font_for_code_point(cp);
|
||||
if (!font.is_emoji_font())
|
||||
if (!font.is_emoji_font() && has_glyph(font))
|
||||
return font;
|
||||
// Text is coming from an emoji face; we'll fall back to (3).
|
||||
break;
|
||||
|
|
@ -650,8 +652,8 @@ Gfx::Font const& TextNode::ChunkIterator::font_for_space(size_t at_index) const
|
|||
i = m_grapheme_segmenter.next_boundary(i).value_or(m_view.length_in_code_units());
|
||||
}
|
||||
|
||||
// 3. No text around (leading/trailing/all spaces) — pick the first *text* face in the cascade.
|
||||
return m_font_cascade_list.first_text_face();
|
||||
// 3. No text around (leading/trailing/all spaces) — pick a font with the glyph from the cascade.
|
||||
return m_font_cascade_list.font_for_code_point(space_code_point);
|
||||
}
|
||||
|
||||
Optional<TextNode::Chunk> TextNode::ChunkIterator::next_without_peek()
|
||||
|
|
@ -677,7 +679,7 @@ Optional<TextNode::Chunk> TextNode::ChunkIterator::next_without_peek()
|
|||
|
||||
auto const& expected_font_for = [&](u32 cp) -> Gfx::Font const& {
|
||||
return is_interword_space(cp)
|
||||
? font_for_space(m_current_index)
|
||||
? font_for_space(m_current_index, cp)
|
||||
: m_font_cascade_list.font_for_code_point(cp);
|
||||
};
|
||||
|
||||
|
|
@ -749,7 +751,7 @@ Optional<TextNode::Chunk> TextNode::ChunkIterator::next_without_peek()
|
|||
// Otherwise, commit the whitespace!
|
||||
m_current_index = next_grapheme_boundary();
|
||||
can_break_at_current_position = is_at_line_break_opportunity();
|
||||
auto const& space_font = font_for_space(m_current_index);
|
||||
auto const& space_font = font_for_space(m_current_index, code_point);
|
||||
if (auto result = try_commit_chunk(start_of_chunk, m_current_index, false, broken_on_tab, false, space_font, text_type); result.has_value())
|
||||
return result.release_value();
|
||||
continue;
|
||||
|
|
|
|||
|
|
@ -60,7 +60,7 @@ public:
|
|||
Optional<Chunk> try_commit_chunk(size_t start, size_t end, bool has_breaking_newline, bool has_breaking_tab, bool can_break_after, Gfx::Font const&, Gfx::GlyphRun::TextType) const;
|
||||
|
||||
[[nodiscard]] bool is_at_line_break_opportunity() const;
|
||||
[[nodiscard]] Gfx::Font const& font_for_space(size_t at_index) const;
|
||||
[[nodiscard]] Gfx::Font const& font_for_space(size_t at_index, u32 space_code_point) const;
|
||||
|
||||
bool const m_should_wrap_lines;
|
||||
bool const m_should_respect_linebreaks;
|
||||
|
|
|
|||
BIN
Tests/LibWeb/Assets/HashSansNoSpace.woff
Normal file
BIN
Tests/LibWeb/Assets/HashSansNoSpace.woff
Normal file
Binary file not shown.
|
|
@ -0,0 +1 @@
|
|||
space matches fallback: true
|
||||
37
Tests/LibWeb/Text/input/font-cascade-space-fallback.html
Normal file
37
Tests/LibWeb/Text/input/font-cascade-space-fallback.html
Normal file
|
|
@ -0,0 +1,37 @@
|
|||
<!DOCTYPE html>
|
||||
<style>
|
||||
@font-face {
|
||||
font-family: "HashFontNoSpace";
|
||||
src: url("../../Assets/HashSansNoSpace.woff");
|
||||
unicode-range: U+0041-005A;
|
||||
}
|
||||
.subset {
|
||||
font-family: "HashFontNoSpace", "SerenitySans";
|
||||
font-size: 100px;
|
||||
}
|
||||
.fallback-only {
|
||||
font-family: "SerenitySans";
|
||||
font-size: 100px;
|
||||
}
|
||||
span {
|
||||
white-space: pre;
|
||||
}
|
||||
</style>
|
||||
<script src="include.js"></script>
|
||||
<div>
|
||||
<span class="subset" id="letters-only">AB</span>
|
||||
<span class="subset" id="with-space">A B</span>
|
||||
<span class="fallback-only" id="ref-space"> </span>
|
||||
</div>
|
||||
<script>
|
||||
test(() => {
|
||||
// HashFontNoSpace covers only A–Z; the space in "A B" must come from the
|
||||
// SerenitySans fallback, not from HashFontNoSpace's .notdef glyph.
|
||||
let lettersOnlyWidth = document.getElementById("letters-only").getBoundingClientRect().width;
|
||||
let withSpaceWidth = document.getElementById("with-space").getBoundingClientRect().width;
|
||||
let fallbackSpaceWidth = document.getElementById("ref-space").getBoundingClientRect().width;
|
||||
|
||||
let spaceWidth = withSpaceWidth - lettersOnlyWidth;
|
||||
println("space matches fallback: " + (Math.abs(spaceWidth - fallbackSpaceWidth) < 2));
|
||||
});
|
||||
</script>
|
||||
Loading…
Reference in a new issue