diff --git a/frontend/playwright/ui/specs/text-editor-v3.spec.js b/frontend/playwright/ui/specs/text-editor-v3.spec.js index 53b439ab19..963d48a9ca 100644 --- a/frontend/playwright/ui/specs/text-editor-v3.spec.js +++ b/frontend/playwright/ui/specs/text-editor-v3.spec.js @@ -129,6 +129,63 @@ test("BUG 10467 - Auto-width text captures every typed character", async ({ await workspace.waitForSelectedShapeName("hello world"); }); +test.describe("BUG 10910 - Text is not replaced when there is a selection", () => { + // Non-ascii on purpose: selection offsets are counted in characters. + test("Typing over a selection replaces it", async ({ page }) => { + const workspace = new WasmWorkspacePage(page, { textEditor: true }); + await workspace.setupEmptyFile(); + await workspace.goToWorkspace(); + await workspace.waitForFirstRender(); + + await workspace.createAutoWidthTextShape(200, 150, "Añadir"); + + await page.keyboard.press("ControlOrMeta+a"); + await page.keyboard.type("nuevo"); + + await workspace.textEditor.stopEditing(); + + await workspace.layers.getByTestId("layer-row").first().click(); + await workspace.waitForSelectedShapeName("nuevo"); + }); + + test("Typing over a selection that contains emoji replaces it", async ({ + page, + }) => { + const workspace = new WasmWorkspacePage(page, { textEditor: true }); + await workspace.setupEmptyFile(); + await workspace.goToWorkspace(); + await workspace.waitForFirstRender(); + + await workspace.createAutoWidthTextShape(200, 150, "Hola 😀"); + + await page.keyboard.press("ControlOrMeta+a"); + await page.keyboard.type("ok"); + + await workspace.textEditor.stopEditing(); + + await workspace.layers.getByTestId("layer-row").first().click(); + await workspace.waitForSelectedShapeName("ok"); + }); + + test("Backspace deletes the selection", async ({ page }) => { + const workspace = new WasmWorkspacePage(page, { textEditor: true }); + await workspace.setupEmptyFile(); + await workspace.goToWorkspace(); + await workspace.waitForFirstRender(); + + await workspace.createAutoWidthTextShape(200, 150, "Añadir texto"); + + await page.keyboard.press("ControlOrMeta+a"); + await page.keyboard.press("Backspace"); + await page.keyboard.type("ok"); + + await workspace.textEditor.stopEditing(); + + await workspace.layers.getByTestId("layer-row").first().click(); + await workspace.waitForSelectedShapeName("ok"); + }); +}); + test("BUG 10531 - Entering the editor auto-selects the whole text", async ({ page, }) => { diff --git a/render-wasm/src/globals.rs b/render-wasm/src/globals.rs index cd0fcf0bbf..b1de8c9271 100644 --- a/render-wasm/src/globals.rs +++ b/render-wasm/src/globals.rs @@ -40,6 +40,17 @@ pub(crate) fn get_render_state() -> &'static mut RenderState { } } +#[inline(always)] +pub(crate) fn current_browser() -> u8 { + unsafe { + if DESIGN_STATE.is_null() { + 0 + } else { + (*DESIGN_STATE).current_browser + } + } +} + #[inline(always)] pub(crate) fn has_render_state() -> bool { unsafe { !RENDER_STATE.is_null() } diff --git a/render-wasm/src/render/text_editor.rs b/render-wasm/src/render/text_editor.rs index 8ed53bb436..169f4cb735 100644 --- a/render-wasm/src/render/text_editor.rs +++ b/render-wasm/src/render/text_editor.rs @@ -156,12 +156,13 @@ fn calculate_cursor_rect( .map(|span| span.text.chars().count()) .sum(); + // Skia ranges are UTF-16 code units, not characters. let (cursor_x, cursor_y, cursor_width, cursor_height) = if para_char_count == 0 { // Empty paragraph - use default height (0.0, 0.0, 1.0, laid_out_para.height()) } else if char_pos == 0 { let rects = laid_out_para.get_rects_for_range( - 0..1, + 0..para.char_utf16_len_at(0), RectHeightStyle::Max, RectWidthStyle::Tight, ); @@ -172,8 +173,10 @@ fn calculate_cursor_rect( (0.0, 0.0, 1.0, laid_out_para.height()) } } else if char_pos >= para_char_count { + let last_char = para_char_count.saturating_sub(1); + let last_start = para.char_offset_to_utf16(last_char); let rects = laid_out_para.get_rects_for_range( - para_char_count.saturating_sub(1)..para_char_count, + last_start..last_start + para.char_utf16_len_at(last_char), RectHeightStyle::Max, RectWidthStyle::Tight, ); @@ -189,8 +192,9 @@ fn calculate_cursor_rect( ) } } else { + let utf16_pos = para.char_offset_to_utf16(char_pos); let rects = laid_out_para.get_rects_for_range( - char_pos..char_pos + 1, + utf16_pos..utf16_pos + para.char_utf16_len_at(char_pos), RectHeightStyle::Max, RectWidthStyle::Tight, ); @@ -264,7 +268,7 @@ fn calculate_selection_rects( if range_start < range_end { use skia_safe::textlayout::{RectHeightStyle, RectWidthStyle}; let text_boxes = laid_out_para.get_rects_for_range( - range_start..range_end, + para.char_offset_to_utf16(range_start)..para.char_offset_to_utf16(range_end), RectHeightStyle::Max, RectWidthStyle::Tight, ); diff --git a/render-wasm/src/shapes/text.rs b/render-wasm/src/shapes/text.rs index f5f58f5a80..d5506850c0 100644 --- a/render-wasm/src/shapes/text.rs +++ b/render-wasm/src/shapes/text.rs @@ -653,38 +653,14 @@ impl TextContent { let position_with_affinity = layout_paragraph.get_glyph_position_at_coordinate((para_pt.x, para_pt.y)); if let Some(paragraph) = self.paragraphs().get(paragraph_index) { - // Computed position keeps the current position in terms - // of number of characters of text. This is used to know - // in which span we are. - let mut computed_position: usize = 0; - - // If paragraph has no spans, default to span 0, offset 0 - if !paragraph.children().is_empty() { - for span in paragraph.children() { - let length = span.text.chars().count(); - let start_position = computed_position; - let end_position = computed_position + length; - let current_position = position_with_affinity.position as usize; - - // Handle empty spans: if the span is empty and current position - // matches the start, this is the right span - if length == 0 && current_position == start_position { - break; - } - - if start_position <= current_position - && end_position >= current_position - { - break; - } - computed_position += length; - } - } + // Skia reports UTF-16 code units, the model counts characters. + let offset = + paragraph.utf16_offset_to_char(position_with_affinity.position as usize); return Some(TextPositionWithAffinity::new( position_with_affinity, paragraph_index, - position_with_affinity.position as usize, + offset, )); } } @@ -1160,6 +1136,53 @@ impl Paragraph { &mut self.children } + fn char_count(&self) -> usize { + self.children + .iter() + .map(|span| span.text.chars().count()) + .sum() + } + + /// Translate a character offset into the UTF-16 offset Skia indexes by. + /// Both differ on astral-plane characters (emoji) and whenever a text + /// transform changes the length of the laid out text (`ß` -> `SS`). + pub fn char_offset_to_utf16(&self, char_offset: usize) -> usize { + let mut remaining = char_offset; + let mut utf16 = 0; + for span in &self.children { + if remaining == 0 { + break; + } + let span_len = span.text.chars().count(); + let take = remaining.min(span_len); + let prefix: String = span.text.chars().take(take).collect(); + utf16 += span.transform_text(&prefix).encode_utf16().count(); + remaining -= take; + } + utf16 + } + + /// Translate a UTF-16 offset coming from Skia into a character offset. + /// An offset inside a character rounds up, so it never splits a glyph. + pub fn utf16_offset_to_char(&self, utf16_offset: usize) -> usize { + let (mut low, mut high) = (0, self.char_count()); + while low < high { + let middle = (low + high) / 2; + if self.char_offset_to_utf16(middle) < utf16_offset { + low = middle + 1; + } else { + high = middle; + } + } + low + } + + /// UTF-16 length of the character at `char_offset`, so a caret range covers + /// the whole glyph. + pub fn char_utf16_len_at(&self, char_offset: usize) -> usize { + self.char_offset_to_utf16(char_offset + 1) - self.char_offset_to_utf16(char_offset) + } + pub fn line_height(&self) -> f32 { self.line_height } @@ -1377,9 +1400,8 @@ impl TextSpan { format!("{}", self.font_family) } - pub fn apply_text_transform(&self) -> String { - let browser = crate::with_state!(state, { state.current_browser }); - let text = process_ignored_chars(&self.text, browser); + pub fn transform_text(&self, text: &str) -> String { + let text = process_ignored_chars(text, crate::globals::current_browser()); match self.text_transform { Some(TextTransform::Uppercase) => text.to_uppercase(), Some(TextTransform::Lowercase) => text.to_lowercase(), @@ -1388,6 +1410,10 @@ impl TextSpan { } } + pub fn apply_text_transform(&self) -> String { + self.transform_text(&self.text) + } + pub fn scale_content(&mut self, value: f32) { self.font_size *= value; } @@ -1732,4 +1758,85 @@ mod tests { "ab" ); } + + fn test_paragraph(texts: &[&str]) -> Paragraph { + let spans = texts + .iter() + .map(|text| { + TextSpan::new( + text.to_string(), + FontFamily::new(Uuid::nil(), 400, crate::shapes::FontStyle::Normal), + 14.0, + 1.2, + 0.0, + None, + None, + TextDirection::LTR, + 400, + Uuid::nil(), + vec![], + ) + }) + .collect(); + + Paragraph::new( + TextAlign::Left, + TextDirection::LTR, + None, + None, + 1.2, + 0.0, + spans, + ) + } + + #[test] + fn char_offsets_match_utf16_offsets_for_bmp_text() { + let para = test_paragraph(&["Añadir"]); + for offset in 0..=6 { + assert_eq!(para.char_offset_to_utf16(offset), offset); + assert_eq!(para.utf16_offset_to_char(offset), offset); + } + } + + #[test] + fn char_offsets_account_for_astral_characters() { + let para = test_paragraph(&["a", "😀b"]); + + assert_eq!(para.char_offset_to_utf16(0), 0); + assert_eq!(para.char_offset_to_utf16(1), 1); + assert_eq!(para.char_offset_to_utf16(2), 3); + assert_eq!(para.char_offset_to_utf16(3), 4); + + assert_eq!(para.utf16_offset_to_char(0), 0); + assert_eq!(para.utf16_offset_to_char(1), 1); + assert_eq!(para.utf16_offset_to_char(3), 2); + assert_eq!(para.utf16_offset_to_char(4), 3); + } + + #[test] + fn utf16_offset_inside_a_surrogate_pair_rounds_to_a_char_boundary() { + let para = test_paragraph(&["a😀b"]); + assert_eq!(para.utf16_offset_to_char(2), 2); + } + + #[test] + fn char_offsets_account_for_text_transforms() { + // Skia lays out the transformed text, where "Straße" is "STRASSE". + let mut para = test_paragraph(&["Straße"]); + para.children_mut()[0].text_transform = Some(TextTransform::Uppercase); + + assert_eq!(para.char_offset_to_utf16(4), 4); + assert_eq!(para.char_offset_to_utf16(6), 7); + assert_eq!(para.char_utf16_len_at(4), 2); + assert_eq!(para.utf16_offset_to_char(7), 6); + } + + #[test] + fn char_utf16_len_at_covers_the_whole_glyph() { + let para = test_paragraph(&["a😀b"]); + assert_eq!(para.char_utf16_len_at(0), 1); + assert_eq!(para.char_utf16_len_at(1), 2); + assert_eq!(para.char_utf16_len_at(2), 1); + } } diff --git a/render-wasm/src/state/text_editor.rs b/render-wasm/src/state/text_editor.rs index 3e46f77116..ecb792f775 100644 --- a/render-wasm/src/state/text_editor.rs +++ b/render-wasm/src/state/text_editor.rs @@ -447,10 +447,8 @@ impl TextEditorState { let Some(_last_text_span) = last_paragraph.children().last() else { return false; }; - let mut offset = 0; - for span in last_paragraph.children() { - offset += span.text.len(); - } + // Offsets are counted in characters, not bytes. + let offset = text_helpers::paragraph_char_count(last_paragraph); self.extend_selection_from_position(&TextPositionWithAffinity::new( PositionWithAffinity { position: offset as i32, diff --git a/render-wasm/src/wasm/text/helpers.rs b/render-wasm/src/wasm/text/helpers.rs index 32ed8238d7..fedfdba215 100644 --- a/render-wasm/src/wasm/text/helpers.rs +++ b/render-wasm/src/wasm/text/helpers.rs @@ -459,11 +459,13 @@ pub fn delete_selection_range(text_content: &mut TextContent, selection: &TextSe let end = selection.end(); let paragraphs = text_content.paragraphs_mut(); - if start.paragraph >= paragraphs.len() { + if paragraphs.is_empty() || start.paragraph >= paragraphs.len() { return; } - if start.paragraph == end.paragraph { + let end_paragraph = end.paragraph.min(paragraphs.len() - 1); + + if start.paragraph == end_paragraph { delete_range_in_paragraph(&mut paragraphs[start.paragraph], start.offset, end.offset); } else { let start_para_len = paragraph_char_count(¶graphs[start.paragraph]); @@ -473,19 +475,15 @@ pub fn delete_selection_range(text_content: &mut TextContent, selection: &TextSe start_para_len, ); - delete_range_in_paragraph(&mut paragraphs[end.paragraph], 0, end.offset); + delete_range_in_paragraph(&mut paragraphs[end_paragraph], 0, end.offset); - if end.paragraph < paragraphs.len() { - let end_para_children: Vec<_> = - paragraphs[end.paragraph].children_mut().drain(..).collect(); - paragraphs[start.paragraph] - .children_mut() - .extend(end_para_children); - } + let end_para_children: Vec<_> = + paragraphs[end_paragraph].children_mut().drain(..).collect(); + paragraphs[start.paragraph] + .children_mut() + .extend(end_para_children); - if end.paragraph < paragraphs.len() { - paragraphs.drain((start.paragraph + 1)..=end.paragraph); - } + paragraphs.drain((start.paragraph + 1)..=end_paragraph); let children = paragraphs[start.paragraph].children_mut(); let has_content = children.iter().any(|span| !span.text.is_empty()); @@ -499,6 +497,11 @@ pub fn delete_selection_range(text_content: &mut TextContent, selection: &TextSe /// Delete a range of characters within a single paragraph. pub fn delete_range_in_paragraph(para: &mut Paragraph, start_offset: usize, end_offset: usize) { + // An out of bounds offset must not skip the deletion. + let para_len = paragraph_char_count(para); + let start_offset = start_offset.min(para_len); + let end_offset = end_offset.min(para_len); + if start_offset >= end_offset { return; } @@ -848,3 +851,124 @@ pub fn split_paragraph_at_cursor( true } + +#[cfg(test)] +mod tests { + use super::*; + use crate::math::Rect; + use crate::shapes::{FontFamily, FontStyle, GrowType, TextAlign, TextSpan}; + use crate::uuid::Uuid; + + fn span(text: &str) -> TextSpan { + TextSpan::new( + text.to_string(), + FontFamily::new(Uuid::nil(), 400, FontStyle::Normal), + 14.0, + 1.2, + 0.0, + None, + None, + TextDirection::LTR, + 400, + Uuid::nil(), + vec![], + ) + } + + fn paragraph(texts: &[&str]) -> Paragraph { + Paragraph::new( + TextAlign::Left, + TextDirection::LTR, + None, + None, + 1.2, + 0.0, + texts.iter().copied().map(span).collect(), + ) + } + + fn content(paragraphs: Vec) -> TextContent { + let mut content = + TextContent::new(Rect::from_xywh(0.0, 0.0, 100.0, 100.0), GrowType::Fixed); + for para in paragraphs { + content.add_paragraph(para); + } + content + } + + fn text_of(content: &TextContent) -> String { + content + .paragraphs() + .iter() + .map(|para| { + para.children() + .iter() + .map(|span| span.text.as_str()) + .collect::() + }) + .collect::>() + .join("\n") + } + + fn selection(start: (usize, usize), end: (usize, usize)) -> TextSelection { + TextSelection { + anchor: TextPositionWithAffinity::new_without_affinity(start.0, start.1), + focus: TextPositionWithAffinity::new_without_affinity(end.0, end.1), + } + } + + #[test] + fn delete_selection_range_deletes_a_whole_multibyte_paragraph() { + let mut content = content(vec![paragraph(&["Añadir"])]); + delete_selection_range(&mut content, &selection((0, 0), (0, 6))); + assert_eq!(text_of(&content), ""); + } + + #[test] + fn delete_range_in_paragraph_clamps_an_overshooting_end_offset() { + let mut para = paragraph(&["Añadir"]); + delete_range_in_paragraph(&mut para, 0, 7); + assert_eq!(para.children().len(), 1); + assert_eq!(para.children()[0].text, ""); + } + + #[test] + fn delete_selection_range_deletes_emoji() { + let mut content = content(vec![paragraph(&["Hi 😀"])]); + delete_selection_range(&mut content, &selection((0, 0), (0, 4))); + assert_eq!(text_of(&content), ""); + } + + #[test] + fn delete_selection_range_never_splits_an_emoji() { + let mut content = content(vec![paragraph(&["a😀b"])]); + delete_selection_range(&mut content, &selection((0, 1), (0, 2))); + assert_eq!(text_of(&content), "ab"); + } + + #[test] + fn delete_selection_range_spanning_several_spans() { + let mut content = content(vec![paragraph(&["Añ", "adir"])]); + delete_selection_range(&mut content, &selection((0, 1), (0, 4))); + assert_eq!(text_of(&content), "Air"); + } + + #[test] + fn delete_selection_range_across_paragraphs() { + let mut content = content(vec![ + paragraph(&["Añadir"]), + paragraph(&["Más"]), + paragraph(&["Fin"]), + ]); + delete_selection_range(&mut content, &selection((0, 0), (2, 3))); + assert_eq!(content.paragraphs().len(), 1); + assert_eq!(text_of(&content), ""); + } + + #[test] + fn delete_selection_range_clamps_an_out_of_range_focus() { + let mut content = content(vec![paragraph(&["Añadir"])]); + delete_selection_range(&mut content, &selection((0, 0), (5, 99))); + assert_eq!(text_of(&content), ""); + } +} diff --git a/render-wasm/src/wasm/text_editor.rs b/render-wasm/src/wasm/text_editor.rs index ca54edc39a..af37164af9 100644 --- a/render-wasm/src/wasm/text_editor.rs +++ b/render-wasm/src/wasm/text_editor.rs @@ -1060,17 +1060,40 @@ pub extern "C" fn text_editor_export_selection() -> *mut u8 { #[no_mangle] pub extern "C" fn text_editor_get_selection(buffer_ptr: *mut u32) -> bool { - if !get_text_editor_state().selection.is_selection() { - return false; - } - let sel = &get_text_editor_state().selection; - unsafe { - *buffer_ptr = sel.anchor.paragraph as u32; - *buffer_ptr.add(1) = sel.anchor.offset as u32; - *buffer_ptr.add(2) = sel.focus.paragraph as u32; - *buffer_ptr.add(3) = sel.focus.offset as u32; - } - true + with_state!(state, { + if !get_text_editor_state().selection.is_selection() { + return false; + } + + let sel = get_text_editor_state().selection; + + // The frontend indexes these offsets into JS strings, which are UTF-16. + let (anchor_offset, focus_offset) = match get_text_editor_state() + .active_shape_id + .and_then(|shape_id| state.shapes.get(&shape_id)) + .map(|shape| &shape.shape_type) + { + Some(Type::Text(text_content)) => { + let paragraphs = text_content.paragraphs(); + let to_utf16 = |position: TextPositionWithAffinity| { + paragraphs + .get(position.paragraph) + .map(|para| para.char_offset_to_utf16(position.offset)) + .unwrap_or(position.offset) + }; + (to_utf16(sel.anchor), to_utf16(sel.focus)) + } + _ => (sel.anchor.offset, sel.focus.offset), + }; + + unsafe { + *buffer_ptr = sel.anchor.paragraph as u32; + *buffer_ptr.add(1) = anchor_offset as u32; + *buffer_ptr.add(2) = sel.focus.paragraph as u32; + *buffer_ptr.add(3) = focus_offset as u32; + } + true + }) } // ============================================================================ @@ -1099,11 +1122,11 @@ fn get_cursor_rect( let mut y_offset = valign_offset; for (idx, laid_out_para) in layout_paragraphs.iter().enumerate() { if idx == cursor.paragraph { - let char_pos = cursor.offset; + let utf16_pos = paragraphs[cursor.paragraph].char_offset_to_utf16(cursor.offset); use skia_safe::textlayout::{RectHeightStyle, RectWidthStyle}; let rects = laid_out_para.get_rects_for_range( - char_pos..char_pos, + utf16_pos..utf16_pos, RectHeightStyle::Tight, RectWidthStyle::Tight, ); @@ -1187,7 +1210,7 @@ fn get_selection_rects( if range_start < range_end { use skia_safe::textlayout::{RectHeightStyle, RectWidthStyle}; let text_boxes = laid_out_para.get_rects_for_range( - range_start..range_end, + para.char_offset_to_utf16(range_start)..para.char_offset_to_utf16(range_end), RectHeightStyle::Tight, RectWidthStyle::Tight, );