Modifier key events have their modifier bits set differently on Linux vs OSX

[neurocyte]

Edited by @kovidgoyal to add the poll:

We are discussing the best approach to take for fixing this issue, the two options are below with updated votes.

1) The modifier bits should reflect the modifier state taking into account the current key event. This is the platform behavior of macOS , Windows and Qt ( @neurocyte @j4james @dankamongmen @kovidgoyal @rockorager ) 2) The modifier bits should reflect the modifier state as it was before the current key event. This is the platform behavior of Linux and GTK on Linux (havent been able to test GTK on other platforms). (@kchibisov @mitchellh)

(1) requires no changes to the existing kitty keyboard protocol beyond specifying the behavior. (2) requires adding a new "fake key code" and sending these events to update the modifier state.

Original issue is below:

On linux the modifier bit is clear on press events and set on release events. OSX is the other way around.

For example:

Linux: LEFT_CONTROL PRESS CSI 57442 u

ctrl+LEFT_CONTROL RELEASE CSI 57442 ; 5 : 3 u

macOS: ctrl+LEFT_CONTROL PRESS CSI 57442 ; 5 u

LEFT_CONTROL RELEASE CSI 57442 ; 1 : 3 u

This comes from a discussion with ghostty dev on which behaviour should be considered correct.

See: https://github.com/mitchellh/ghostty/issues/1082

[mitchellh]

To be clear, these are the show_key outputs I see in Kitty on macOS and Linux above. I'd just like guidance over what I should be doing in this situation. 😄

I know you can't see the private repo issue linked above, so just being clear here.

[mitchellh]

In this screenshot I did the following:

1. Press left control
2. Press right control
3. Release right control
4. Release left control

That is in the order of the events showing... I'm not sure how the bottom right release events are there just ignore those since I just can't recall. But the odd thing to me is that release right control showed up as repeat right control. Is this expected? Do you want me to make a new issue?

[kovidgoyal]

Actually I am pretty sure this is vnc related, I will look into the macOS behavior later when I have physical access.

[mitchellh]

Yes, in my terminal I hook into flagsChanged and emit synthetic key events. Implementation looks like this if it helps. Ultimately, I'm just trying to match Kitty's behavior so whatever you feel is correct I'll sync up.

(The resetting of the ctrl bit in the Kitty encoding is done at a lower level, the logic below is all about just getting the right action since the left/right key codes are unique.)

override func flagsChanged(with event: NSEvent) {
    let mod: UInt32;
    switch (event.keyCode) {
    case 0x39: mod = GHOSTTY_MODS_CAPS.rawValue
    case 0x38, 0x3C: mod = GHOSTTY_MODS_SHIFT.rawValue
    case 0x3B, 0x3E: mod = GHOSTTY_MODS_CTRL.rawValue
    case 0x3A, 0x3D: mod = GHOSTTY_MODS_ALT.rawValue
    case 0x37, 0x36: mod = GHOSTTY_MODS_SUPER.rawValue
    default: return
    }

    // The keyAction function will do this AGAIN below which sucks to repeat
    // but this is super cheap and flagsChanged isn't that common.
    let mods = Ghostty.ghosttyMods(event.modifierFlags)

    // We assume the action is release, but if the bit is set in the modifier
    // flags we check further if it is actually pressed. Its not as simple as
    // assuming pressed, see the next comment.
    var action = GHOSTTY_ACTION_RELEASE
    if (mods.rawValue & mod != 0) {
        // If the key is pressed, its slightly more complicated, because we
        // want to check if the pressed modifier is the correct side. If the
        // correct side is pressed then its a press event otherwise its a release
        // event with the opposite modifier still held.
        let sidePressed: Bool
        switch (event.keyCode) {
        case 0x3C:
            sidePressed = event.modifierFlags.rawValue & UInt(NX_DEVICERSHIFTKEYMASK) != 0;
        case 0x3E:
            sidePressed = event.modifierFlags.rawValue & UInt(NX_DEVICERCTLKEYMASK) != 0;
        case 0x3D:
            sidePressed = event.modifierFlags.rawValue & UInt(NX_DEVICERALTKEYMASK) != 0;
        case 0x36:
            sidePressed = event.modifierFlags.rawValue & UInt(NX_DEVICERCMDKEYMASK) != 0;
        default:
            sidePressed = true
        }

        if (sidePressed) {
            action = GHOSTTY_ACTION_PRESS
        }
    }

    keyAction(action, event: event)
}

