Skip key down event if preceded by its key equivalent version (#2692)

Fixes
https://linear.app/zed-industries/issue/Z-2552/pressing-two-keystrokes-in-rapid-succession-ignores-the-latter

Previously, we would only track whether the previous key down event was
a key equivalent. However, this could cause issues when pressing certain
keystrokes in rapid succession, e.g.:

- Pressing `shift-right` (to select a character, dispatched as a key
equivalent)
- Pressing a character (with or without `shift` held down, dispatched as
a key down)

This would cause GPUI to ignore the second event because it was preceded
by a key equivalent event. With this commit, we track the last key
equivalent event, and skip the key down event only if it matches the
last key equivalent event.

Release Notes:

- Fixed a bug that could cause certain keystrokes performed in rapid
succession to incorrectly get ignored.

crates/gpui/src/platform/event.rs

@@ -4,7 +4,7 @@ use pathfinder_geometry::vector::vec2f;
 
 use crate::{geometry::vector::Vector2F, keymap_matcher::Keystroke};
 
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, Eq, PartialEq)]
 pub struct KeyDownEvent {
     pub keystroke: Keystroke,
     pub is_held: bool,

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

@@ -232,10 +232,6 @@ unsafe fn build_window_class(name: &'static str, superclass: &Class) -> *const C
         sel!(canBecomeKeyWindow),
         yes as extern "C" fn(&Object, Sel) -> BOOL,
     );
-    decl.add_method(
-        sel!(sendEvent:),
-        send_event as extern "C" fn(&Object, Sel, id),
-    );
     decl.add_method(
         sel!(windowDidResize:),
         window_did_resize as extern "C" fn(&Object, Sel, id),
@@ -299,7 +295,7 @@ struct WindowState {
     appearance_changed_callback: Option<Box<dyn FnMut()>>,
     input_handler: Option<Box<dyn InputHandler>>,
     pending_key_down: Option<(KeyDownEvent, Option<InsertText>)>,
-    performed_key_equivalent: bool,
+    last_key_equivalent: Option<KeyDownEvent>,
     synthetic_drag_counter: usize,
     executor: Rc<executor::Foreground>,
     scene_to_render: Option<Scene>,
@@ -521,7 +517,7 @@ impl Window {
                 appearance_changed_callback: None,
                 input_handler: None,
                 pending_key_down: None,
-                performed_key_equivalent: false,
+                last_key_equivalent: None,
                 synthetic_drag_counter: 0,
                 executor,
                 scene_to_render: Default::default(),
@@ -965,36 +961,34 @@ extern "C" fn handle_key_event(this: &Object, native_event: id, key_equivalent:
     let window_height = window_state_borrow.content_size().y();
     let event = unsafe { Event::from_native(native_event, Some(window_height)) };
 
-    if let Some(event) = event {
+    if let Some(Event::KeyDown(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
+        // the "key down" event if we've already just processed its "key equivalent" version.
         if key_equivalent {
-            window_state_borrow.performed_key_equivalent = true;
+            window_state_borrow.last_key_equivalent = Some(event.clone());
-        } else if window_state_borrow.performed_key_equivalent {
+        } else if window_state_borrow.last_key_equivalent.take().as_ref() == Some(&event) {
             return NO;
         }
 
-        let function_is_held;
+        let keydown = event.keystroke.clone();
-        window_state_borrow.pending_key_down = match event {
+        let fn_modifier = keydown.function;
-            Event::KeyDown(event) => {
+        // Ignore events from held-down keys after some of the initially-pressed keys
-                let keydown = event.keystroke.clone();
+        // were released.
-                // Ignore events from held-down keys after some of the initially-pressed keys
+        if event.is_held {
-                // were released.
+            if window_state_borrow.last_fresh_keydown.as_ref() != Some(&keydown) {
-                if event.is_held {
+                return YES;
-                    if window_state_borrow.last_fresh_keydown.as_ref() != Some(&keydown) {
-                        return YES;
-                    }
-                } else {
-                    window_state_borrow.last_fresh_keydown = Some(keydown);
-                }
-                function_is_held = event.keystroke.function;
-                Some((event, None))
             }
-
+        } else {
-            _ => return NO,
+            window_state_borrow.last_fresh_keydown = Some(keydown);
-        };
+        }
-
+        window_state_borrow.pending_key_down = Some((event, None));
         drop(window_state_borrow);
 
-        if !function_is_held {
+        // Send the event to the input context for IME handling, unless the `fn` modifier is
+        // being pressed.
+        if !fn_modifier {
             unsafe {
                 let input_context: id = msg_send![this, inputContext];
                 let _: BOOL = msg_send![input_context, handleEvent: native_event];
@@ -1143,13 +1137,6 @@ extern "C" fn cancel_operation(this: &Object, _sel: Sel, _sender: id) {
     }
 }
 
-extern "C" fn send_event(this: &Object, _: Sel, native_event: id) {
-    unsafe {
-        let _: () = msg_send![super(this, class!(NSWindow)), sendEvent: native_event];
-        get_window_state(this).borrow_mut().performed_key_equivalent = false;
-    }
-}
-
 extern "C" fn window_did_resize(this: &Object, _: Sel, _: id) {
     let window_state = unsafe { get_window_state(this) };
     window_state.as_ref().borrow().move_traffic_light();