diff --git a/Cargo.lock b/Cargo.lock index 60708446f..6f7c8bf02 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5394,6 +5394,7 @@ dependencies = [ "serde_json", "sys-locale", "toml", + "unicode-bidi", "wordcraft-doc", "wordcraft-engine", "wordcraft-fonts", diff --git a/ROADMAP.md b/ROADMAP.md index 094b504bc..6ef6b3370 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -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 | diff --git a/contributors/people.toml b/contributors/people.toml index bda2f3bbc..6a405d053 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -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" diff --git a/crates/ui-egui/Cargo.toml b/crates/ui-egui/Cargo.toml index f4a0b6645..28a0c979f 100644 --- a/crates/ui-egui/Cargo.toml +++ b/crates/ui-egui/Cargo.toml @@ -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 } diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index ba2557df0..4eca0d304 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -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::() + 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) -> 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 { + 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> { + 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()); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 6fd52cb76..662516fbd 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -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, @@ -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()) } diff --git a/docs/ui-parity.md b/docs/ui-parity.md index ac498d8c5..b1c503f5c 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -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) |