[neurocyte]

Thanks! This is great. It's a minor detail, but it really helps a lot if keybindings can be the same anywhere kitty protocol is used.

[kovidgoyal]

https://github.com/kovidgoyal/kitty/commit/b2587c1d54ff674d2c925ff28b2e16e394794838

[mitchellh]

Thanks, works great.

[kchibisov]

I'm strongly against the change in the way it's done and would suggest to use simple wording to what wayland has

If this event produces a change in modifiers, then the resulting wl_keyboard.modifiers event must be sent after this event.

(see https://wayland.app/protocols/wayland#wl_keyboard:event:key)

This is also how it works on X11 with xkbcommon when you use toolkits, how it's done on macOS usually, and how it's expected to work. The modifier events are also not tied to keys, because you have sticky keys, etc. The common case to demonstrate is how to press Shift + Shift to cahnge layout, since if you do the way you're doing it's basically Shift+Shift, so it's considered a double shift for a single shift press, which is not how it really works, because modifier update is after key event introducing the update.

For example, alacritty implementation just forwards OS state in the order OS wants it, so the behavior you see in alacritty is how OS wants us to report modifiers. Tracking modifiers inside the application is error prone because key press doesn't mean it's a modifier change per se (sticky keys, etc).

I'd also suggest to at least ping other implementors of the protocol before adding such clarifications and breaking everyone with a grace period of e.g. 30 days instead of pushing directly to master and forcing new undesired behavior on everyone else.

[kchibisov]

cc @dnkl

[kovidgoyal]

Your proposal would precisely require the terminal to track modifier state on macOS because in macOS key events are not even reported for modifier keys. You get only a flagsChanged event which contains only the updated state of the modifiers not the state before the modifier was pressed. The current rule is simple and local it does not require tracking the state of modifiers on any platform. And note that this protocol does not support sticky keys which is a linux only abomination.

And pushing to master does not mean a decision is set in stone. You have till the next release to argue all you like. And if you wish to be notified of changes, subscribe the rss feed for the protocol spec file.

[mitchellh]

Sharing an opinion: I think the release event of one side while the other is still pressed should have the associated modifier set, not reset.

The reason is for the scenario that an application wants to implement stateful modifiers. A hypothetical example is when "shift" is held URLs should be highlighted, or something like that. With the current implementation, an application would have to check the bit state AND whether the key is a sided control key while remembering if the opposite side is still pressed.

If we changed the wording to something like:

When both left and right control keys are pressed and one is released, the release event must have the ctrl bit set if the other key is still pressed. Otherwise, the ctrl bit is reset.

With this wording, an application implementing stateful modifiers could inspect only the modifier state in every Kitty keyboard event in isolation in order to set the proper application state.

[kchibisov]

Your proposal would precisely require the terminal to track modifier state on macOS because in macOS key events are not even reported for modifier keys.

The same on Wayland, it's a separate event. You just encode them as modifiers bit, that's how alacritty does it and how OS wants us to do that. We don't track keys, we forward flags changed and modifiers events. The same on X11 (xkbcommon) and windows.

The current rule is simple and local it does not require tracking the state of modifiers on any platform. And note that this protocol does not support sticky keys which is a linux only abomination.

Yet I have to get out of my way to revert the order of events because modifier is sent after key introducing it, and you want before it. You don't even need to support it, it's just how OS wants you to work if you just listen for events. Just attaching mods OS tells you should be fine.

And pushing to master does not mean a decision is set in stone. You have till the next release to argue all you like. And if you wish to be notified of changes, subscribe the rss feed for the protocol spec file.

If you don't want to bother with pinging people which you list on your website, I'd suggest to create a separate repo, I'm not interested in setting up rss reader just to know whether the spec was arbitrary changed without any discussion.

[mitchellh]

For example, alacritty implementation just forwards OS state in the order OS wants it, so the behavior you see in alacritty is how OS wants us to report modifiers. Tracking modifiers inside the application is error prone because key press doesn't mean it's a modifier change per se (sticky keys, etc).

I agree sticky keys are an issue in general, but I think that the keyboard protocol in whatever it specifies should be consistent across platforms. It seems untenable to push the burden of OS-specific keyboard behavior onto terminal application developers.

[kovidgoyal]

Your proposal would precisely require the terminal to track modifier state on macOS because in macOS key events are not even reported for modifier keys.

The same on Wayland, it's a [separate event]

Except it's not the same on wayland because once again, ON MACOS UNLIKE ON WAYLAND, THERE ARE NO KEY EVENTS FOR MODIFIER KEY PRESSES.

Yet I have to get out of my way to revert the order of events because modifier is sent after key introducing it, and you want before it. You don't even need to support it, it's just how OS wants you to work if you just listen for events. Just attaching mods OS tells you should be fine.

And pushing to master does not mean a decision is set in stone. You have till the next release to argue all you like. And if you wish to be notified of changes, subscribe the rss feed for the protocol spec file.

If you don't want to bother with pinging people which you list on your website, I'd suggest to create a separate repo, I'm not interested in setting up rss reader just to know whether the spec was arbitrary changed without any discussion.

If you cant be bothered to track the state of the spec you are implementing, feel free to just remove it from alacritty. It's patently absurd to expect me to do all the work of creating the spec and then on top of that all the work for notifying your lazy ass. I will happily remove the reference to alacritty supporting it.

[kovidgoyal]

@mitchellh I am fine with having the modifier bits represent the overall modifier state on release events.

[kchibisov]

Except it's not the same on wayland because once again, ON MACOS UNLIKE ON WAYLAND, THERE ARE NO KEY EVENTS FOR MODIFIER KEY PRESSES.

They are encoded as a part of flagsUpdate event, since they have NSKeyEvent type and you can extract actual keys if you want. https://github.com/rust-windowing/winit/blob/master/src/platform_impl/macos/view.rs#L928 , but you don't really need keys, because modifiers could be changed without any key being pressed at all. Wayland does that when you don't have keyboard focus and it just sends you modifiers without any key event, so you can apply them for mouse.

Besides, how should I press Shift + shift with your proposal, because it's the thing that is generally not possible, without assumption of left+right shift to be both shift modifiers, etc.

Also, how should I implement it when the event for modifiers change is send after the key introducing the modifier on Wayland? I'm not even tracking anything to get the behavior you observe in alacritty, since we just forward the events from the windowing system.

[kovidgoyal]

Except it's not the same on wayland because once again, ON MACOS UNLIKE ON WAYLAND, THERE ARE NO KEY EVENTS FOR MODIFIER KEY PRESSES.

They are encoded as a part of flagsUpdate event, since they have NSKeyEvent type and you can extract actual keys if you want. https://github.com/rust-windowing/winit/blob/master/src/platform_impl/macos/view.rs#L928 , but you don't really need keys, because modifiers could be changed without any key being pressed at all. Wayland does that when you don't have keyboard focus and it just sends you modifiers without any key event, so you can apply them for mouse.

Again, the flagsUpdate event contains the state of the modifiers after the key has been pressed. There is no way that I know of to extract the value of the modifiers before the event is sent. Therefore, in order to implement your proposal in macos a terminal would need to store the previous modifiers state somewhere and use it when reporting the flagUpdate event to the application.

Besides the how should I press Shift + shift with your proposal, because it's the thing that is generally not possible, without assumption of left+right shift to be both shift modifiers, etc.

I dont follow, You want to handle shift + shift you handle the actual key press events. The state of the shift modifier bit is irrelevant.

Also, how should I implement it when the event for modifiers change is send after the key introducing the modifier on Wayland? I'm not even tracking anything to get the behavior you observe in alacritty, since we just forward the events from the windowing system.

You cannot in general implement a cross platform protocol without doing some work. to convert platform specific events into cross platform events. How you chose to do it is up to you. Run kitten showkey -m kitty inside kitty on wayland and see that it is indeed implemented. kitty use libxkbcommon on both X11 and wayland I dont recall having to do anything particularly difficult to implement this.

[kchibisov]

You cannot in general implement a cross platform protocol without doing some work. to convert platform specific events into cross platform events. How you chose to do it is up to you. Run kitten showkey -m kitty inside kitty on wayland and see that it is indeed implemented. kitty use libxkbcommon on both X11 and wayland I dont recall having to do anything particularly difficult to implement this.

Seems like you just don't want to listen, the event on wayland is after the key, and the only way to know is to batch somehow or desync with what Wayland compositor wants from you, because compositor is the one sending events, you have no control over that.

Again, the flagsUpdate event contains the state of the modifiers after the key has been pressed.

it should be tracked by the toolkit, but it's something that possible to make cross platform, unlike what we have now, since again, you have no control over modifier updates on Wayland.

Like alacritty somehow works the same on every platform when it comes to modifiers behavior. And the modifier change sequence you see on alacritty repo is the same on every platform. It even works with sticky keys, etc, because we just listen to modifier changes.

[kchibisov]

Run kitten showkey -m kitty inside kitty on wayland and see that it is indeed implemented.

Yes, it's broken now. Compositor send events in that order:

[2397299.895] wl_keyboard@18.key(44459, 6864952, 42, 0) [2397299.950] wl_keyboard@18.modifiers(44460, 0, 0, 0, 0) [2397299.994] wl_keyboard@18.key(44461, 6864953, 29, 1) [2397300.108] wl_keyboard@18.modifiers(44462, 4, 0, 0, 0)

Yet my first shift is with shift modifier, but Wayland compositor told me that it was without it, so you just desync and break keyboard input.

[kovidgoyal]

At this point you are just saying, the spec should be changed to match alacritty's behavior rather than anybody else's. If alacritty had done the work to create the protocol that might even be a justifiable position, given that you didn't, it isn't.

If you want to argue for a particular rule then please justify the rule on its own merits rather than on how easy or difficult it is for you to implement. Both kitty and ghostty are existence proofs that the current rule is easily implementable on macOS and Linux. It was literally a one line change for me to implement it in kitty. I dont know what it took for @mitchellh to implement it since his repo is private but I doubt it was more than half an hour of work, less time than you have spent arguing here already.

[kovidgoyal]

Run kitten showkey -m kitty inside kitty on wayland and see that it is indeed implemented.

Yes, it's broken now. Compositor send events in that order:

[2397299.895] wl_keyboard@18.key(44459, 6864952, 42, 0) [2397299.950] wl_keyboard@18.modifiers(44460, 0, 0, 0, 0) [2397299.994] wl_keyboard@18.key(44461, 6864953, 29, 1) [2397300.108] wl_keyboard@18.modifiers(44462, 4, 0, 0, 0)

Yet my first shift is with shift modifier, but Wayland compositor told me that it was without it, so you just desync and break keyboard input.

The purpose of the kitty keyboard protocol is not to match the wayland compositor. When I press shift in wayland the kitten correctly hsows me the event shift+LEFT_SHIFT_PRESS as expected. When I release it, it shows me LEFT_SHIFT_RELEASE.

[kchibisov]

You understand that you deliberately break Wayland input model here and shift doesn't mean that the shift modifier should be pressed and we have special events for modifiers which we should encode instead of relying on keys? That way you can have keyboard input consistent in your platform of choice and even have things like sticky keys working automatically.

My point is not how easy it's to do something or not, but because it's literally impossible to invert the order, because you don't know whether shift keysym was a modifier change or not, since you get this information from the OS and not develop on your own.

The problem is not the amount of changes needed, but about making sense and obey to the rules supported by platform. What you're doing on Wayland is not how it should be done.

[kovidgoyal]

Again, this is a cross platform protocol. One of its goals is to allow the development of command line programs that do not need to care what platform the terminal emulator they are running in is. Tomorrow, when Linux people get bored of Wayland and invert FooBar display server that decides to implement keyboard events in some other fashion, commnad line program developers should not need to care. It is up to the terminal implementors to keep the protocol working as per spec.

[kchibisov]

Again, you can't press Shift + shift with your approach, you also don't have sticky keys working, while they just work in alacritty without anything required or any tracking at all. X11 toolkits also update after the key, not before. Simply ignoring wl_keyboard::modifiers(you do that on glfw level, so you change behavior of kitty itself from what I can see) is not how you should handle modifiers on Wayland, sorry.

[kovidgoyal]

So to summarize the current status:

1) @mitchellh and I are in agreement that the the current wording should be changed so that release events have modifier bits set as per the modifier state. If anybody has any objections to this let me know, otherwise I will push it.

