Key events with wrong key code (regression in wxGTK) (Issue #23680)
taler21
Description
When pressing, for example, the keys Z and Y on a German keyboard, the key events EVT_CHAR_HOOK, EVT_KEY_DOWN and EVT_KEY_UP are generated with a wrong key code.
Note that the German keyboard layout has the Z and Y keys swapped compared to the US keyboard layout.
So it seems that these key events are generated according to the US keyboard layout.
With wxWidgets-3.2.2.1 it works correct.
So it is a regression that can be reproduced with both the current master and the current 3.2 branch.
The issue can be seen in the keyboard sample.
With wxWidgets 3.2.2.1 it works correct.
With current master and 3.2 branch:
Platform and version information
- wxWidgets version you use: current master and 3.2 branch
- wxWidgets port you use: wxGTK
- OS and its version: openSUSE Leap 15.1
- GTK version: 2.24.32
- Which GDK backend is used: X11
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
This could be due to 2c0f6a2 (Fix wxKeyEvent::GetKeyCode() for non-US keyboard layouts, 2023-03-31), cherry picked as 87ae8d5 in 3.2, see #23410.
In any case, this definitely needs to be fixed or reverted before 3.2.3, thanks a lot for testing and reporting this.
cc @unxed
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
taler21
Yes, I tested it and can confirm that this commit is the culprit.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
unxed
@taler21 thanks for reporting this! I will try to look into it then I have some time. Btw, what key exactly do you press when getting "/" as KeyDown? Also, what exact German keyboard layout are you using? I see three ones in my settings: E1, E2 and US.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
unxed
needs to be fixed
Are we sure it is a bug? We agreed that the "key code" field should identify the key, and not the letter written on it. Thus, the current behavior looks correct: physically the German Y is the same key as the English Z, so since we decided the key codes should correspond to the English layout key ASCII values, then the current behavior is ok. And Char event should return the letter, not key code, so it returns the German Y, just as it should.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
So while the new behaviour is arguably more logical, we can't change it like this in 3.2 and I'd like to find a solution working both for master and 3.2. One thing I thought about was to keep the old behaviour for the ASCII keys, which should take care of `Y`/`Z`. But it won't help with `Ö`, `Ü` or `Ä` that are also present on German keyboards. Nor with `/` shown above (which I don't quite understand as `/` is supposed to be `Shift-7` AFAICS?).
OTOH I don't think we ever guaranteed anything about `Ö` and friends, so it would be fine to change the behaviour for them at least in master
taler21
So while the new behaviour is arguably more logical, ...
Why would the new behaviour be more logical?
For the same shortcut (e.g. Ctrl+Z for Undo) different physical keys have to be pressed on a German keyboard than on an US English one.
I can't see any sense in wxKeyEvent::GetKeyCode() always returning the key code for an US English keyboard layout, regardless of the actual keyboard layout used.
The new behaviour is also inconsistent with other platforms (at least with wxMSW).
Nor with / shown above (which I don't quite understand as / is supposed to be Shift-7 AFAICS?).
The - key on a German keyboard corresponds to the / key on an US English keyboard.
It's not just the Y and Z keys that are assigned differently on the German and US English keyboards.
And the German keyboard layout is not the only one that differs from US English keyboard layout.
A French keyboard, for example, also has differences.
I agree that compatibility for the ASCII keys is the really essential compatibility.
However, I also think that for the other keys, compatibility should only be broken if there is a really good reason to do so.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
This behaviour is logical for non-Latin keyboards because it allows to get something meaningful as the key code instead of just 0 for them. It was also supposed to make things more consistent with MSW, not less...
Re -, I looked at the wrong keyboard layout: what matters is indeed where '/is on the US layout, not on the German one. And [Cyrillic-using layout](https://upload.wikimedia.org/wikipedia/commons/thumb/6/60/KB_Russian.svg/600px-KB_Russian.svg.png) probably has the same problem with.key: if the key code returned for it is/too (and it should be, unless I'm really missing something because it's the same physical key), thenCtrl-.` accelerators don't work as expected there.
@unxed I think we should only use US layout key codes for the keys for which we wouldn't have any code otherwise. What do you think and can you modify your patches to do it like this instead?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
unxed
@unxed I think we should only use US layout key codes for the keys for which we wouldn't have any code otherwise. What do you think and can you modify your patches to do it like this instead?
Yes, that's for sure possible. Btw, let me think about this a little bit. We should be sure that chosen solution is best.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
The problem is that I wanted to make a release this Friday. We need to decide whether we should
- Revert the commits mentioned above and go ahead with the release as planned (safest).
- Apply new fixes on top of the existing commit and include them in the release (probably infeasible in the remaining time?).
- Delay the release until this is fixed (could be worth it to include the fixes there).
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
unxed
Can we introduce some kind of switch or flag to toggle en-us key codes usage by client application?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Can we introduce some kind of switch or flag to toggle en-us key codes usage by client application?
I thought about this, but this would be the very last choice for me for all the usual reasons global switches like this are undesirable, i.e. briefly because they have non-local effects, e.g.your application might use a library that is only compatible with the default value (or, worse, you use a library which changes the default value, which is even worse).
In this particular case I also think that there must be the right choice for the event keys and I just don't think we have chosen it correctly. In particular, it seems that the key code for . must be wrong right now even with Russian keyboard layout, isn't it?
The only acceptable global option for me would be one that would allow to opt-in into the future behaviour in 3.2 branch, but I definitely don't want to have any such option in master, where we should be able to convincingly justify any incompatible changes, and, ideally, not in 3.2 neither.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
unxed
I've tested this with MSW wx version. It's behavior is the same as current GTK version behavior:
So if we will enable new behavior only for zero keycodes, we will make different backends behave in different way.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
This is very strange because I definitely don't see this here (it's all "Z"s for me). Are you using remote X display or something like this? Could it be that it somehow interferes with the keys you see?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
unxed
No remote display, just Wine in X11 on latest Linux Mint x64 Cinnamon.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Maybe the key codes come from X11 then? How do you even change the keyboard layout in Wine? I don't think I've ever done this...
Please test in a virtual machine (KVM can run it just fine), you should see the correct behaviour.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
unxed
It is implausible that in some cases Windows would use key codes tied to the physical location of the key, and in other cases the same API method would return in the same field the key codes tied to the selected keyboard layout.
I need time to make some tests to determine exact Windows behavior.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
unxed
It is implausible that in some cases Windows would use key codes tied to the physical location of the key, and in other cases the same API method would return in the same field the key codes tied to the selected keyboard layout.
I need time to make some tests to determine exact Windows behavior.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
unxed
It is implausible that in some cases Windows would use key codes tied to the physical location of the key, and in other cases the same API method would return in the same field the key codes tied to the selected keyboard layout.
I need time to make some tests to determine exact Windows behavior.
First of all I asked my friend having Windows PC to make screenshots of pressing "-" in German layout (in German layout it should produce "ß" char) and "o" in Russian layout (it should produce "щ" char) in the same app. Let's see what happens.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
unxed
I need to make sure that the rule "key press/release events return key codes associated with the selected keyboard layout" always work on Windows, in all cases, and that the bug in my case is inside Wine.
So I need time to make some tests to determine exact Windows behavior.
First of all I asked my friend having Windows PC to make screenshots of pressing "-" in German layout (in German layout it should produce "ß" char) and "o" in Russian layout (it should produce "щ" char) in the same app. Let's see what will happen.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
unxed
It's hard for me to imagine that in KeyDown events Windows gives us key codes in English kb layout if Russian kb layout is selected, but in German kb layout if German kb layout is selected.
So I need time to make some tests to determine exact Windows behavior.
First of all I asked my friend having Windows PC to make screenshots of pressing "-" in German layout (in German layout it should produce "ß" char) and "o" in Russian layout (it should produce "щ" char) in the same app. Let's see what will happen.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
unxed
It's hard for me to imagine that in KeyDown events Windows gives key codes in English kb layout if Russian kb layout is selected, but in German kb layout if German kb layout is selected. What is the trigger in Windows that determines that a Latin character from en-us kb layout should be used as key code? This trigger obviously fires in the case of Cyrillic, but in what other cases? If we do not know this for sure, we cannot be sure that we are reproducing the behavior of the Windows port correctly.
So I need time to make some tests to determine exact Windows behavior.
First of all I asked my friend having Windows PC to make screenshots of pressing "-" in German layout (in German layout it should produce "ß" char) and "o" in Russian layout (it should produce "щ" char) in the same app. Let's see what will happen.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
unxed
German layout, "y" key ("z" char):
German layout, "-" key ("ß" char):
German layout, "[" key ("ü" char):
Russian layout, "o" key ("щ" char):
I still can’t understand in which cases in KeyUp/Down events the characters are in the current keyboard layout, and in which the characters are in English layout (as in the case of the Cyrillic layout). I have a hypothesis that the corresponding character of the English layout is used if the character in current layout is not in the ANSI code page for system locale. Do you have any other ideas what could be the trigger?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
I think it's because we return the key if its Unicode key code can fit into 0..256 range, see https://github.com/wxWidgets/wxWidgets/blob/44b99195bc4395944bab8071c6d7adcbdcdf8773/src/msw/window.cpp#L6821 and the code around it.
IOW Windows always gives us fixed VK_xxx and we call MapVirtualKey() on it and use its value as the key code if it's "Latin". This may or not be a good idea, but considering that this worked like this for a very long time, I'd be very reluctant to change it.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
unxed
Are you sure "ü" fits 0..255 in Unicode?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
unxed
Okay, it does.
So, if you assumption is true, are you sure it would be right to keep old behavior for all cases except keycode is zero? Are we sure that it is zero only in cases then code does not fit 0..255, and it is zero in all such cases?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
For these keys (VK_OEM_1, ...) we are certainly sure that we use WXK_NONE as the key code because this is what our code does. There might be other keys not listed there though on some non-standard keyboards, but this won't/can't change the behaviour for all the "normal" keys.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@unxed I still think the proposal from my earlier comment is the best thing to do here. It would be great if you could please do it to restore compatibility while improving the behaviour for non-Latin layouts. If not, I'll try to do it myself, but if I can't do it reasonably quickly, I'kk have to just revert all these changes.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
unxed
I'm on kind of little vacation without a computer. I can probably watch on Tuesday. We should probably move the new keycode detection code inside the condition that the keycode is still null after trying to set it in older way.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Sorry for nagging, but I'm back from vacation now and would really like to do something about this issue because the longer it stays like this, the higher the chance of people being confused by this behaviour, especially when it reverts back.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
unxed
try #23818
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Closed #23680 as completed via 3492bac.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.