Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMD+Shift is not reported correctly on macOS #3078

Closed
fredizzimo opened this issue Sep 2, 2023 · 8 comments · Fixed by #3361
Closed

CMD+Shift is not reported correctly on macOS #3078

fredizzimo opened this issue Sep 2, 2023 · 8 comments · Fixed by #3361
Labels
B - bug Dang, that shouldn't have happened DS - macos

Comments

@fredizzimo
Copy link

The corresponding bug in Neovide is neovide/neovide#1237

I have been wanting report this bug for quite some time now, since it has caused a pretty bad regression in Neovide, and has a big impact for some users. But unfortunately, I don't own a Mac, so it has been difficult to gather the information I need to report it.

The issue is that CMD + character reports the same text as CMD + Shift + character, the text always contains the base character. This does not play well with Vim, which assumes that you map the resulting characters, not a key + modifiers. We do know from the modifiers that shift is pressed, but that information can't really be used without knowing the keymap, it's possible to turn standard alpha keys to uppercase, but in combination with the number keys for example, we don't have any idea what character to send to Neovim.

Here's a trace captured through the Neovide logging, where CMD+shift+d was pressed.

TRACE [neovide::window::keyboard_manager] KeyEvent {
    physical_key: SuperLeft,
    logical_key: Super,
    text: None,
    location: Left,
    state: Pressed,
    repeat: false,
    platform_specific: KeyEventExtra {
        text_with_all_modifiers: None,
        key_without_modifiers: Super,
    },
}
TRACE [neovide::redraw_scheduler] Next frame queued
TRACE [neovide::window::keyboard_manager] KeyEvent {
    physical_key: ShiftLeft,
    logical_key: Shift,
    text: None,
    location: Left,
    state: Pressed,
    repeat: false,
    platform_specific: KeyEventExtra {
        text_with_all_modifiers: None,
        key_without_modifiers: Shift,
    },
}
TRACE [neovide::window::keyboard_manager] Modifiers { state: ModifiersState(SHIFT | SUPER), pressed_mods: ModifiersKeys(LSHIFT | LSUPER) }
TRACE [neovide::window::keyboard_manager] KeyEvent {
    physical_key: KeyD,
    logical_key: Character(
        "d",
    ),
    text: Some(
        "d",
    ),
    location: Standard,
    state: Pressed,
    repeat: false,
    platform_specific: KeyEventExtra {
        text_with_all_modifiers: Some(
            "d",
        ),
        key_without_modifiers: Character(
            "d",
        ),
    },
}

As, you can see, there's absolutely no way of translating this to <D-D>, which Neovim wants, instead of <D-d>, other than manually detect that shift is pressed and try to convert it to uppercase, and hope that's the correct character. But as mentioned, it only deals with the alphas, and without other modifier combinations.

All the other platforms report this correctly, well, they don't have CMD, but alt+shift+character, for example works correctly (except for some combinations #3012). I have no idea if alt and other combinations work correctly on macOS or not, since I have not been able to gather that information and that's why I have been waiting to create this issue before now.

Note that we are using 0.29.0-beta0, and since I can't test this myself, I haven't done any testing on beta1, but I don't see anything in the changelogs that would indicate it's fixed.

@fredizzimo
Copy link
Author

Note that the alternative to text, logical_key is also set to the lowercase d

@kchibisov
Copy link
Member

Yeah, it looks like a bug, but given that we take the information from the macOS it could be doing this sort of reporting intentionally? There could be an issue with the way we build up the key, but most of the time we just query macOS for that stuff.

@fredizzimo
Copy link
Author

It was reported that MacVim does it correctly neovide/neovide#1237 (comment).

But they could just be converting it to uppercase manually. So, that’s probably how we would end up doing it, if this is not fixable on the winit side.

@kchibisov
Copy link
Member

Hm, yeah. Someone should try and see what macOS reports here.

@mgax
Copy link

mgax commented Sep 5, 2023

I've added logging to event.rs:

diff --git a/src/platform_impl/macos/event.rs b/src/platform_impl/macos/event.rs
index 9f13787b..505093ba 100644
--- a/src/platform_impl/macos/event.rs
+++ b/src/platform_impl/macos/event.rs
@@ -114,6 +114,10 @@ pub(crate) fn create_key_event(
     is_repeat: bool,
     key_override: Option<KeyCode>,
 ) -> KeyEvent {
+    if is_press {
+        trace!("ns_event: {ns_event:?}");
+    }
+
     use ElementState::{Pressed, Released};
     let state = if is_press { Pressed } else { Released };

Then tried the following keystrokes while running cargo run --example=window | grep 'winit::platform_impl::platform::event':

  • d: TRACE [winit::platform_impl::platform::event] ns_event: NSEvent { __superclass: NSEvent: type=KeyDown loc=(-529.309,-3.04297) time=9747.3 flags=0x100 win=0x130e4c040 winNum=1124 ctxt=0x0 chars="c" unmodchars="c" repeat=0 keyCode=8 }
  • shift-d: TRACE [winit::platform_impl::platform::event] ns_event: NSEvent { __superclass: NSEvent: type=KeyDown loc=(-516.059,6.25391) time=9752.1 flags=0x20102 win=0x130e4c040 winNum=1124 ctxt=0x0 chars="D" unmodchars="D" repeat=0 keyCode=2 }
  • command-d: TRACE [winit::platform_impl::platform::event] ns_event: NSEvent { __superclass: NSEvent: type=KeyDown loc=(-516.059,6.25391) time=9753.8 flags=0x100108 win=0x130e4c040 winNum=1124 ctxt=0x0 chars="d" unmodchars="d" repeat=0 keyCode=2 }
  • command-shift-d: TRACE [winit::platform_impl::platform::event] ns_event: NSEvent { __superclass: NSEvent: type=KeyDown loc=(-516.059,6.25391) time=9754.6 flags=0x12010a win=0x130e4c040 winNum=1124 ctxt=0x0 chars="d" unmodchars="D" repeat=0 keyCode=2 }

It looks like unmodchars is the value returned by ns_event.charactersIgnoringModifiers(). key_without_modifiers however always seems to be lowercase.

@fredizzimo
Copy link
Author

I guess the solution here would be to use unmodchar for logical_key, from the documentation

The string of characters generated by the key event as if no modifier key had been pressed (except for Shift). This argument is useful for getting the “basic” key value in a hardware-independent manner.

We still need some special handling in Neovide for this, to use logical_key instead of text in this specific case, but I think that's the best that can be done. text will still be needed in order to deal with dead keys.

@kchibisov
Copy link
Member

It sounds like what logical_key should be, indeed.

@emilk
Copy link
Contributor

emilk commented Jan 5, 2024

I also ran into this problem, trying to write a shortcut for zooming in with ctrl + (the plus key). To hit the plus key on an English keyboard one has to type shift =. Hitting shift =, we get a logical key of + (as expected). Hitting ctrl+shift = changes the logical key to =.

Fix coming in #3361

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B - bug Dang, that shouldn't have happened DS - macos
Development

Successfully merging a pull request may close this issue.

5 participants