2) @kchibisov wants the keyboard protocol to follow wayland. This is an absolute no as far as I am concerned. What I am willing to countenance, is the following wording: The modifier bits should reflect the modifier state as it was before the key event.

Note however that (2) requires terminal emulators to track modifier state globally on macOS so I am not a fan. If other people are, feel free to chime in I am willing to have my mind changed. But please refrain from talking about Wayland, that is going to carry no water with me.

[kchibisov]

Also, when it comes to writing cross platform protocol, you pick the common denominator at the time of writing, which is Wayland behavior with updating keys after event introducing the modifier change, since you can clearly do that anywhere, and no-one will bother with before since it simply doesn't work.

In general, modifier change is not attached for keys.

[kovidgoyal]

Again, you can't press Shift + shift with your approach, you also don't have sticky keys working, while they just work in alacritty without anything required or any tracking at all. X11 toolkits also update after the key, not before. Simply ignoring wl_keyboard::modifiers(you do that on glfw level, so you change behavior of kitty itself from what I can see) is not how you should handle modifiers on Wayland, sorry.

I have no idea what you mean. I pressed the left shift key and then the right shift key and both events were reported as expected by the kitten.

[kchibisov]

I have no idea what you mean. I pressed the left shift key and then the right shift key and both events were reported as expected by the kitten.

