Properly handle backspace when in dead key state (#7494)

Previously we wouldn't handle Backspace in dead key state correctly:
instead of removing what was typed, we'd insert the text that was in the
dead key state.

Example: on a US English layout, press `opt-u` to end up in a dead key
state with `¨` waiting for the next character to be typed. Type
`backspace`. The `¨` should be removed, but it's not.

With this change, the `backspace` is interpreted instead of being
ignored.

Release Notes:

- Fixed backspace not working for dead keys (i.e. when typing accents or
umlauts)

---------

Co-authored-by: Antonio <antonio@zed.dev>
Co-authored-by: Conrad <conrad@zed.dev>

crates/gpui/src/platform/mac/window.rs

@@ -309,21 +309,11 @@ unsafe fn build_window_class(name: &'static str, superclass: &Class) -> *const C
     decl.register()
 }
 
-///Used to track what the IME does when we send it a keystroke.
+#[allow(clippy::enum_variant_names)]
-///This is only used to handle the case where the IME mysteriously
+enum ImeInput {
-///swallows certain keys.
+    InsertText(String, Option<Range<usize>>),
-///
+    SetMarkedText(String, Option<Range<usize>>, Option<Range<usize>>),
-///Basically a direct copy of the approach that WezTerm uses in:
+    UnmarkText,
-///github.com/wez/wezterm : d5755f3e : window/src/os/macos/window.rs
-enum ImeState {
-    Continue,
-    Acted,
-    None,
-}
-
-struct InsertText {
-    replacement_range: Option<Range<usize>>,
-    text: String,
 }
 
 struct MacWindowState {
@@ -344,16 +334,14 @@ struct MacWindowState {
     close_callback: Option<Box<dyn FnOnce()>>,
     appearance_changed_callback: Option<Box<dyn FnMut()>>,
     input_handler: Option<PlatformInputHandler>,
-    pending_key_down: Option<(KeyDownEvent, Option<InsertText>)>,
     last_key_equivalent: Option<KeyDownEvent>,
     synthetic_drag_counter: usize,
     last_fresh_keydown: Option<Keystroke>,
     traffic_light_position: Option<Point<Pixels>>,
     previous_modifiers_changed_event: Option<PlatformInput>,
     // State tracking what the IME did after the last request
-    ime_state: ImeState,
+    input_during_keydown: Option<SmallVec<[ImeInput; 1]>>,
-    // Retains the last IME Text
+    previous_keydown_inserted_text: Option<String>,
-    ime_text: Option<String>,
     external_files_dragged: bool,
 }
 
@@ -565,7 +553,6 @@ impl MacWindow {
                 close_callback: None,
                 appearance_changed_callback: None,
                 input_handler: None,
-                pending_key_down: None,
                 last_key_equivalent: None,
                 synthetic_drag_counter: 0,
                 last_fresh_keydown: None,
@@ -574,8 +561,8 @@ impl MacWindow {
                     .as_ref()
                     .and_then(|titlebar| titlebar.traffic_light_position),
                 previous_modifiers_changed_event: None,
-                ime_state: ImeState::None,
+                input_during_keydown: None,
-                ime_text: None,
+                previous_keydown_inserted_text: None,
                 external_files_dragged: false,
             })));
 
@@ -1116,6 +1103,15 @@ extern "C" fn handle_key_down(this: &Object, _: Sel, native_event: id) {
     handle_key_event(this, native_event, false);
 }
 
+// Things to test if you're modifying this method:
+//  Brazilian layout:
+//   - `" space` should type a quote
+//   - `" backspace` should delete the marked quote
+//   - `" up` should type the quote, unmark it, and move up one line
+//   - `" cmd-down` should not leave a marked quote behind (it maybe should dispatch the key though?)
+//   - `cmd-ctrl-space` and clicking on an emoji should type it
+//  Czech (QWERTY) layout:
+//   - in vim mode `option-4`  should go to end of line (same as $)
 extern "C" fn handle_key_event(this: &Object, native_event: id, key_equivalent: bool) -> BOOL {
     let window_state = unsafe { get_window_state(this) };
     let mut lock = window_state.as_ref().lock();
@@ -1123,7 +1119,7 @@ extern "C" fn handle_key_event(this: &Object, native_event: id, key_equivalent:
     let window_height = lock.content_size().height;
     let event = unsafe { PlatformInput::from_native(native_event, Some(window_height)) };
 
-    if let Some(PlatformInput::KeyDown(event)) = event {
+    if let Some(PlatformInput::KeyDown(mut event)) = event {
         // For certain keystrokes, macOS will first dispatch a "key equivalent" event.
         // If that event isn't handled, it will then dispatch a "key down" event. GPUI
         // makes no distinction between these two types of events, so we need to ignore
@@ -1143,13 +1139,14 @@ extern "C" fn handle_key_event(this: &Object, native_event: id, key_equivalent:
                 return YES;
             }
         } else {
-            lock.last_fresh_keydown = Some(keydown);
+            lock.last_fresh_keydown = Some(keydown.clone());
         }
-        lock.pending_key_down = Some((event, None));
+        lock.input_during_keydown = Some(SmallVec::new());
         drop(lock);
 
         // Send the event to the input context for IME handling, unless the `fn` modifier is
         // being pressed.
+        // this will call back into `insert_text`, etc.
         if !fn_modifier {
             unsafe {
                 let input_context: id = msg_send![this, inputContext];
@@ -1159,48 +1156,63 @@ extern "C" fn handle_key_event(this: &Object, native_event: id, key_equivalent:
 
         let mut handled = false;
         let mut lock = window_state.lock();
-        let ime_text = lock.ime_text.clone();
+        let previous_keydown_inserted_text = lock.previous_keydown_inserted_text.take();
-        if let Some((event, insert_text)) = lock.pending_key_down.take() {
+        let mut input_during_keydown = lock.input_during_keydown.take().unwrap();
-            let is_held = event.is_held;
+        let mut callback = lock.event_callback.take();
-            if let Some(mut callback) = lock.event_callback.take() {
+        drop(lock);
-                drop(lock);
 
-                let is_composing =
+        let last_ime = input_during_keydown.pop();
-                    with_input_handler(this, |input_handler| input_handler.marked_text_range())
+        // on a brazilian keyboard typing `"` and then hitting `up` will cause two IME
-                        .flatten()
+        // events, one to unmark the quote, and one to send the up arrow.
-                        .is_some();
+        for ime in input_during_keydown {
+            send_to_input_handler(this, ime);
+        }
+
+        let is_composing =
+            with_input_handler(this, |input_handler| input_handler.marked_text_range())
+                .flatten()
+                .is_some();
+
+        if let Some(ime) = last_ime {
+            if let ImeInput::InsertText(text, _) = &ime {
                 if !is_composing {
-                    handled = callback(PlatformInput::KeyDown(event));
+                    window_state.lock().previous_keydown_inserted_text = Some(text.clone());
-                }
+                    if let Some(callback) = callback.as_mut() {
-
+                        event.keystroke.ime_key = Some(text.clone());
-                if !handled {
+                        handled = callback(PlatformInput::KeyDown(event));
-                    if let Some(insert) = insert_text {
-                        handled = true;
-                        with_input_handler(this, |input_handler| {
-                            input_handler
-                                .replace_text_in_range(insert.replacement_range, &insert.text)
-                        });
-                    } else if !is_composing && is_held {
-                        if let Some(last_insert_text) = ime_text {
-                            //MacOS IME is a bit funky, and even when you've told it there's nothing to
-                            //inter it will still swallow certain keys (e.g. 'f', 'j') and not others
-                            //(e.g. 'n'). This is a problem for certain kinds of views, like the terminal
-                            with_input_handler(this, |input_handler| {
-                                if input_handler.selected_text_range().is_none() {
-                                    handled = true;
-                                    input_handler.replace_text_in_range(None, &last_insert_text)
-                                }
-                            });
-                        }
                     }
                 }
-
-                window_state.lock().event_callback = Some(callback);
             }
-        } else {
+
-            handled = true;
+            if !handled {
+                handled = true;
+                send_to_input_handler(this, ime);
+            }
+        } else if !is_composing {
+            let is_held = event.is_held;
+
+            if let Some(callback) = callback.as_mut() {
+                handled = callback(PlatformInput::KeyDown(event));
+            }
+
+            if !handled && is_held {
+                if let Some(text) = previous_keydown_inserted_text {
+                    // MacOS IME is a bit funky, and even when you've told it there's nothing to
+                    // enter it will still swallow certain keys (e.g. 'f', 'j') and not others
+                    // (e.g. 'n'). This is a problem for certain kinds of views, like the terminal.
+                    with_input_handler(this, |input_handler| {
+                        if input_handler.selected_text_range().is_none() {
+                            handled = true;
+                            input_handler.replace_text_in_range(None, &text)
+                        }
+                    });
+                    window_state.lock().previous_keydown_inserted_text = Some(text);
+                }
+            }
         }
 
+        window_state.lock().event_callback = callback;
+
         handled as BOOL
     } else {
         NO
@@ -1615,11 +1627,6 @@ extern "C" fn first_rect_for_character_range(
 
 extern "C" fn insert_text(this: &Object, _: Sel, text: id, replacement_range: NSRange) {
     unsafe {
-        let window_state = get_window_state(this);
-        let mut lock = window_state.lock();
-        let pending_key_down = lock.pending_key_down.take();
-        drop(lock);
-
         let is_attributed_string: BOOL =
             msg_send![text, isKindOfClass: [class!(NSAttributedString)]];
         let text: id = if is_attributed_string == YES {
@@ -1631,28 +1638,10 @@ extern "C" fn insert_text(this: &Object, _: Sel, text: id, replacement_range: NS
             .to_str()
             .unwrap();
         let replacement_range = replacement_range.to_range();
-
+        send_to_input_handler(
-        window_state.lock().ime_text = Some(text.to_string());
+            this,
-        window_state.lock().ime_state = ImeState::Acted;
+            ImeInput::InsertText(text.to_string(), replacement_range),
-
+        );
-        let is_composing =
-            with_input_handler(this, |input_handler| input_handler.marked_text_range())
-                .flatten()
-                .is_some();
-
-        if is_composing || text.chars().count() > 1 || pending_key_down.is_none() {
-            with_input_handler(this, |input_handler| {
-                input_handler.replace_text_in_range(replacement_range, text)
-            });
-        } else {
-            let mut pending_key_down = pending_key_down.unwrap();
-            pending_key_down.1 = Some(InsertText {
-                replacement_range,
-                text: text.to_string(),
-            });
-            pending_key_down.0.keystroke.ime_key = Some(text.to_string());
-            window_state.lock().pending_key_down = Some(pending_key_down);
-        }
     }
 }
 
@@ -1664,9 +1653,6 @@ extern "C" fn set_marked_text(
     replacement_range: NSRange,
 ) {
     unsafe {
-        let window_state = get_window_state(this);
-        window_state.lock().pending_key_down.take();
-
         let is_attributed_string: BOOL =
             msg_send![text, isKindOfClass: [class!(NSAttributedString)]];
         let text: id = if is_attributed_string == YES {
@@ -1680,24 +1666,14 @@ extern "C" fn set_marked_text(
             .to_str()
             .unwrap();
 
-        window_state.lock().ime_state = ImeState::Acted;
+        send_to_input_handler(
-        window_state.lock().ime_text = Some(text.to_string());
+            this,
-
+            ImeInput::SetMarkedText(text.to_string(), replacement_range, selected_range),
-        with_input_handler(this, |input_handler| {
+        );
-            input_handler.replace_and_mark_text_in_range(replacement_range, text, selected_range);
-        });
     }
 }
-
 extern "C" fn unmark_text(this: &Object, _: Sel) {
-    unsafe {
+    send_to_input_handler(this, ImeInput::UnmarkText);
-        let state = get_window_state(this);
-        let mut borrow = state.lock();
-        borrow.ime_state = ImeState::Acted;
-        borrow.ime_text.take();
-    }
-
-    with_input_handler(this, |input_handler| input_handler.unmark_text());
 }
 
 extern "C" fn attributed_substring_for_proposed_range(
@@ -1723,14 +1699,7 @@ extern "C" fn attributed_substring_for_proposed_range(
     .unwrap_or(nil)
 }
 
-extern "C" fn do_command_by_selector(this: &Object, _: Sel, _: Sel) {
+extern "C" fn do_command_by_selector(_: &Object, _: Sel, _: Sel) {}
-    unsafe {
-        let state = get_window_state(this);
-        let mut borrow = state.lock();
-        borrow.ime_state = ImeState::Continue;
-        borrow.ime_text.take();
-    }
-}
 
 extern "C" fn view_did_change_effective_appearance(this: &Object, _: Sel) {
     unsafe {
@@ -1881,6 +1850,30 @@ where
     }
 }
 
+fn send_to_input_handler(window: &Object, ime: ImeInput) {
+    unsafe {
+        let window_state = get_window_state(window);
+        let mut lock = window_state.lock();
+        if let Some(ime_input) = lock.input_during_keydown.as_mut() {
+            ime_input.push(ime);
+            return;
+        }
+        if let Some(mut input_handler) = lock.input_handler.take() {
+            drop(lock);
+            match ime {
+                ImeInput::InsertText(text, range) => {
+                    input_handler.replace_text_in_range(range, &text)
+                }
+                ImeInput::SetMarkedText(text, range, marked_range) => {
+                    input_handler.replace_and_mark_text_in_range(range, &text, marked_range)
+                }
+                ImeInput::UnmarkText => input_handler.unmark_text(),
+            }
+            window_state.lock().input_handler = Some(input_handler);
+        }
+    }
+}
+
 unsafe fn display_id_for_screen(screen: id) -> CGDirectDisplayID {
     let device_description = NSScreen::deviceDescription(screen);
     let screen_number_key: id = NSString::alloc(nil).init_str("NSScreenNumber");