From 62612b86ade8d313042c2d06f1d3eace3fd583a2 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sat, 8 Aug 2026 21:39:28 +0200 Subject: [PATCH 1/2] fix(ui): emoji ZWJ sequences compose, and wrapping measures the drawn font MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🧑‍🚀 drew as a person standing next to a rocket. The cause was not the shaper — go-opentype/shape composes the sequence correctly when handed it whole — but the toolkit's fallback routing, which split runs per rune. The zero-width joiner is carried by several script faces (Thai, Arabic, Devanagari and Hebrew all ship U+200D for their own shaping), so first-face-wins routing sent it to the Thai face, and the sequence reached the shaper as three runs: person, joiner, rocket. The GSUB ligature never saw them together. Fixed in the owning library (go-widgets/toolkit v0.127.0): a rune that continues the preceding grapheme stays in the current run whenever that run's face can render it. This bumps to it, and asserts the result through the reader's own font chain — 🧑‍🚀 and 👨‍👩‍👦 each measure exactly one glyph's advance. While verifying, three callers turned out to violate the rule wrapMeasured's own doc comment states: they wrapped with a textFace (the x/image measurer) but drew with a toolkit.Font. detailContent, previewContent and previewHeaderLines now wrap with the font that actually draws the line, as the feed card already did. A line that "fits" one rasteriser can overflow the other and be clipped mid-word; composed emoji widened the gap, since a ZWJ sequence is two glyphs to the x/image measurer and one to the shaper that draws it. wrapText is deleted rather than left as a trap: it defaulted to the textFace measurer, which is exactly the wrong choice for every remaining caller, and wrapMeasured takes the measurer explicitly. 100% coverage held; race-clean. Verified on pixels: the composed astronaut is a single helmeted face in a live @NASA card, and 👩‍💻 / 👨‍🍳 / 👨‍👩‍👦 compose too. Co-Authored-By: Claude Opus 4.8 --- go.mod | 2 +- go.sum | 2 ++ ui/detail.go | 18 ++++++++++-------- ui/detail_test.go | 17 +++++++++++------ ui/emoji_test.go | 33 +++++++++++++++++++++++++++++++++ ui/preview_view.go | 9 ++++++--- 6 files changed, 63 insertions(+), 18 deletions(-) diff --git a/go.mod b/go.mod index 8d941c1..efabe87 100644 --- a/go.mod +++ b/go.mod @@ -25,7 +25,7 @@ require ( github.com/go-webengine/engine v0.3.2 github.com/go-widgets/mvvm v0.5.0 github.com/go-widgets/painter v0.2.0 - github.com/go-widgets/toolkit v0.124.0 + github.com/go-widgets/toolkit v0.127.0 github.com/go-widgets/tray v0.1.1 github.com/jezek/xgb v1.3.1 golang.org/x/image v0.44.0 diff --git a/go.sum b/go.sum index 8e9b1ea..639d59d 100644 --- a/go.sum +++ b/go.sum @@ -68,6 +68,8 @@ github.com/go-widgets/painter v0.2.0 h1:C7anwlKLYnlTeCK90xKYvy0srxYXqhuvyDFismlf github.com/go-widgets/painter v0.2.0/go.mod h1:ccmlkH2UmcXQh6rt9Fu2eDt2RI0B5V0POaGmUKeW0EQ= github.com/go-widgets/toolkit v0.124.0 h1:o+DOb6jZdEuq2y4yWNLKwXSxlbdPaFjBYO918HAXPCc= github.com/go-widgets/toolkit v0.124.0/go.mod h1:RfJFcIy5JU/klT40+hC1U6qB5DCOh3W92mi7pBvnfe0= +github.com/go-widgets/toolkit v0.127.0 h1:8ldvVeRiOQOBvTQXoSu1w8CmEnYLOlU3ZTK/gtqwQBw= +github.com/go-widgets/toolkit v0.127.0/go.mod h1:hekxU+C0moasoJPAI366VWNiZBbRSAjloC0tKnwXkWY= github.com/go-widgets/tray v0.1.1 h1:6wpYFpfkObLbxmrI8IyqcP528Ml+OrVT+msRjSpncDg= github.com/go-widgets/tray v0.1.1/go.mod h1:qftThNTUbYYsk/oWCjDvwDNRIojtYhY6zRZLP6c001A= github.com/goccy/go-yaml v1.19.2 h1:PmFC1S6h8ljIz6gMRBopkjP1TVT7xuwrButHID66PoM= diff --git a/ui/detail.go b/ui/detail.go index cb247f7..b1cdabd 100644 --- a/ui/detail.go +++ b/ui/detail.go @@ -57,10 +57,18 @@ func (s *Scene) detailContent() detailBody { titleFace := getFace(rpxOf(s, 22), true) bodyFace := getFace(rpxOf(s, 15), false) it := s.detail + // Wrap with the SAME fonts drawDetail draws these lines with (stock toolkit + // Labels carrying ttFont), not with the textFace whose heights size the rows. + // wrapMeasured says so explicitly, and the feed card already obeys it: a line + // that "fits" one rasteriser can overflow the other and be clipped mid-word. + // Composed emoji widened the gap — a ZWJ sequence is two glyphs to the + // x/image measurer and one to the shaper that actually draws it. + titleWrap := ttFont(true, rpxOf(s, 22)) + bodyWrap := ttFont(false, rpxOf(s, 15)) d := detailBody{ x: x, w: w, titleFace: titleFace, bodyFace: bodyFace, - titleLines: wrapText(titleFace, it.Title, w), - bodyLines: wrapText(bodyFace, stripHTML(it.Body), w), + titleLines: wrapMeasured(titleWrap.Measure, it.Title, w), + bodyLines: wrapMeasured(bodyWrap.Measure, stripHTML(it.Body), w), meta: metaLine(it), } gap := rpxOf(s, 10) @@ -180,12 +188,6 @@ func (s *Scene) detailHitTest(x, y int) Hit { return Hit{Kind: HitNone} } -// wrapText greedily word-wraps text to maxW pixels in face, preserving paragraph -// breaks ("\n"). A word longer than maxW is left un-broken on its own line. -func wrapText(face textFace, text string, maxW int) []string { - return wrapMeasured(face.width, text, maxW) -} - // wrapMeasured is the width-measurer-agnostic core of wrapText: it greedily // word-wraps text to maxW pixels, deciding each break with measure(s). Callers // whose text is rendered by a toolkit.Font (not a textFace) must pass that diff --git a/ui/detail_test.go b/ui/detail_test.go index 3c87bdf..666c464 100644 --- a/ui/detail_test.go +++ b/ui/detail_test.go @@ -116,22 +116,27 @@ func TestDetailScroll(t *testing.T) { } func TestWrapText(t *testing.T) { - f := getFace(14, false) - if wrapText(f, " ", 100) != nil { + // wrapMeasured is the only wrapper now: wrapText, which defaulted to the + // textFace measurer, was removed because every caller draws with a + // toolkit.Font and must wrap against THAT font's metrics. + wrap := func(text string, maxW int) []string { + return wrapMeasured(getFace(14, false).width, text, maxW) + } + if wrap(" ", 100) != nil { t.Fatal("blank -> nil") } - if got := wrapText(f, "hi", 10000); len(got) != 1 || got[0] != "hi" { + if got := wrap("hi", 10000); len(got) != 1 || got[0] != "hi" { t.Fatalf("fits = %v", got) } - if got := wrapText(f, "one two three four five six seven eight nine ten", 60); len(got) < 2 { + if got := wrap("one two three four five six seven eight nine ten", 60); len(got) < 2 { t.Fatalf("should wrap: %v", got) } // A single word wider than maxW stays on its own line. - if got := wrapText(f, "supercalifragilisticexpialidocious", 20); len(got) != 1 { + if got := wrap("supercalifragilisticexpialidocious", 20); len(got) != 1 { t.Fatalf("long word = %v", got) } // Paragraph break preserved as a blank line. - got := wrapText(f, "a\n\nb", 1000) + got := wrap("a\n\nb", 1000) if len(got) != 3 || got[1] != "" { t.Fatalf("paragraphs = %v", got) } diff --git a/ui/emoji_test.go b/ui/emoji_test.go index 8b05851..a78db67 100644 --- a/ui/emoji_test.go +++ b/ui/emoji_test.go @@ -75,6 +75,39 @@ func TestEmojiRenderInTheToolkitChain(t *testing.T) { } } +// TestZWJSequencesComposeToOneGlyph is the payoff of routing whole graphemes to +// one face (toolkit v0.127.0): the sequence reaches the shaper intact, so the +// GSUB ligature fires and 🧑‍🚀 is ONE astronaut rather than a person standing +// next to a rocket. Before, the joiner was claimed by the Thai face — which +// ships U+200D for its own shaping and sits earlier in the chain — and the +// sequence arrived as three separate runs. +func TestZWJSequencesComposeToOneGlyph(t *testing.T) { + f := ttFont(false, 32) + const ( + person = "\U0001F9D1" + rocket = "\U0001F680" + zwj = "‍" + ) + one := f.Measure(person) + if one <= 0 { + t.Fatal("the emoji face measured nothing") + } + if got := f.Measure(person + zwj + rocket); got != one { + t.Fatalf("🧑‍🚀 measures %d, want one glyph's %d (un-composed would be %d)", + got, one, f.Measure(person+rocket)) + } + // A three-part family sequence composes too. + family := "\U0001F468" + zwj + "\U0001F469" + zwj + "\U0001F466" + if got := f.Measure(family); got != one { + t.Fatalf("👨‍👩‍👦 measures %d, want one glyph's %d", got, one) + } + // The card wraps with this very measurer, so a composed sequence costs one + // glyph of line width rather than two. + if f.Measure("a"+person+zwj+rocket+"b") != f.Measure("a"+person+"b") { + t.Fatal("a sequence embedded in text did not compose") + } +} + // TestFormatCharactersAreNeverRasterised covers the defect that only becomes // visible once emoji render: an emoji sequence is held together by invisible // controls (🧑‍🚀 is person + ZWJ + rocket), and nothing in the chain has a glyph diff --git a/ui/preview_view.go b/ui/preview_view.go index 0c727c9..fc71d3d 100644 --- a/ui/preview_view.go +++ b/ui/preview_view.go @@ -370,10 +370,13 @@ func (s *Scene) previewContent() previewBody { titleFace := getFace(rpxOf(s, 17), true) bodyFace := getFace(rpxOf(s, 14), false) it := s.previewItem + // Wrap with the SAME fonts drawPreview draws these lines with (stock toolkit + // Labels carrying ttFont), not with the textFaces whose heights size the + // rows — see wrapMeasured. d := previewBody{ innerX: x, innerW: w, titleFace: titleFace, bodyFace: bodyFace, - titleLines: wrapText(titleFace, it.Title, w), - bodyLines: wrapText(bodyFace, stripHTML(it.Body), w), + titleLines: wrapMeasured(ttFont(true, rpxOf(s, 17)).Measure, it.Title, w), + bodyLines: wrapMeasured(ttFont(false, rpxOf(s, 14)).Measure, stripHTML(it.Body), w), meta: metaLine(it), } gap := rpxOf(s, 8) @@ -467,7 +470,7 @@ func (s *Scene) layoutPreview() { // fixed web-preview header. func (s *Scene) previewHeaderLines() []string { _, w := s.previewInner() - lines := wrapText(getFace(rpxOf(s, 17), true), s.previewItem.Title, w) + lines := wrapMeasured(ttFont(true, rpxOf(s, 17)).Measure, s.previewItem.Title, w) if len(lines) > 2 { lines = lines[:2] } From 7e80f9120c7defebdd814a8067fbfe49e155a7cf Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sat, 8 Aug 2026 21:41:46 +0200 Subject: [PATCH 2/2] style: gofmt ui/icons.go and ui/scene.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both were already gofmt-unclean on main; a directory-wide gofmt while working next door picked them up. Kept as its own commit so it does not hide inside a behaviour change — the diff is alignment only, no code. Co-Authored-By: Claude Opus 4.8 --- ui/icons.go | 18 +++++++++--------- ui/scene.go | 10 +++++----- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/ui/icons.go b/ui/icons.go index 9ca1134..5cb785b 100644 --- a/ui/icons.go +++ b/ui/icons.go @@ -36,15 +36,15 @@ func iconInset(r toolkit.Rect, frac int) toolkit.Rect { // Cached Iconoir icon lookups. Get is cheap, but caching avoids a registry // lookup per frame. Names verified present in iconoir v0.1.0. var ( - iconMenu = iconoir.MustGet("menu") // burger / open-sidebar - iconLock = iconoir.MustGet("lock") // auth-banner padlock - iconUser = iconoir.MustGet("user") // sidebar Accounts - iconList = iconoir.MustGet("list") // sidebar Network log - iconSettings = iconoir.MustGet("settings") // sidebar Settings (gear) - iconSearch = iconoir.MustGet("search") // topbar SearchEntry magnifier - iconPlus = iconoir.MustGet("plus") // sidebar "Browse newsgroups" + subscribe - iconRefresh = iconoir.MustGet("refresh-double") // browse view Refresh control + web preview Reload - iconCheck = iconoir.MustGet("check") // subscribed / complete marker + iconMenu = iconoir.MustGet("menu") // burger / open-sidebar + iconLock = iconoir.MustGet("lock") // auth-banner padlock + iconUser = iconoir.MustGet("user") // sidebar Accounts + iconList = iconoir.MustGet("list") // sidebar Network log + iconSettings = iconoir.MustGet("settings") // sidebar Settings (gear) + iconSearch = iconoir.MustGet("search") // topbar SearchEntry magnifier + iconPlus = iconoir.MustGet("plus") // sidebar "Browse newsgroups" + subscribe + iconRefresh = iconoir.MustGet("refresh-double") // browse view Refresh control + web preview Reload + iconCheck = iconoir.MustGet("check") // subscribed / complete marker iconNavLeft = iconoir.MustGet("nav-arrow-left") // web preview Back iconNavRight = iconoir.MustGet("nav-arrow-right") // web preview Forward iconZoomIn = iconoir.MustGet("zoom-in") // web preview zoom in diff --git a/ui/scene.go b/ui/scene.go index 6a608ab..b18b5c2 100644 --- a/ui/scene.go +++ b/ui/scene.go @@ -770,11 +770,11 @@ func (s *Scene) Settings() *settings.Settings { // stored as false and is not re-defaulted to single-tab on the next load. singleTab := s.BrowserSingleTab() return &settings.Settings{ - Profiles: s.Profiles, - Active: s.activeProf, - Theme: s.themeName, - CachePath: s.cachePath, - Accounts: s.EditedAccounts(), + Profiles: s.Profiles, + Active: s.activeProf, + Theme: s.themeName, + CachePath: s.cachePath, + Accounts: s.EditedAccounts(), BrowserSingleTab: &singleTab, HideBrowserChrome: s.BrowserChromeHidden(), ZoomInKey: s.BrowserZoomInKey(),