first shift must not have have shift modifier attach, since it looks like you already pressed double shift. (what if you have it latched upon focusing the application?).

[kovidgoyal]

I have no idea what you mean. I pressed the left shift key and then the right shift key and both events were reported as expected by the kitten.

first shift must not have have shift modifier attach, since it looks like you already pressed double shift.

Given the wording of the spec having shift attached explicitly does not mean that. So again, I dont know what your problem is.

[kchibisov]

The modifier bits should reflect the modifier state as it was before the key event.

Isn't it what Wayland spec is all about? Because before no modifier was pressed, so you'll have Shift for regular shift press, and shift + shift when you press other shift? If that's so then acked-by.

[kovidgoyal]

Phew. Enough arguing. @kchibisov is happy with (2). @mitchellh is happy with (1). I dont much care either way. I'll leave this here for a few days if anyone else wants to comment.

Please comment only to vote on (1) or (2). To recap:

1) The modifier bits should reflect the modifier state taking into account the current key event. This is the platform behavior of macOS and Windows ( @neurocyte @j4james @dankamongmen @kovidgoyal ) 2) The modifier bits should reflect the modifier state as it was before the current key event. This is the platform behavior of Linux. (@kchibisov @rockorager @mitchellh)

[kchibisov]

Just to clarify, so we have more concrete example. Does 2) mean that behavior will match kitty's behavior on linux before your recent patch?

