macOS: Fix crash when pressing Caps Lock (#4024)

Events emitted by `flagsChanged:` cannot access
`charactersIgnoringModifiers`. We were previously doing this because we
were trying to re-use the `create_key_event` function, but that is unsuited
for this purpose, so I have separated the `flagsChanged:` logic out from it.

src/changelog/unreleased.md

@@ -212,3 +212,4 @@ changelog entry.
 - On macOS, fixed the scancode conversion for audio volume keys.
 - On macOS, fixed the scancode conversion for `IntlBackslash`.
 - On macOS, fixed redundant `SurfaceResized` event at window creation.
+- On macOS, fix crash when pressing Caps Lock in certain configurations.

src/platform_impl/apple/appkit/event.rs

@@ -92,17 +92,12 @@ fn get_logical_key_char(ns_event: &NSEvent, modifierless_chars: &str) -> Key {
 /// Create `KeyEvent` for the given `NSEvent`.
 ///
 /// This function shouldn't be called when the IME input is in process.
-pub(crate) fn create_key_event(
+pub(crate) fn create_key_event(ns_event: &NSEvent, is_press: bool, is_repeat: bool) -> KeyEvent {
-    ns_event: &NSEvent,
-    is_press: bool,
-    is_repeat: bool,
-    key_override: Option<PhysicalKey>,
-) -> KeyEvent {
     use ElementState::{Pressed, Released};
     let state = if is_press { Pressed } else { Released };
 
     let scancode = unsafe { ns_event.keyCode() };
-    let mut physical_key = key_override.unwrap_or_else(|| scancode_to_physicalkey(scancode as u32));
+    let mut physical_key = scancode_to_physicalkey(scancode as u32);
 
     // NOTE: The logical key should heed both SHIFT and ALT if possible.
     // For instance:
@@ -111,20 +106,15 @@ pub(crate) fn create_key_event(
     // * Pressing CTRL SHIFT A: logical key should also be "A"
     // This is not easy to tease out of `NSEvent`, but we do our best.
 
-    let text_with_all_modifiers: Option<SmolStr> = if key_override.is_some() {
+    let characters = unsafe { ns_event.characters() }.map(|s| s.to_string()).unwrap_or_default();
+    let text_with_all_modifiers = if characters.is_empty() {
         None
     } else {
-        let characters =
+        if matches!(physical_key, PhysicalKey::Unidentified(_)) {
-            unsafe { ns_event.characters() }.map(|s| s.to_string()).unwrap_or_default();
+            // The key may be one of the funky function keys
-        if characters.is_empty() {
+            physical_key = extra_function_key_to_code(scancode, &characters);
-            None
-        } else {
-            if matches!(physical_key, PhysicalKey::Unidentified(_)) {
-                // The key may be one of the funky function keys
-                physical_key = extra_function_key_to_code(scancode, &characters);
-            }
-            Some(SmolStr::new(characters))
         }
+        Some(SmolStr::new(characters))
     };
 
     let key_from_code = code_to_key(physical_key, scancode);

src/platform_impl/apple/appkit/view.rs

@@ -21,13 +21,13 @@ use super::app_state::AppState;
 use super::cursor::{default_cursor, invisible_cursor};
 use super::event::{
     code_to_key, code_to_location, create_key_event, event_mods, lalt_pressed, ralt_pressed,
-    scancode_to_physicalkey,
+    scancode_to_physicalkey, KeyEventExtra,
 };
 use super::window::WinitWindow;
 use crate::dpi::{LogicalPosition, LogicalSize};
 use crate::event::{
-    DeviceEvent, ElementState, Ime, Modifiers, MouseButton, MouseScrollDelta, PointerKind,
+    DeviceEvent, ElementState, Ime, KeyEvent, Modifiers, MouseButton, MouseScrollDelta,
-    PointerSource, TouchPhase, WindowEvent,
+    PointerKind, PointerSource, TouchPhase, WindowEvent,
 };
 use crate::keyboard::{Key, KeyCode, KeyLocation, ModifiersState, NamedKey};
 use crate::platform::macos::OptionAsAlt;
@@ -490,7 +490,7 @@ declare_class!(
             };
 
             if !had_ime_input || self.ivars().forward_key_to_app.get() {
-                let key_event = create_key_event(&event, true, unsafe { event.isARepeat() }, None);
+                let key_event = create_key_event(&event, true, unsafe { event.isARepeat() });
                 self.queue_event(WindowEvent::KeyboardInput {
                     device_id: None,
                     event: key_event,
@@ -513,7 +513,7 @@ declare_class!(
             ) {
                 self.queue_event(WindowEvent::KeyboardInput {
                     device_id: None,
-                    event: create_key_event(&event, false, false, None),
+                    event: create_key_event(&event, false, false),
                     is_synthetic: false,
                 });
             }
@@ -560,7 +560,7 @@ declare_class!(
                 .expect("could not find current event");
 
             self.update_modifiers(&event, false);
-            let event = create_key_event(&event, true, unsafe { event.isARepeat() }, None);
+            let event = create_key_event(&event, true, unsafe { event.isARepeat() });
 
             self.queue_event(WindowEvent::KeyboardInput {
                 device_id: None,
@@ -946,22 +946,36 @@ impl WinitView {
                 let scancode = unsafe { ns_event.keyCode() };
                 let physical_key = scancode_to_physicalkey(scancode as u32);
 
-                // We'll correct the `is_press` later.
+                let logical_key = code_to_key(physical_key, scancode);
-                let mut event = create_key_event(ns_event, false, false, Some(physical_key));
-
-                let key = code_to_key(physical_key, scancode);
                 // Ignore processing of unknown modifiers because we can't determine whether
                 // it was pressed or release reliably.
-                let Some(event_modifier) = key_to_modifier(&key) else {
+                //
+                // Furthermore, sometimes normal keys are reported inside flagsChanged:, such as
+                // when holding Caps Lock while pressing another key, see:
+                // https://github.com/alacritty/alacritty/issues/8268
+                let Some(event_modifier) = key_to_modifier(&logical_key) else {
                     break 'send_event;
                 };
-                event.physical_key = physical_key;
+
-                event.logical_key = key.clone();
+                let mut event = KeyEvent {
-                event.location = code_to_location(physical_key);
+                    location: code_to_location(physical_key),
+                    logical_key: logical_key.clone(),
+                    physical_key,
+                    repeat: false,
+                    // We'll correct this later.
+                    state: Pressed,
+                    text: None,
+                    platform_specific: KeyEventExtra {
+                        text_with_all_modifiers: None,
+                        key_without_modifiers: logical_key.clone(),
+                    },
+                };
+
                 let location_mask = ModLocationMask::from_location(event.location);
 
                 let mut phys_mod_state = self.ivars().phys_modifiers.borrow_mut();
-                let phys_mod = phys_mod_state.entry(key).or_insert(ModLocationMask::empty());
+                let phys_mod =
+                    phys_mod_state.entry(logical_key).or_insert(ModLocationMask::empty());
 
                 let is_active = current_modifiers.state().contains(event_modifier);
                 let mut events = VecDeque::with_capacity(2);