From ed42b806b148c54b52977533194779b466b298b5 Mon Sep 17 00:00:00 2001 From: Sagnik Mandal Date: Thu, 19 Mar 2026 22:32:27 +0530 Subject: [PATCH] editor: Fix Accessibility Keyboard word completion corrupting text (#50676) When typing with the macOS Accessibility Keyboard and clicking a word completion suggestion, text was corrupted in several ways. macOS sends `insertText:replacementRange:` when a completion is selected. The replacement range was passed to `selection_replacement_ranges` using the wrong offset type, causing incorrect delta calculation for multi-cursor scenarios. Additionally, `backspace` was called unconditionally even when the replacement range was empty (as with the trailing space the Accessibility Keyboard appends), deleting the last character of every completed word. Finally, an empty replacement range in a multi-cursor context carries a stale cursor position from macOS's single-cursor view of the buffer, so it is now ignored and text is inserted at each cursor's actual position instead. Closes #38052 Release Notes: - Fixed text corruption when using macOS Accessibility Keyboard word completion. --- crates/editor/src/editor.rs | 35 +++++++++++++++-- crates/editor/src/editor_tests.rs | 65 +++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 4 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 0387fc1e39d25e26f0cd6c1917bbe4545d1bd201..bfd82dac0ceeb38ee4d4eb74fa2270c3d885839f 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -28268,9 +28268,29 @@ impl EntityInputHandler for Editor { self.transact(window, cx, |this, window, cx| { let new_selected_ranges = if let Some(range_utf16) = range_utf16 { - let range_utf16 = MultiBufferOffsetUtf16(OffsetUtf16(range_utf16.start)) - ..MultiBufferOffsetUtf16(OffsetUtf16(range_utf16.end)); - Some(this.selection_replacement_ranges(range_utf16, cx)) + if let Some(marked_ranges) = this.marked_text_ranges(cx) { + // During IME composition, macOS reports the replacement range + // relative to the first marked region (the only one visible via + // marked_text_range). The correct targets for replacement are the + // marked ranges themselves — one per cursor — so use them directly. + Some(marked_ranges) + } else if range_utf16.start == range_utf16.end { + // An empty replacement range means "insert at cursor" with no text + // to replace. macOS reports the cursor position from its own + // (single-cursor) view of the buffer, which diverges from our actual + // cursor positions after multi-cursor edits have shifted offsets. + // Treating this as range_utf16=None lets each cursor insert in place. + None + } else { + // Outside of IME composition (e.g. Accessibility Keyboard word + // completion), the range is an absolute document offset for the + // newest cursor. Fan it out to all cursors via + // selection_replacement_ranges, which applies the delta relative + // to the newest selection to every cursor. + let range_utf16 = MultiBufferOffsetUtf16(OffsetUtf16(range_utf16.start)) + ..MultiBufferOffsetUtf16(OffsetUtf16(range_utf16.end)); + Some(this.selection_replacement_ranges(range_utf16, cx)) + } } else { this.marked_text_ranges(cx) }; @@ -28299,10 +28319,17 @@ impl EntityInputHandler for Editor { }); if let Some(new_selected_ranges) = new_selected_ranges { + // Only backspace if at least one range covers actual text. When all + // ranges are empty (e.g. a trailing-space insertion from Accessibility + // Keyboard sends replacementRange=cursor..cursor), backspace would + // incorrectly delete the character just before the cursor. + let should_backspace = new_selected_ranges.iter().any(|r| r.start != r.end); this.change_selections(SelectionEffects::no_scroll(), window, cx, |selections| { selections.select_ranges(new_selected_ranges) }); - this.backspace(&Default::default(), window, cx); + if should_backspace { + this.backspace(&Default::default(), window, cx); + } } this.handle_input(text, window, cx); diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 7d15fb2e2d0746f8f47fd400ed0b18602caa3429..d6de2bc194e3cff7af7c1fc77acc8e6701511457 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -320,6 +320,71 @@ fn test_undo_redo_with_selection_restoration(cx: &mut TestAppContext) { }); } +#[gpui::test] +fn test_accessibility_keyboard_word_completion(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + // Simulates the macOS Accessibility Keyboard word completion panel, which calls + // insertText:replacementRange: to commit a completion. macOS sends two calls per + // completion: one with a non-empty range replacing the typed prefix, and one with + // an empty replacement range (cursor..cursor) to append a trailing space. + + cx.add_window(|window, cx| { + let buffer = MultiBuffer::build_simple("ab", cx); + let mut editor = build_editor(buffer, window, cx); + + // Cursor is after the 2-char prefix "ab" at offset 2. + editor.change_selections(SelectionEffects::no_scroll(), window, cx, |s| { + s.select_ranges([MultiBufferOffset(2)..MultiBufferOffset(2)]) + }); + + // macOS completes "about" by replacing the prefix via range 0..2. + editor.replace_text_in_range(Some(0..2), "about", window, cx); + assert_eq!(editor.text(cx), "about"); + + // macOS sends a trailing space as an empty replacement range (cursor..cursor). + // Must insert at the cursor position, not call backspace first (which would + // delete the preceding character). + editor.replace_text_in_range(Some(5..5), " ", window, cx); + assert_eq!(editor.text(cx), "about "); + + editor + }); + + // Multi-cursor: the replacement must fan out to all cursors, and the trailing + // space must land at each cursor's actual current position. After the first + // completion, macOS's reported cursor offset is stale (it doesn't account for + // the offset shift caused by the other cursor's insertion), so the empty + // replacement range must be ignored and the space inserted at each real cursor. + cx.add_window(|window, cx| { + // Two cursors, each after a 2-char prefix "ab" at the end of each line: + // "ab\nab" — cursors at offsets 2 and 5. + let buffer = MultiBuffer::build_simple("ab\nab", cx); + let mut editor = build_editor(buffer, window, cx); + + editor.change_selections(SelectionEffects::no_scroll(), window, cx, |s| { + s.select_ranges([ + MultiBufferOffset(2)..MultiBufferOffset(2), + MultiBufferOffset(5)..MultiBufferOffset(5), + ]) + }); + + // macOS reports the newest cursor (offset 5) and sends range 3..5 to + // replace its 2-char prefix. selection_replacement_ranges applies the same + // delta to fan out to both cursors: 0..2 and 3..5. + editor.replace_text_in_range(Some(3..5), "about", window, cx); + assert_eq!(editor.text(cx), "about\nabout"); + + // Trailing space via empty range. macOS thinks the cursor is at offset 10 + // (5 - 2 + 7 = 10), but the actual cursors are at 5 and 11. The stale + // offset must be ignored and the space inserted at each real cursor position. + editor.replace_text_in_range(Some(10..10), " ", window, cx); + assert_eq!(editor.text(cx), "about \nabout "); + + editor + }); +} + #[gpui::test] fn test_ime_composition(cx: &mut TestAppContext) { init_test(cx, |_| {});