And 1 is a new behavior + modifier attached for release?

[kovidgoyal]

Yes, (2) will match kitty's current released behavior on linux. (1) matches kitty's released behavior on macOS. And there wont be a kitty release before I make a final decision on this issue.

[kchibisov]

cc @dnkl (foot) @wez (wezterm) as other terminals implementing this protocol.

foot should be doing 2 already.

[neurocyte]

I would like cross platform consistency. That is what is most important.

Besides that though, option 1 seems much more logical to me as a TUI application developer because the last event received always represents the current global modifier state.

[kovidgoyal]

@neurocyte yes, (2) means you wont know what the global modifier state is until the next key or mouse event. Which means, for example, if you want your application to react in some way to a modifier key press, say underline links when the user presses shift, you would need to listen for left and right shift events rather than the shift modifier state.

[kchibisov]

@neurocyte just fyi, option 2 is how it actually works cross platform, and how a lot of toolkits work, that's also how Wayland/X11 works. Only macOS requires a bit of tuning here iirc, but if you abstract away from actual keys on macOS the change is also how it's. While it could sound logical, in practice it's unreliable because you could have modifier latched without actually a key event (you pressed it before you focused your terminal and then pressed shift again).

Which means, for example, if you want your application to react in some way to a modifier key press, say underline links when the user presses shift, you would need to listen for left and right shift events rather than the shift modifier state.

