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