🐛 Fix text replacement on selection and text offsets (#10983)

* 🐛 Fix text not being replaced when there is a selection

* 🐛 Fix text editor offsets on transformed text

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,
 }) => {

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() }

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,
             );

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);
+    }
 }

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,

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(&paragraphs[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<Paragraph>) -> 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::<String>()
+            })
+            .collect::<Vec<_>>()
+            .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), "");
+    }
+}

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,
             );