Yeah, but it's mostly because modifiers are attached to key/mouse events in the first place, while in reality they are separate (you can change them without any key press). If you want a compromise you can simulate modifiers change, when it actually happens on OS level by defining some synthetic events users of the protocol could listen to when they enable encode all or similar, because that issue only arises when you actually want to listen for modifiers.

[neurocyte]

While it could sound logical, in practice it's unreliable because you could have modifier latched without actually a key event (you pressed it before you focused your terminal and then pressed shift again).

And how is an application supposed to deal with that? That doesn't make sense. The terminal emulator has to compensate and create a consistent state by synthesizing release and press events on unfocus/focus.

[kovidgoyal]

Changed the wording of the options to make it symmetric and clearer what the choice is.

[rockorager]

TUI dev here: I prefer option 2. Keeps key matching easier on the app side. Code path can be the same for the following cases:

A. Did I press "a"? B. Did I press "alt+a" C. Did I press "alt"? D. Did I press "alt+alt"?

With option 1, I have to ignore alt state when trying to match scenario C. Scenario D I have to keep state of modifiers within my app since this could be given by two events (single alt press or alt + alt)

Under option 2, all of these follow the same logic for key matching.

Edit: I recognize my app logic would actually be listening to "left_alt || right_alt" in scenario C and "alt+left_alt || alt+right_alt" in D.

[kchibisov]

Usually event state is broadcasted for the gui application itself when you focus it and on some platforms it's global, so you should track them. Focus/Unfocus is not really an issue, since if you have mouse mode enabled you'll get mouse move with the correct modifiers. If you start typing it'll also be with correct modifiers. The issue is only when pressing modifier without actually moving as it was said, but I don't see issue with having synthetic event to sync modifiers if this is really wanted for encode all, given that you can send it from the flags changed/wl_keyboard::modifiers/batching secondary.

[neurocyte]

@rockorager Neither option resolves this issue because there is only one modifier bit for both alt keys. So your application has to track the state itself if you want to differentiate between left and right modifiers.

[rockorager]

@rockorager Neither option resolves this issue because there is only one modifier bit for both alt keys. So your application has to track the state itself if you want to differentiate between left and right modifiers.

If I want to track if a key was right_alt modified or left_alt modified, yes. But not if I don't care which side modified the press.

[kovidgoyal]

I am tracking the votes of everyone in the summary post. We seem to have an even split so far :)

[neurocyte]

@rockorager Neither option resolves this issue because there is only one modifier bit for both alt keys. So your application has to track the state itself if you want to differentiate between left and right modifiers.

If I want to track if a key was right_alt modified or left_alt modified, yes. But not if I don't care which side modified the press.

Then I don't quite see your point. The modifier bits are always set in a press event. We were discussing release events and whether or not the modifier bits in release events should reflect the resultant state of the event or the previous state (i.e. repeat the bits that were in the press event)

[rockorager]

@neurocyte It looks like from your example the modifier bit changes to "on" for the modifier keypress as well...is that not the case? (Im on linux so the linux behavior is the only way I've ever seen it)

[rockorager]

The modifier bits are always set in a press event. We were discussing release events and whether or not the modifier bits in release events should reflect the resultant state of the event

Ah I see. In Linux, the modifier bit is not present on the modifier key press: only for any resulting presses. This is the behavior I want.

[neurocyte]

Yes, sorry. This discussion has drifted from the original issue to a discussion about the changes to the kitty keyboard protocol spec that were made as a part of the fix to the original issue.

[rockorager]

Alright. @kovidgoyal will the press events change as a result of this vote? If this only affects releases, I retract my vote - I only care about how the modifier bits are reported on the press events

[kchibisov]

@rockorager you'll get Shift + Shift for just pressing Shift with 1) and just shift with 2. This changes presses as well and that's why I care about 2 since 1 forces into glfw impl of macOS input, which can do 2, but you can't do correct wayland impl of 1.