fix(ui): draw Arabic and other non-Latin comment text in balloons (#950)

* fix(ui): draw Arabic and other non-Latin comment text in balloons

Comment balloons and the Comments pane draw comment text and author
names with the interface faces (Inter, Source Sans, CJK), which have no
Arabic, Hebrew, Thai and so on: every such letter was a box. The app now
borrows a document fallback face that covers the comments, as the window
caption does (#840), checked once per document revision.

egui also lays bidi runs and words out left to right, so a right-to-left
comment read backwards. Read-only balloons now wrap the text by word
widths, put each row in display order (Unicode bidi runs, words of a
right-to-left run reversed, brackets mirrored) and set right-to-left
paragraphs against the right edge. Text without right-to-left letters is
laid out exactly as before. The editable fields (the selected balloon and
the Comments pane) keep logical order.

Also adds my contributor entry (sync_github_name).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* One contributor entry each

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Mohamed Youssef <ms.rightsteps@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
This commit is contained in:
authored and GitHub committed 2026-10-15 01:34:11 -04:00
1 parent 6a2bad8afa
commit b3b6c451e3
7 files changed
+243 -4

No files matched your search

Generated
+1
View File
@@ -5394,6 +5394,7 @@ dependencies = [
"serde_json",
"sys-locale",
"toml",
"unicode-bidi",
"wordcraft-doc",
"wordcraft-engine",
"wordcraft-fonts",
+1
View File
@@ -239,6 +239,7 @@ Ranked, to beta (detail and remaining milestones in [docs/roadmap.md](docs/roadm
| 2026-10-12 | Ribbon groups fit above their labels: View › Show splits its four check boxes into two columns (Hide White Space, #645, had pushed the fourth onto the "Show" label), Developer › Controls loses the gaps that ran into its label, and Layout › Paragraph (#654) no longer trips a debug assertion and sits 2.5 pt higher; a test draws every tab in every language and fails on any group taller than the ribbon |
| 2026-10-12 | Down and Up from the start of a line beside a table go into it (#1007): the caret, left of the first cell's text there, used to skip the whole table to the paragraph beyond it; it now lands in the cell under it or the nearest one, as in Word |
| 2026-10-12 | Window caption: a non-Latin file name is drawn with a document fallback face (or the CJK UI font), instead of `?` |
| 2026-10-12 | Comments: Arabic, Hebrew and other non-Latin comment text and author names are drawn with a document fallback face (or the CJK UI font) in the balloons and Comments pane, instead of boxes; read-only balloons show right-to-left text in reading order, set against the right edge |
| 2026-10-13 | Word 365 cover pages (#743): shapes placed relative to the page's height now sit where Word puts them (they were at the top of the column), and a freeform drawn back to its start without being closed is filled, so the cover's coloured panel and its white title show |
| 2026-10-12 | 32-bit Windows starts on OpenGL. DirectX 12 graphics pipeline creation returns "The parameter is incorrect" and the app exited before the window appeared |
| 2026-10-12 | New documents follow the system region (#838): A4 and centimetres outside the US (Letter in the CLDR Letter territories, inches in the US), lengths written with the region's decimal separator (`2,54 cm`) and typed with either; an explicit Measurement units choice wins; `file.options {"region"}` for agents |
+3
View File
@@ -100,6 +100,9 @@ sync_github_name = true
[people.leoncral]
sync_github_name = true
[people.ms-youssef]
sync_github_name = true
[people.finwarman]
real_name = "Fin Warman"
+1
View File
@@ -12,6 +12,7 @@ workspace = true
craft-ui = { git = "https://github.com/storytold/craft-libs", rev = "d27646a5f7c8b4fc85d9dd3213b8c88b58e0088d" }
wordcraft-geom = { workspace = true }
wordcraft-doc = { workspace = true }
unicode-bidi = { workspace = true }
wordcraft-fonts = { workspace = true }
wordcraft-layout = { workspace = true }
wordcraft-render = { workspace = true }
+171 -4
View File
@@ -1754,14 +1754,14 @@ fn balloons(app: &mut WordApp, ui: &mut Ui, painter: &egui::Painter, rects: &[Re
Rect::from_min_size(pos2(x0, top), vec2(w, app.canvas.balloon_h.max(24.0)))
} else {
let dim = |r: bool| if r { t.text_dim } else { t.text };
lines.push((painter.layout(author, semibold(fs), t.text, w - 16.0), 0.0));
lines.push((balloon_text(painter, author, semibold(fs), t.text, w - 16.0), 0.0));
let body = app.session.doc.plain_text(StoryRef::Part(c.part));
lines.push((painter.layout(body.trim().to_string(), regular(fs), dim(resolved), w - 16.0), 4.0));
lines.push((balloon_text(painter, body.trim().to_string(), regular(fs), dim(resolved), w - 16.0), 4.0));
for rid in &thread {
let Some(r) = app.session.doc.comments.get(rid) else { continue };
lines.push((painter.layout(author_name(&r.author), semibold(fs), t.text, w - 16.0), 9.0));
lines.push((balloon_text(painter, author_name(&r.author), semibold(fs), t.text, w - 16.0), 9.0));
let text = app.session.doc.plain_text(StoryRef::Part(r.part));
lines.push((painter.layout(text.trim().to_string(), regular(fs), dim(resolved), w - 16.0), 4.0));
lines.push((balloon_text(painter, text.trim().to_string(), regular(fs), dim(resolved), w - 16.0), 4.0));
}
let h = lines.iter().map(|(g, gap)| g.size().y + gap).sum::<f32>() + 12.0;
Rect::from_min_size(pos2(x0, top), vec2(w, h))
@@ -1840,6 +1840,108 @@ fn card_shapes(painter: &egui::Painter, t: &Tokens, card: Rect, color: Color32,
}
}
/// Does `text` hold right-to-left letters (Arabic, Hebrew, Syriac, Thaana, N'Ko …)?
fn has_rtl(text: &str) -> bool {
text.chars().any(|c| matches!(unicode_bidi::bidi_class(c), unicode_bidi::BidiClass::R | unicode_bidi::BidiClass::AL))
}
/// The row `line` of paragraph `p` of `para` in display order, for egui: the bidi runs left to
/// right as they are shown, and inside a right-to-left run its words reversed. The letters of each
/// word stay in logical order, because egui's shaper already runs an Arabic word right to left.
fn visual_row(info: &unicode_bidi::BidiInfo, p: &unicode_bidi::ParagraphInfo, para: &str, line: std::ops::Range<usize>) -> String {
// `visual_runs` indexes the line's first byte: an empty line (an empty last row) would panic.
if line.start >= line.end || line.end > p.range.end {
return String::new();
}
let (levels, runs) = info.visual_runs(p, line);
let mut out = String::new();
for run in runs {
let rtl = levels.get(run.start).is_some_and(|l| l.is_rtl());
// Runs lie on character boundaries; a missing slice is skipped, never a panic.
let Some(piece) = para.get(run) else { continue };
if rtl {
let mut words: Vec<&str> = piece.split_inclusive(char::is_whitespace).collect();
words.reverse();
for w in words {
// The space that followed a word now goes before it.
let word = w.trim_end_matches(char::is_whitespace);
out.push_str(w.get(word.len()..).unwrap_or_default());
out.extend(word.chars().map(mirrored));
}
} else {
out.push_str(piece);
}
}
out
}
/// `c` as drawn in a right-to-left run: brackets face the other way (Unicode's Bidi_Mirrored
/// pairs that interface text uses), since egui draws the character it is given.
fn mirrored(c: char) -> char {
match c {
'(' => ')',
')' => '(',
'[' => ']',
']' => '[',
'{' => '}',
'}' => '{',
'<' => '>',
'>' => '<',
'«' => '»',
'»' => '«',
'‹' => '›',
'›' => '‹',
_ => c,
}
}
/// A read-only balloon's text, wrapped to `width`. egui joins Arabic letters but places them left
/// to right, so a right-to-left comment read its words backwards. Each wrapped row is put into
/// display order (Unicode bidi, per paragraph) and a right-to-left paragraph's rows are set against
/// the right edge. Text without right-to-left letters is laid out exactly as before.
fn balloon_text(painter: &egui::Painter, text: String, font: egui::FontId, color: Color32, width: f32) -> std::sync::Arc<egui::Galley> {
if !has_rtl(&text) {
return painter.layout(text, font, color, width);
}
let format = egui::TextFormat::simple(font.clone(), color);
let mut job = egui::text::LayoutJob::default();
let mut rows: Vec<(String, bool)> = Vec::new();
for para in text.split('\n') {
let info = unicode_bidi::BidiInfo::new(para, None);
if info.paragraphs.is_empty() {
rows.push((String::new(), false));
}
for p in &info.paragraphs {
let rtl = p.level.is_rtl();
// Wrap in logical order at spaces, by the words' widths (the order doesn't change a word's
// width), then put each row in display order. egui's own rows can't be used: it draws a
// lam-alef pair as one glyph, so its glyph count isn't the character count.
let mut start = p.range.start;
let mut end = start;
let mut used = 0.0;
for word in para.get(p.range.clone()).unwrap_or_default().split_inclusive(char::is_whitespace) {
let w = painter.layout_no_wrap(word.trim_end().to_string(), font.clone(), color).size().x;
let gap = painter.layout_no_wrap(word.get(word.trim_end().len()..).unwrap_or_default().to_string(), font.clone(), color).size().x;
if end > start && used + w > width {
rows.push((visual_row(&info, p, para, start..end).trim().to_string(), rtl));
start = end;
used = 0.0;
}
used += w + gap;
end += word.len();
}
rows.push((visual_row(&info, p, para, start..end).trim().to_string(), rtl));
}
}
let last = rows.len().saturating_sub(1);
for (i, (row, rtl)) in rows.into_iter().enumerate() {
let lead = if rtl { (width - painter.layout_no_wrap(row.clone(), font.clone(), color).size().x).max(0.0) } else { 0.0 };
let row = if i < last { row + "\n" } else { row };
job.append(&row, lead, format.clone());
}
painter.layout_job(job)
}
/// The selected balloon: the comment and its replies as editable fields (each edit is one undo
/// step via `review.editComment`), an optional reply field, and Reply / Resolve / Delete.
/// `card` carries last frame's size; returns this frame's.
@@ -3455,6 +3557,71 @@ mod tests {
/// #167: a comment balloon edits in place like Word's: click it, type, leave it, and the
/// comment has the new text as one undo step.
/// Lays `text` out as a read-only balloon does, with a document face that covers Arabic, and
/// returns each glyph's character and left edge.
fn balloon_glyphs(text: &str, width: f32) -> Option<Vec<(char, f32)>> {
let face = wordcraft_fonts::FontDb::global().fallback_for('ش', u32::MAX)?;
let ctx = egui::Context::default();
let mut fonts = crate::theme::font_definitions(false, false);
crate::theme::add_fallback_faces(&mut fonts, &[face]);
ctx.set_fonts(fonts);
ctx.run_ui(egui::RawInput::default(), |_| {}).drop_without_applying_deltas();
let mut out = Vec::new();
ctx.run_ui(egui::RawInput::default(), |ui| {
let g = balloon_text(ui.painter(), text.to_string(), egui::FontId::proportional(14.0), Color32::BLACK, width);
for r in &g.rows {
out.extend(r.row.glyphs.iter().map(|gl| (gl.chr, r.pos.x + gl.pos.x)));
}
})
.drop_without_applying_deltas();
Some(out)
}
/// Where egui draws a right-to-left balloon's words, measured: the first word on the right.
#[test]
fn a_right_to_left_balloon_reads_from_the_right() {
let x = |g: &[(char, f32)], c: char| g.iter().find(|(ch, _)| *ch == c).map(|(_, x)| *x).unwrap();
let Some(g) = balloon_glyphs("شركة اي", 400.0) else { return };
assert!(x(&g, 'ش') > x(&g, 'ي'), "the first word is right of the second: {g:?}");
assert!(x(&g, 'ش') > 300.0, "the line is set against the right edge: {g:?}");
// A Latin word inside keeps its letters left to right; its brackets face it.
let g = balloon_glyphs("الربط (APIs) هنا", 400.0).unwrap();
assert!(x(&g, 'A') < x(&g, 's'));
assert!(x(&g, 'ط') > x(&g, 'A') && x(&g, 'A') > x(&g, 'ه'), "{g:?}");
// The opening bracket, on the right in Arabic, is drawn mirrored: `(APIs)` on screen.
assert!(x(&g, '(') < x(&g, 'A') && x(&g, ')') > x(&g, 's'), "brackets face the word: {g:?}");
// A paragraph that starts in Latin runs left to right.
let g = balloon_glyphs("@Ahmed برجاء مراجعة", 400.0).unwrap();
assert!(x(&g, '@') < x(&g, 'ب') && x(&g, '@') < 50.0, "{g:?}");
assert!(x(&g, 'ب') > x(&g, 'م'), "the Arabic words still read from the right: {g:?}");
// Wrapped rows stay inside the width.
let long = "بيان الأنظمة المطلوب ربطها ومتطلبات التكامل وواجهات الربط البرمجي وآلية تبادل البيانات";
let g = balloon_glyphs(long, 200.0).unwrap();
assert!(g.iter().all(|(_, x)| *x <= 200.0), "{g:?}");
}
#[test]
fn balloon_rows_put_right_to_left_words_in_display_order() {
let row = |text: &str| {
let info = unicode_bidi::BidiInfo::new(text, None);
let p = info.paragraphs.first().unwrap();
visual_row(&info, p, text, p.range.clone())
};
// Words swap, each word's letters stay as they are (egui shapes the word).
assert_eq!(row("شركة اي للكهرباء"), "للكهرباء اي شركة");
// A Latin name and a number inside Arabic keep their own order.
assert_eq!(row("يجب ربط Odoo 19 هنا"), "هنا Odoo 19 ربط يجب");
assert_eq!(row("Should include Meters system API"), "Should include Meters system API");
assert!(!has_rtl("Should include Meters system API"));
assert!(has_rtl("נא לבדוק"));
// An empty row at the end of the text (egui's last row after a wrap) is empty, not a panic.
let text = "شركة اي";
let info = unicode_bidi::BidiInfo::new(text, None);
let p = info.paragraphs.first().unwrap();
assert_eq!(visual_row(&info, p, text, text.len()..text.len()), "");
assert_eq!(visual_row(&info, p, text, 0..text.len() + 5), "");
}
#[test]
fn a_balloon_edits_its_comment() {
let mut a = WordApp::new(wordcraft_engine::Session::new(wordcraft_doc::Document::new()), Default::default());
+65
View File
@@ -469,6 +469,9 @@ pub struct WordApp {
/// CJK text is (about to be) on screen in a non-CJK interface, e.g. the language names in
/// File ▸ Options: load the installed CJK fallback font if no embedded face covers it.
pub(crate) want_system_cjk: bool,
/// The document (id, revision) whose comment text [`WordApp::ensure_comment_fonts`] last
/// looked at.
comment_fonts_for: Option<(u64, u64)>,
fonts_frames: u32,
/// The effective interface theme last installed in egui ([`theme::apply`]).
applied_theme: Option<theme::Appearance>,
@@ -595,6 +598,7 @@ impl WordApp {
fonts_korean: false,
fonts_system_cjk: false,
want_system_cjk: false,
comment_fonts_for: None,
fonts_frames: 0,
applied_theme: None,
native_scale: 1.0,
@@ -1678,6 +1682,35 @@ impl WordApp {
}
}
/// Comment balloons and the Comments pane draw comment text and author names with the
/// interface faces, which have no Arabic, Hebrew, Thai …: each such letter was drawn as a box.
/// Borrow the document fonts' fallback faces for them, as the caption does. Looked at once per
/// document revision; CJK text uses the CJK UI font instead.
fn ensure_comment_fonts(&mut self) {
let key = (self.session.document_id(), self.session.rev());
if self.comment_fonts_for == Some(key) {
return;
}
self.comment_fonts_for = Some(key);
let doc = &self.session.doc;
let mut chars = std::collections::BTreeSet::new();
for c in doc.comments.values() {
chars.extend(c.author.chars());
chars.extend(doc.plain_text(wordcraft_doc::StoryRef::Part(c.part)).chars());
}
if chars.iter().any(|c| wordcraft_fonts::is_cjk(*c)) {
self.want_system_cjk = true;
}
let text: String = chars.into_iter().collect();
let new = theme::faces_for_preedit(&text, &self.canvas.ime_faces, theme::caption_char_covered, |c| {
wordcraft_fonts::FontDb::global().fallback_for(c, u32::MAX)
});
if !new.is_empty() {
self.canvas.ime_faces.extend(new);
self.styled = false;
}
}
/// Document title for the title bar (and the suggested file name). An untitled document is
/// named in the interface language, as `Document1` is in English.
pub fn title_stem(&self) -> String {
@@ -1724,6 +1757,7 @@ impl WordApp {
// language names in Options, or a file name the caption is drawing. A file opened later
// in this frame (dialog, drop, control channel) is picked up again at the end.
self.ensure_caption_fonts();
self.ensure_comment_fonts();
self.install_ui_fonts(ctx, lang);
if let Some(answer) = self.desktop_theme.take_change() {
self.desktop_dark = Some(answer);
@@ -1833,6 +1867,7 @@ impl WordApp {
}
// The open above may have changed the file name the caption draws.
self.ensure_caption_fonts();
self.ensure_comment_fonts();
self.install_ui_fonts(ctx, lang);
self.report_dirty();
}
@@ -2496,6 +2531,36 @@ mod tests {
assert!(cjk.canvas.ime_faces.is_empty(), "CJK collections are not copied into the fallback list");
}
/// Comment balloons draw comment text and author names with the interface faces, which have
/// no Arabic, so an Arabic comment was all boxes. A document with one borrows a document
/// fallback face that covers it, once per document revision.
#[test]
fn arabic_comment_text_is_drawn_in_the_balloons() {
let ctx = egui::Context::default();
let frame = |app: &mut WordApp| {
ctx.run_ui(egui::RawInput::default(), |ui| app.logic(ui.ctx())).drop_without_applying_deltas();
};
let mut latin = app();
latin.run("text.insert", json!({"text": "Scope of works"})).unwrap();
latin.run("select.text", json!({"text": "Scope"})).unwrap();
latin.run("review.newComment", json!({"text": "Please check"})).unwrap();
frame(&mut latin);
assert!(latin.canvas.ime_faces.is_empty(), "a Latin comment needs no extra face");
let mut arabic = app();
arabic.run("text.insert", json!({"text": "Scope of works"})).unwrap();
arabic.run("select.text", json!({"text": "Scope"})).unwrap();
arabic.run("review.newComment", json!({"text": "لتحديد نطاق الأعمال"})).unwrap();
frame(&mut arabic);
let letter = 'ن';
if wordcraft_fonts::FontDb::global().fallback_for(letter, u32::MAX).is_some_and(|f| f.data().len() <= 16 << 20) {
assert!(arabic.canvas.ime_faces.iter().any(|f| f.covers(letter)), "the balloons borrow a face that covers the comment");
}
let n = arabic.canvas.ime_faces.len();
frame(&mut arabic);
assert_eq!(arabic.canvas.ime_faces.len(), n, "the face is added once");
}
fn app() -> WordApp {
WordApp::new(Session::new(wordcraft_doc::Document::new()), Services::default())
}
+1
View File
@@ -115,6 +115,7 @@ Document title opens Rename Document; the account badge opens Options › Genera
| 2026-10-11 | trivial | Quick Access Toolbar KeyTips (#620): 1–9, zero-prefixed numbers for longer toolbars, disabled commands stay disabled, Escape cancels, badge keystrokes never type into the document; persisted File › Options switch |
| 2026-10-11 | trivial | Ribbon buttons that run a command (alignment, Bold, styles…) hand keyboard focus back to the document, so typing continues without another click; they also report their name and enabled state to screen readers (#686) |
| 2026-10-12 | trivial | Keytips use Word for Windows' tab letters, as observed there: File F (opens the Backstage), Design G, References S, Review R, View W, Help Y (were L, R, S, V and none) |
| 2026-10-12 | trivial | Comments: comment text and author names in a script the interface fonts lack (Arabic, Hebrew, and the rest; CJK via the CJK UI font) are drawn with a fallback face in the balloons and Comments pane instead of boxes; read-only balloons show right-to-left text in reading order (words, bidi runs and brackets), set against the right edge |
| 2026-10-13 | trivial | Keytips reach the contextual table tabs with Word's two-letter badges: Alt, J, T opens Table Design and Alt, J, L Table Layout; after J only the J badges stay (#1183) |
| 2026-10-12 | trivial | Shift+F10 opens the page's context menu below the caret, with the spelling suggestions and synonyms for the word there; `ui.contextMenu` for agents (#821). The Menu key itself isn't passed on by the windowing layer yet |
| 2026-10-12 | trivial | Search: Alt+Q (and Alt, Q) opens the Search box from anywhere, as in Word; the key's own "q" isn't typed; every word of a query matches, so "insert table" finds Insert › Tables › Table; `tools.search` lists matches for agents (#819) |