Created attachment 331324 [details]
Testcase (with proper UTF-8 literal)

I can reproduce this here (attaching a fixed version of the testcase from bug 136280, with a message when button is pressed, and with window destruction being handled correctly).

When the testcase starts with a RU input language initially, the output in the console is:
** Message: Keyval 1734
** Message: keyval = 65513, hardcode = 18, group = 0
** Message: keyval = 1734, hardcode = 65, group = 0
** Message: clicked

When the testcase starts with an EN input language initially, and then layout is switched to RU, the output is:
** Message: Keyval 1734
** Message: keyval = 65513, hardcode = 18, group = 0
** Message: keyval = 1734, hardcode = 65, group = 0
(no "clicked" message)

More code info:
* GTK calls _gtk_key_hash_add_entry(), which populates GtkKeyHash->entries_list (which is a singly-linked list). This is done for each mnemonic.
* When GTK needs to check the mnemonics (on keypress), it calls _gtk_key_hash_lookup(), which calls key_hash_get_keycode_hash(), which goes through the entries_list and populates GtkKeyHash->keycode_hash hash table with its contents. Further calls to key_hash_get_keycode_hash() will return the same hash table.
  The hash table is populated by key_hash_insert_entry() calls, which calls gdk_keymap_get_entries_for_keyval() to translate keyval to keycodes.
  Once hash table exists _gtk_key_hash_add_entry() will also add entries to it
* gdk_keymap_get_entries_for_keyval() returns different results depending on which layout is currently active. When layout doesn't match, it returns 0 keycodes. When it does, it returns 1 keycode.
* If on runs the testcase with EN keyboard layout, then changes the keyboard layout to RU without using the keyboard (clicking the layout switch in the tray and choosing a different layout from the menu), then triggers the mnemonic - it works, because key_hash_get_keycode_hash() (and thus gdk_keymap_get_entries_for_keyval()) is called with the correct keyboard layout.

One "easy" fix this is to propagate keyboard layout changes from GDK to GTK and trigger GtkKeyHash->keycode_hash rebuild. That said, it would still mean that mnemonics will work with the "right" layout and not work with the "wrong" layout, whereas, i think, the desired outcome is for mnemonics to work in all layouts (so Alt+Ф clicks the button in RU layout and Alt+A clicks the button in EN layout).

In bug 136280 it was suggested that GDK should fetch the list of all available keyboard layouts and later use that info to correctly supply keycodes when given a keyval.
Does X11 backend actually work this way?

This is complicated by the fact that W32 interpretation of the X11 concepts of "level" and "group" seem to be different.

In X11, "group" is the keyboard layout (i.e. input language), while "level" is the shift of the meaning of the key, which depends on the state of the modifiers. So the number of levels is 2 to the power of the number of modifier keys. Number of groups can also be different. There's an "active group" (currently-active keyboard layout), and X will indicate which group is active, when it sends a key event. Level is an internal X nomenclature, it's used to navigate the keymap date sent by X, and GTK computes the level from the state of the modifier keys.

W32 backend has 2 groups and 2 levels. Group 0 is the "normal" group, Group 1 is the "AltGr" group (activated while Ctrl and Alt are both pressed). Level 0 is when Shift is not pressed. Level 1 is when Shift is pressed.
Thus W32 backend does not use groups do indicate keyboard layout. In fact, it will completely recreate the keymap when keyboard layout changes, using currently-active keyboard layout to fill the keymap.

Therefore i can't just bolt on extra layout lookups. The "correct" fix for this is to rewrite the level/group logic for the W32 backend to make it use level for shift & altgr (4 levels) and group for layouts (the number of groups would be the number of available keyboard layouts).

Created attachment 331451 [details] [review]
GDK W32: Cache multiple keyboard layouts simultaneously

Good news: this patch totally fixes this problem.

Bad news: lots of moving parts here, impossible to fully test
(i've only tested with EN-US, EN-US-International and RU-RU),
has (needed) whitespace changes mixed in with the code changes,
and i do not really understand some of the finer points of
keyboard input handling (gdk_win32_keymap_translate_keyboard_state() is
especially toublesome).

Created attachment 331620 [details] [review]
GDK W32: Cache multiple keyboard layouts simultaneously

v1.1:
* minor comment fix

Hi,

I am also having some hard time trying to reproduce this exactly, as the keyboards we have here are based on EN-US layouts (we use an IME to input CJK characters by using a combination of keys that are put together to give the characters we need on a EN-US based keyboard).

I think it would be great if Siarhei can come back and give us an idea whether the fix works for him, as this is an issue that I couldn't manage to come across even after trying to install various input languages from Windows, including Russian, and trying to follow the steps that were outlined here.

With blessings, and cheers!

Hello,

Could you please point me to some builds of GTK for windows that include that fix?
i've searched for them recently and found 3.20.6 version only.

Actually i want to thank LRN for taking care of keyboard because i managed to workaround this staff by working with Gimp in virtual machine with Ubuntu with english language only.

Thanks

If you feel like it, you could try installing MSYS2 and then asking MSYS2 devs about using MSYS2 to build a GTK+ package from the source (with a patch applied). They must have some kind of streamlined build procedure, especially for stable packages (like 3.20.6).

Same problem here with macbook OS X, Please fix

This is a Windows-specific bug with a Windows-specific fix. If you experience the same problem on OSX (which backend, by the way? Quartz or X11?), file a separate bug. I'd like to see this bug closed sooner rather than later, and adding yet another facet to it will prevent that (especially since i can't fix anything for OSX myself, and we'll have to look for someone who can).

When you do file a separate bug, do explain which keyboard layouts you're using, what program you're running, etc.

Hello

Actually i cloned latest GTK+ sources, but i stuck at its compilation. Instruction provided in README.win32 seems to be outdated (at least there was no such packages in the url). Next thing i found is https://wiki.gnome.org/Projects/GTK+/Win32/MSVCCompilationOfGTKStack. I see there sooo much command line ninjustsu... Arhh! in my .net projects i'm using submodules and build.bat to build all the staff, but i see how much systems are supported by your project...

Is there any ways i can help you any way without compiling all that staff. as i see its not so straight and i'll have to get on with scripting, compilers.

Thanks

For 32-bit builds, replace "mingw64" with "mingw32" and x86_64 with i686
For gtk2 builds, you need to port the patch to gtk2, which is a non-trivial task for a non-developer

Install MSYS2
Run MSYS

$ pacman -Sy pacman
$ pacman -Syu

Close MSYS, kill pacman
Run msys2_shell.cmd

$ pacman -Su
$ exit

Run mingw64.exe

$ mkdir -p /src/gtk+-3.20.6-1
$ cd /src/gtk+-3.20.6-1

put PKGBUILD and .installs from https://github.com/Alexpux/MINGW-packages/tree/master/mingw-w64-gtk3 into c:/msys64/src/gtk+-3.20.6-1 or wherever it ends up on your machine

$ pacman -S mingw-w64-x86_64-toolchain msys/make msys/patch
$ MINGW_INSTALLS="mingw64" makepkg-mingw -sLf --skippgpcheck

Verify that it works and the bug is present:
$ PANGOCAIRO_BACKEND=fc ./pkg/mingw-w64-x86_64-gtk3/mingw64/bin/gtk3-demo.exe

Drop patches into that directory

$ sha256sum *.patch

Edit PKGBUILD to add new sources, i.e.
> source=("https://download.gnome.org/sources/gtk+/${pkgver%.*}/gtk+-${pkgver}.tar.xz"
>         "9400-GDK-W32-Cache-multiple-keyboard-layouts-simultan.all.patch"
>         "9401-W32-Prefer-the-deadkey-combinations-that-the-OS-.all.patch"
>         "9403-GDK-W32-Special-handling-for-VKPAUSE.all.patch")
> sha256sums=('3f8016563a96b1cfef4ac9e795647f6316deb2978ff939b19e4e4f8f936fa4b2'
>             'b11fed4f286d4dd2b3f134d4fafd8597df81b44f1f6f10e09797c46d8c88744e'
>             'e32a5b50cb719835f374c6de20d50e8ceb98cbb42e4a4c1e795280ef378a3900'
>             '16e664113c9289bf69cd8562922bb116096146ca604df4ef7a173ff084f8e354')
and add them to prepare() section, i.e.:
>  patch -p1 -i ${srcdir}/9400-GDK-W32-Cache-multiple-keyboard-layouts-simultan.all.patch
>  patch -p1 -i ${srcdir}/9401-W32-Prefer-the-deadkey-combinations-that-the-OS-.all.patch
>  patch -p1 -i ${srcdir}/9403-GDK-W32-Special-handling-for-VKPAUSE.all.patch
and build:
$ MINGW_INSTALLS="mingw64" makepkg-mingw -sLf --skippgpcheck

If it fails to build GdkWin32-3.0.gir, edit PKGBUILD and change enable-introspection to disable-introspection

Verify that it works and the bug is fixed:
$ PANGOCAIRO_BACKEND=fc ./pkg/mingw-w64-x86_64-gtk3/mingw64/bin/gtk3-demo.exe

You might need to build the tescase (attachment 331324 [details]) against that version of gtk to test this bug, if there's no mnemonic in gtk3-demo or gtk3-widget-factory. Hopefully, that shouldn't require explanations...

Disclaimer: 32-bit build failed on my system for unspecified reasons (internal MSYS error, it seems), while 64-bit one worked. Go figure...

Created attachment 332411 [details] [review]
Backport some of the GTK3 changes to win32/gdkkeys (gtk2)

Created attachment 332412 [details] [review]
GDK W32: Cache multiple keyboard layouts simultaneously (gtk2)

Requires attachment 332411 [details] [review]

Thank you for the compilation steps, especially for command line ninjutsu.

I managed to reproduce bug on source code without fix

1. Start mingw
2. Type
$ PANGOCAIRO_BACKEND=fc ./pkg/mingw-w64-x86_64-gtk3/mingw64/bin/gtk3-demo.exe
and before pressing Enter switch to Russian.
3. Launch Builder demo
4. Press Ctrl+Q
==> Nothing happens

1. Start mingw
2. Enter with English active keyboard
$ PANGOCAIRO_BACKEND=fc ./pkg/mingw-w64-x86_64-gtk3/mingw64/bin/gtk3-demo.exe
3. Launch Builder demo
4. Press Ctrl+Q
Demo closes

Now i will test it with your patch and post here results.

Also thanks for backport for GTK2, because i see gimp still uses that version somewhy.

With your patch both scenarios works ok

1. Start mingw
2. Type
$ PANGOCAIRO_BACKEND=fc ./pkg/mingw-w64-x86_64-gtk3/mingw64/bin/gtk3-demo.exe
and before pressing Enter switch to Russian.
3. Launch Builder demo
4. Press Ctrl+Q
Demo closes

1. Start mingw
2. Enter with English active keyboard
$ PANGOCAIRO_BACKEND=fc ./pkg/mingw-w64-x86_64-gtk3/mingw64/bin/gtk3-demo.exe
3. Launch Builder demo
4. Press Ctrl+Q
Demo closes

Thanks for fixing that issue. 

Can you please say how can i track when changes will come to gtk-2 so i will ask Gimp to release build. Support for shortcuts is soooo important for me!

So much people around the world will be happy seeing shortcuts working in their GTK applications...

Created attachment 332740 [details] [review]
GDK W32: Cache multiple keyboard layouts simultaneously

v1.2:
* Use gdk_keymap_get_default () in gdkkeyuni.c

Comment on attachment 332740 [details] [review]
GDK W32: Cache multiple keyboard layouts simultaneously

Attachment 332740 [details] pushed as 52c7e07 - GDK W32: Cache multiple keyboard layouts simultaneously

Review of attachment 332740 [details] [review]:

See the minor comments.

::: gdk/win32/gdkevents-win32.c
@@ +527,3 @@
     event->key.state |= GDK_BUTTON5_MASK;
 
+  keymap = GDK_WIN32_KEYMAP (_gdk_win32_display_get_keymap (_gdk_display));

any chance you can avoid using the global var _gdk_display? there is a method to get the default one.

::: gdk/win32/gdkkeys-win32.c
@@ +106,3 @@
 G_DEFINE_TYPE (GdkWin32Keymap, gdk_win32_keymap, GDK_TYPE_KEYMAP)
 
+guint _gdk_keymap_serial = 0;

shouldn't this be static?

@@ +131,1 @@
+  g_clear_pointer (&keymap->keysym_tab, g_free);

g_free already checks for NULL so there is no need to use g_clear_pointer in finalize

@@ +445,3 @@
+
+static gboolean
+layouts_are_the_same (GArray *array, HKL *hkls, gint hkls_len)

one param per line

@@ +485,3 @@
+update_keymap (GdkKeymap *gdk_keymap)
+{
+  int                      hkls_len;

local variables do not need to be aligned

(In reply to Ignacio Casal Quinteiro (nacho) from comment #21)
> Review of attachment 332740 [details] [review] [review]:
> 
> See the minor comments.
> 
> ::: gdk/win32/gdkevents-win32.c
> @@ +527,3 @@
>      event->key.state |= GDK_BUTTON5_MASK;
>  
> +  keymap = GDK_WIN32_KEYMAP (_gdk_win32_display_get_keymap (_gdk_display));

Doable.

> ::: gdk/win32/gdkkeys-win32.c
> @@ +106,3 @@
>  G_DEFINE_TYPE (GdkWin32Keymap, gdk_win32_keymap, GDK_TYPE_KEYMAP)
>  
> +guint _gdk_keymap_serial = 0;
> 
> shouldn't this be static?

No, it's used in multiple files. Inelegant, i know, but there was no urgent need to turn it into a method.

> 
> @@ +131,1 @@
> +  g_clear_pointer (&keymap->keysym_tab, g_free);
> 
> g_free already checks for NULL so there is no need to use g_clear_pointer in
> finalize

I'm trying to use g_clear_pointer() and its ilk when i can. Seems to be cleaner.

> 
> @@ +445,3 @@
> +
> +static gboolean
> +layouts_are_the_same (GArray *array, HKL *hkls, gint hkls_len)
> 
> one param per line
> 
> @@ +485,3 @@
> +update_keymap (GdkKeymap *gdk_keymap)
> +{
> +  int                      hkls_len;
> 
> local variables do not need to be aligned

Okay. Since i've already pushed things into master, feel free to follow up with a style-fixing commit (maybe even for the whole gdkkeys-win32.c, since it just had another change after that).

I haven't pushed it into gtk-2-24 branch, as you previously requested it to be tested more before that.

I would suggest people interested in the 2.24 backport to ask GIMP developers to apply the patches from this bug to their 2.24 gtk+ dev builds and do some testing. Maybe Inkscape devs as well. These seem to be the projects that would benefit the most from this bugfix.

The gtk2 patch in #13 doesn't compile, since there is no implementation for the gdk_win32_keymap_get_type function that that patch adds and uses.

This line:
> G_DEFINE_TYPE (GdkWin32Keymap, gdk_win32_keymap, GDK_TYPE_KEYMAP)
should provide you with an implementation.

You're right. The issue is not that the implementation doesn't exist, but that the implementation isn't exported from gdk.dll.

gdk_win32_keymap_get_type needs to be added to gdk/gdk.symbols. I can get it to build if I add it after gdk_win32_keymap_check_compose

I will attach a fixed patch soon. Anything else?

No, like I said I can get gtk to build once I add that line. As for testing once you give me a complete patch I'll add it to gtk-win32 and TingPing can get it tested by users.

Created attachment 335030 [details] [review]
Backport some of the GTK3 changes to win32/gdkkeys (gtk2)

v2:
* Add gdk_win32_keymap_get_type to gdk/gdk.symbols

Created attachment 335031 [details] [review]
GDK W32: Cache multiple keyboard layouts simultaneously (gtk2)

v2:
* Merge commit b05ed13710dc52c11bb1fde6f6aaf064d8ce5c58 into this patch

attachment 332740 [details] [review] broke Windows XP support by introducing wcscpy_s dependency. Can this be fixed?

That's a good question. I know for sure that GTK3 is Vista+, as well as current glib, but i'm not sure about GTK2.

Also, what *does* XP have, apart from wcscpy? Does it have wcsncpy?

Did glib drop XP support fairly recently or are you mistaken? glib-2.50.2 seems to be working. GTK2 did work until this patch too.

I would hate to see GIMP and Geany stop working on XP in newer versions, so I hope it's correctable.

Here's a list of what is available on XP:
http://xpdll.nirsoft.net/msvcrt_dll.html
wcsncpy is there.

Created attachment 343501 [details] [review]
GDK W32: Replace wcscpy_s with wcsncpy for XP compatibility

Does gtk2 compile (and work) for you with this patch?
Well, "work" is too strong a word, the code path that does string copying is for when GetKeyboardLayoutNameW() fails, which is unlikely.
If you want to test whether it also *works*, remove the if (!GetKeyboardLayoutNameW (hkl_name)) parts too, then run with GDK_DEBUG=events and grep the output for "active, ".

Hi,

Can you also let us know what compiler you at using?

Note that this only happens in vanilla MinGW if it fails to build, and if I recall correctly mingw-w64 has fallbacks for XP on the _s CRT functions.  This should not happen with supported Visual Studio (2008+) builds, since the build will link to newer CRTs which will implement these properly.

With blessings, and cheers!

The problem was with the dll compiled by MSYS2 project, I believe they used MinGW-w64 to make it, but it was linked to nonexistent wcscpy_s in msvcrt.dll.

LRN, thank you for the patch, it runs now. I'm currently recompiling with the removed "if" and --enable-debug=yes to give you the output.

But it didn't fix the issue with non-working hotkeys in Russian layout for me. Maybe it is another bug, since my default input language is English, and the bug description here says it doesn't appear with English set as default language. How should I proceed? I can reliably reproduce the bug in Geany.

> gtk-demo.exe | grep "active, "
Have 2 keyboard layouts: 0x04090409(active, (NULL)) 0x04190419

Is this the expected output?

(In reply to cordylus from comment #35)
> But it didn't fix the issue with non-working hotkeys in Russian layout for
> me. Maybe it is another bug, since my default input language is English, and
> the bug description here says it doesn't appear with English set as default
> language. How should I proceed? I can reliably reproduce the bug in Geany.

It should work. Try compiling the testcase from this bug and see if it works there. If it does, but doesn't work in Geany, then either Geany does something weird, or there's somehow a path not covered by the new keyboard code.
It shouldn't matter what your default input layout is, gtk should correctly trigger hotkeys from the same physical keyboard keys (regardless of what character they type, i.e. regardless of what layout is being used).

(In reply to cordylus from comment #36)
> > gtk-demo.exe | grep "active, "
> Have 2 keyboard layouts: 0x04090409(active, (NULL)) 0x04190419
> 
> Is this the expected output?

Yes. It correctly copies the (NULL) string.

Compiled attachment 331324 [details]. I don't get "clicked" message in any layout and with any default (initial) layout. Other messages printed are equivalent to those in Comment 1.

Yeah, strange, doesn't work for me either. I'll look into it.

Sorry, i misspoke. The "didn't work for me" part was due to an older version of gtk2 on which i've tried the testcase. Once i've realized that, i turned to gtk2-git-cc2d701, and everything worked just fine there.

Which version of gtk2 were you testing this on, again?

If you compile with --enabled-debug and run with --gtk-debug=keybindings, you should see something like this:

Alt+A
** Message: keyval = 97, hardcode = 65, group = 0
Gtk-Message: Looking up keycode = 65, modifiers = 0x0008,
    keyval = 97, group = 0, level = 0, consumed_modifiers = 0x0003
Gtk-Message:   found group = 1, level = 0
** Message: clicked

Alt+Ф
** Message: keyval = 1734, hardcode = 65, group = 1
Gtk-Message: Looking up keycode = 65, modifiers = 0x0008,
    keyval = 1734, group = 1, level = 0, consumed_modifiers = 0x0003
Gtk-Message:   found exact match, keyval = 1734, modifiers = 0x0008
** Message: clicked

If you want to debug it, the relevant function is _gtk_key_hash_lookup(), put a breakpoint there with cond hardware_keycode == 65 to ensure that it only triggers on "A" physical key.
The function that ultimately calls (or doesn't call) a mnemonic is gtk_window_activate_key().

Thank you for carrying on.

I was using the version from MSYS2, it is based on the latest stable 2.24.31 with some patches, including the patches from this bug. Probably this is the reason why I'm not getting the "clicked", "found group", "found exact match" messages. Perhaps some other changes were introduced in the git, so cherry picking these patches did not brought the fix.

I am currently trying to build gtk2-git-cc2d701, but still without success. What are you using to build on Windows? Don't you use MinGW? What is your build process?

You can find the list of commits since 2.24.31 here[1]. You need all five of my commits. And the wcscpy patch.

MSYS2 gtk2 package[2] applies most of these. It also applies attachment 331324 [details], so it seems that the maintainer is paying attention. I think it's missing the private header patch[3] though. Not sure how these source files correspond to the actual binaries you can get with pacman.

I'm using MSYS2 as well, but a special, self-built version of it. I'm not using pacman or any packages from MSYS2 repositories, though we mostly apply the same patches.

[1] https://git.gnome.org/browse/gtk+/log/?h=gtk-2-24
[2] https://github.com/Alexpux/MINGW-packages/tree/master/mingw-w64-gtk2
[3] https://git.gnome.org/browse/gtk+/commit/?h=gtk-2-24&id=cc2d701289ec3fe7b0e375e89981cde333bef989

I don't know what I'm doing wrong, I've ensured that all your patches are applied (needed to add only the last one, just as you said), but still no "clicked" message. Could you please email me or publish somewhere the dlls with which it works for you, so we could determine whether this is a problem with the bugfix not working on my system or just with the version I'm building?

Installed official msys2 i686 gtk2 (and gcc, and pkg-config), compiled the test program against it, ran it, and it worked (it does have weird issues with outputting into MSYS2 default console, where there's no output at all, until the program terminates, but that's another matter entirely).
Thinking that this might be OS-related, i've also tried to run it on XP (in a VM), and it also worked there.

Thank you for taking your time to test. Did you use the binary package gtk2-2.24.31-2 or compiled from sources gtk2-2.24.31-3? It hardly matters, but just in case.

Also, did you use a Russian version of XP?

(In reply to cordylus from comment #46)
> Thank you for taking your time to test. Did you use the binary package
> gtk2-2.24.31-2 or compiled from sources gtk2-2.24.31-3? It hardly matters,
> but just in case.

Both - i did re-build gtk2 with my mingw, and i *also* tried gtk2-2.24.31-2 binary packages from MSYS2 repos; also both of these were on Windows 10 *and* XP.

> Also, did you use a Russian version of XP?

Yes, in the sense that the "Language for programs that do not support Unicode" setting was set to Russian; the OS UI was in English, but it shouldn't matter.

If it doesn't work for you, the only solution i see is to use gdb and dig into the code (in that case compiling it with -O0 might help). Sprinkle lots of GTK_NOTE or GDK_NOTE macro calls around to dump the info about mnemonics that are registered, and the process of matching them. Also, modify the testcase to include two buttons - one "_Файл" and one "_File", for example to be able to debug both the functional and the non-functional codepaths for comparison.

I feel so sorry for taking your time with all that testing, that was completely my fault with the testcase, I had an AutoHotkey script registered for Alt+A, so my output was for subsequent calls of Alt, then A. And therefore no clicks.

Your patch is great, I tried the testcase in a folder with GIMP, it doesn't click there, and does click with the latest MSYS2 dlls, so it is definitely working.

But my original problem with Geany is indeed not solved with this bugfix. Of course I should now add that it's not a problem with my setup, since the bug was already reported before me, and I've made sure that it reproduces on another (Win7) machine. Geany developers say it should be a GTK problem. To reproduce, open Geany, mouse click the Edit menu item, return to the textarea, switch to Russian layout, write something, try to Ctrl+C, Ctrl+V, it won't copy anymore, but will paste what you had previously in the clipboard. I've tested it by substituting the dlls from the binary release with the new ones. I'm not sure if it should be fixed this way, so I'm on the way to recompile Geany and test the recompiled version.

The recompiled version still has the problem.

Oh-kay. First things first: i do understand the desire to compile Geany with -mwindows, but Geany developers have to realize that this literally kills *all* stdout and stderr output. So if you want to debug Geany, you need to patch -mwindows out of Makefile.am. Or convince Geany developers to re-initialize stdout and stderr streams on startup when running on Windows. It might not be always obvious what the Right Thing (TM) to do about initializing them is though.

Anyway, once you have the output, one can debug this. I did, with geany-1.27 (because it was ready for building, just laying around in my source directory, not sure if it's the latest version). Long story short, GTK+ is not immediately responsible for Ctrl+C no working with Russian keyboard layout.

Geany has its own keyboard handler, on_key_press_event(), and it loops through all keybindings (again, Geany's own) and invokes the handler if one of the entries match. The problem? Geany matches on event->keyval, which is the GDK keyboard code (that is, it's different for Latin 'c' and Cyrillic 'с' characters). Both events have identical hardware key codes, but Geany isn't looking on these.

I'm not sure what the right thing here is. Matching hardware codes directly is not feasible, as these seem to be hardware-dependent. GDK can do that, because GDK has access to all keyboard layouts and is able to check whether current key event matches in a different layout. Geany can't.

I'm curious how this works on X11. What kind of output does "print *ev" in gdb show for the Ctrl+C and Ctrl+Эс combinations there? Does GDK X11 backend do some kind of conversion?

Now, you might be wondering: wait a second! But Ctrl+C *does* work until the Edit menu is opened! Indeed, it does. Because it happens through a different call chain:

+ Trace 237066

This chain stops working once Geany (or GTK, acting on behalf of Geany) adds menu mnemonics, which, apparently, happens when you open the menu.

Anyway, this is as far as i've gotten today. Is there a Geany bug report open for this?

I don't know how it is done, but despite -mwindows Geany prints output when it is launched from inside MSYS2 console, so it is debuggable without recompilation too. They also have Help > Debug Messages window, but that may be of little help since it just can't handle all the flow from GDK_DEBUG=events.

I have found out what is the difference between Ctrl+V and Ctrl+C here: Ctrl+V doesn't stop working because Edit - Paste (that is bound to Ctrl+V) is always active, whereas Edit - Copy (Ctrl+C) is disabled when nothing is selected. Once the menu is opened and this item becomes grayed out, the bound hotkey stops working in the alternative layout. But if we select something in the textarea and reopen the menu, the menu item for Ctrl+C is active again and the hotkey works too.

The latest bug report is here: https://github.com/geany/geany/issues/1368

Comment on attachment 343501 [details] [review]
GDK W32: Replace wcscpy_s with wcsncpy for XP compatibility

Anyway, i've pushed this attachment into gtk-2-24 branch.

Also, this Geany bug you're experiencing is not gtk2-specific, my Geany was built against gtk3 and it has the same problem.

Hello guys

Thank you, especially LNR, for paying soooo much attention and time to that issue and support of ex-ussr users of Gimp and other GTK based applications. Half a year is devoted to this! 

That’s really cool to see such investigation staff as an example of precise and attentive work focused on details.

(In reply to LRN from comment #50)
> Geany has its own keyboard handler, on_key_press_event(), and it loops
> through all keybindings (again, Geany's own) and invokes the handler if one
> of the entries match. The problem? Geany matches on event->keyval, which is
> the GDK keyboard code (that is, it's different for Latin 'c' and Cyrillic
> 'с' characters). Both events have identical hardware key codes, but Geany
> isn't looking on these.

You've nailed the bug, thank you. So it is not a GTK bug indeed. The complexity with the partially working hotkeys was due to the fact that some hotkeys were bound to GTK menu items, and they worked in any layout, but only as long as the correspondent menu items were enabled, and their state didn't change during the normal workflow, only when the menu was clicked.

While it is still unclear how to correctly handle hotkeys in a GTK application in a layout-independent way, and suggestions are appreciated, the discussion here has come to an end. Just want to say thanks again for this and for the GTK patches you've delivered in this bug.

Had to work on a laptop running Debian for a while, so i had an opportunity to try Geany with GTK using the X11 GDK backend. Well, this answers my question: Ctrl+C in non-latin keyboard layouts (where "C" is not "C") stops working once you open the Edit menu with nothing being selected in the text area.

I'm surprised no one caught this earlier. Maybe just too few people with non-Latin-ish keyboard layouts use Geany. Either way, this is most likely something on Geany side (though GTK might need a documentation improvement once Geany developers figure out what are they doing wrong).

Geany, as you found out earlier, implements custom keybindings handling and does not rely on GTK to do it, and its own hotkey handling doesn't work in non-Latin layout at all. But by a coincidence the bound menu items work automatically, so some of the hotkeys work. But they work only while their menu items are enabled. The Copy (Ctrl+C) menu item is changing its enabled/disabled state on the menu opening depending on the current selection. That brought in confusion and "random" reproducibility.

A Geany developer already agreed that it's on their side (see the latest bug report), but it looks like he isn't using any alternative layout and so is not motivated to fix this at all, saying that redoing it the GIMP way, where GTK fully handles hotkeys, is a lot of work.

And not to say it's unknown bug, multiple articles on how to "fix" this by rebinding hotkeys to the alternative layout can be found on the Runet, just nobody to look into and fix it.

They just have to use https://developer.gnome.org/gtk3/stable/gtk3-Bindings.html , like GtkTextView does.

(In reply to LRN from comment #55)
> though GTK might need a documentation improvement

GTK uses GtkKeyHash internally to match keys independently from a keyboard layout, but its functions are preceded by an underscore and are not present in the GTK Reference Manual.

Hi LRN, I think we're seeing some regressions with Inkscape's shortcuts system after this change. I investigated for some time now and I *think* that I know what is going on, but I hope you can confirm or deny and maybe shed some additional light onto this issue:

At the lowest level Inkscape uses "gdk_keymap_translate_keyboard_state()" with "group" set to "0" in order to convert a received "GdkEventKey" to a uniform representation that works across all keyboard layouts the user has configured (this is in order to support a shortcut use the same physical key in the different layouts that might be in use).

In older gtk versions (2.24.29, 3.19.6) this mostly worked. In current builds (3.22.15) however there are often problems with converting the key depending on the "input methods" the user has set up in Windows.

If I understood the change implemented in this bug correctly, gtk will now use a different group for each keyboard layout - is that true? (and they are mutually exclusive, i.e. will share no keys?)

The issue we're seeing as a result is that "group 0" seems to change depending on the order of the "input methods" (i.e. keyboard layouts) set in control panel. Is there a defined logic how those input methods are assigned to a group in gtk?

And some basic questions I still try to wrap my head around:
* How did older gtk versions handle this on Windows? (where the translation apparently worked as expected)
* How does gtk handle this on Linux (where no issues are known either)
* How do newer gtk versions handle this (where users are know complaining that they can't properly set/use shortcut keys

And some "goodies":
* Is there a method to obtain the available keyboard layouts / groups?
* Can these somehow be parsed/linked to a user-consumable string (or similar) that could be used in a possible application setting?

(In reply to Eduard Braun from comment #59)
> If I understood the change implemented in this bug correctly, gtk will now
> use a different group for each keyboard layout - is that true? (and they are
> mutually exclusive, i.e. will share no keys?)

Yes, group == keyboard layout. Layouts are separate (in a sense that they do not share data), but they obviously can partially match one another (for example, Czech 'q' and US 'q' share the same button on), although from the code's point of view this is irrelevant.

> The issue we're seeing as a result is that "group 0" seems to change
> depending on the order of the "input methods" (i.e. keyboard layouts) set in
> control panel. Is there a defined logic how those input methods are assigned
> to a group in gtk?

The order is the same as the handles returned by GetKeyboardLayoutList().
This order changes when GetKeyboardLayoutList() starts returning something else.

> And some basic questions I still try to wrap my head around:
> * How did older gtk versions handle this on Windows? (where the translation
> apparently worked as expected)

With hacky code. But mostly - they didn't (shortcuts do not work in Windows version of GIMP to this day). Geany tried to handle this on its own, also double-checking against GTK, it kind of worked, but ran into some problems.

> * How does gtk handle this on Linux (where no issues are known either)
> * How do newer gtk versions handle this (where users are know complaining
> that they can't properly set/use shortcut keys

Not entirely sure about X (i might have known at some point, but forgot).
Re-reading my own comments above, it seems that this didn't work on X either (or, rather, it worked, until it didn't), at least not for Geany.

If i'm understanding you correctly, the goal you have is to detect keys, not characters, so that shortcuts work the same in all layouts (i.e. "Undo" is always Ctrl+Z, even if in current layout the 'Z' key produces a different character).
This is achievable by relying on GTK to do shortcut processing for you - it will go through the all the alternative characters produced by the key being pressed, and will trigger the shortcut even if currently-active layout does not produce the character needed for it.


> 
> And some "goodies":
> * Is there a method to obtain the available keyboard layouts / groups?

Not from GDK, but if you are not afraid of using undocumented implementation details, you can call GetKeyboardLayoutList() and assume that GDK uses the same layout order.

> * Can these somehow be parsed/linked to a user-consumable string (or
> similar) that could be used in a possible application setting?

I'm assuming that "these" refers to the list of available layouts mentioned above. Since it can't be obtained from GDK, the answer is, obviously, "no". Also, obviously, if you obtain it from GetKeyboardLayoutList(), then you can do whatever you want (specifically - what GDK does).

It's probably not a good engineering decision to [partially] duplicate GDK code inside your application, but that is still an option, if you can't or won't (for whatever reason) offload that functionality to GTK.

I should probably find some time to write a blogpost (or something) about doing mnemonic/shortcut/hotkey thing correctly in GTK3 (and 2). GTK documentation does suck at times :(

I would say that it is actually the implementation that needs improvement, not the docs.

What I think would be really beneficial for GTK is cross-platform keycodes.

Probably based on this spec, like it is done in browsers:
https://www.w3.org/TR/2015/WD-DOM-Level-3-Events-code-20150428/#code-value-tables

Here is a newer version of it, but it misses USB values, so I decided to link to the previous one in the first place:
https://w3c.github.io/uievents-code/

(In reply to LRN from comment #60)
> If i'm understanding you correctly, the goal you have is to detect keys, not
> characters, so that shortcuts work the same in all layouts (i.e. "Undo" is
> always Ctrl+Z, even if in current layout the 'Z' key produces a different
> character).

Exactly (it works mostly for letter-keys, but for special characters it's often problematic).

> This is achievable by relying on GTK to do shortcut processing for you - it
> will go through the all the alternative characters produced by the key being
> pressed, and will trigger the shortcut even if currently-active layout does
> not produce the character needed for it.

Via Accelerator Groups [1]? If so "gtk_accel_group_connect()" seems to be the central function to create a new shortcut here - it's unclear to me however for which layout this will actually set up the shortcut? For example if I specify "accel_key = GDK_KEY_parenleft" and "accel_mods = GDK_CONTROL_MASK" this should result in different effective shortcuts for different layouts (e.g. I'd have to press "Ctrl + Shift + 9" on in an English layout and "Ctrl + Shift + 8" on a German layout!). Does this mean all configurations work in the end? Or would the shortcut change depending on active layout? Or would the layout active on application launch define behavior? I'm afraid GTK docs are very lacking with respect to such important details...

[1]  https://developer.gnome.org/gtk3/stable/gtk3-Keyboard-Accelerators.html

(In reply to Eduard Braun from comment #62)
> (In reply to LRN from comment #60)
> > This is achievable by relying on GTK to do shortcut processing for you - it
> > will go through the all the alternative characters produced by the key being
> > pressed, and will trigger the shortcut even if currently-active layout does
> > not produce the character needed for it.
> 
> Via Accelerator Groups [1]? If so "gtk_accel_group_connect()" seems to be
> the central function to create a new shortcut here - it's unclear to me
> however for which layout this will actually set up the shortcut?
AFAIU, the lookup in gtk_window_activate_key() is performed using the hardware keycode, so all binds are to the key, not the character it produces. According to _gtk_key_hash_lookup() documentation, it will return either only exact matches (keyval and hardware keycode match) or only fuzzy matches (keyval is different, but hardware keycode and level match).

Accelerator groups look like the right API. A bit inconvenient - in GTK3 you don't have to use these, just call gtk_application_set_accels_for_action(); GTK3 seems to be big on using actions (GAction), unlike GTK2.

> For example
> if I specify "accel_key = GDK_KEY_parenleft" and "accel_mods =
> GDK_CONTROL_MASK" this should result in different effective shortcuts for
> different layouts (e.g. I'd have to press "Ctrl + Shift + 9" on in an
> English layout and "Ctrl + Shift + 8" on a German layout!). Does this mean
> all configurations work in the end? Or would the shortcut change depending
> on active layout? Or would the layout active on application launch define
> behavior? I'm afraid GTK docs are very lacking with respect to such
> important details...

AFAIU, GTK will find all the keymap entries for the keyval 9 (keymap entries have hardware keycodes for each key in each group (layout) that is able to produce the keyval 9), and will store GtkKeyHashEntry (keyval, modifiers, some metadata, and a list of all keymap entries that produce its keyval) in a hash map, using this hardware keycode as the hash-key.

Then, when you press '9', it will use its hardware keycode to look up the GtkKeyHashEntry, and compare the keyval.
If keyval matches, then this is an exact match (the same physical key produces the same keyval), and that is what will be returned.
Otherwise it will also check all keymap entries previously stored in GtkKeyHashEnry, and check that the hardware keycode and the level match. If they do (same physical key produces a different character in a different group, but uses the same modifiers - that is, the user pressed exactly the same thing, just in a wrong layout), then this is a fuzzy match, and that will also be returned (unless the "wrong" keyval produced by these seemingly-matching physical keys is also bound by a widget up the stack).

So - no, it won't trigger on Ctrl+Shift+8, because '8' has a different hardware keycode - the very first hashtable lookup will come up empty.
It will trigger on Ctrl+Shift+9 even in German layout, UNLESS a widget up the stack specifically bound Ctrl+Shift+9 in German layout.

Created attachment 354198 [details]
gtk-keyboard-sample (gtk2) v1.0

Here's something i cobbled together. All 4 shortcuts + 1 mnemonic work in any layout.
Not sure how idiomatic the code is though.

Thanks LRN for your comments!

Your example works as I suspected though:
The shortcuts are translated to the active keyboard layout, i.e. I have to press different keys depending on keyboard layout). (Try for example "GDK_KEY_plus" or "GDK_KEY_backslash")

If I use the same physical keys working in one layout while another layout is active there's no reaction at all...




One other thing I noticed (is this correct?):
Hardware keycodes also change with active layout for one and the same physical key! Aren't hardware keycodes supposed to actually represent a physical key regardless of active keyboard layout?

For example with the sample program attached and pressing the "+" key on my German keyboard (then switching to Swiss and finally Russian layout):

  $ g++ `pkg-config --cflags gtk+-3.0` -o shortcuts.exe shortcuts.c `pkg-config --libs gtk+-3.0` && ./shortcuts.exe
  key: plus, keycode: 43
  -> hardware_keycode: 187, group: 0, state: 0 (original event)
  key: dead_diaeresis, keycode: 65111
  -> hardware_keycode: 192, group: 1, state: 0 (original event)
  key: Cyrillic_hardsign, keycode: 1759
  -> hardware_keycode: 221, group: 2, state: 0 (original event)

Created attachment 354207 [details]
sample program for comment 65

Looks like i was mistaken. This indeed works as you said.

At first i thought that my explanation was essentially correct, but that it the situation was screwed by GDK using virtual key codes (instead of scancodes) as "hardware keycodes" on Windows. So i've tried to remedy that.

Then it turned out that this doesn't work even with scancodes. The last part of _gtk_key_hash_lookup() specifically derails this by checking that the "wrong" keyval being reacted to is part of the group. If it is, it bails out and returns NULL.

This means that "Undo" works as Ctrl+Z in US layout, as Ctrl+Z in RU layout (because it's impossible to type 'z' in RU layout, so the last test passes; and the initial test passes because virtual keycodes are positionally the same between US and RU layouts), but only as Ctrl+Y in DE layout (because DE layout does have Z character typed by the key that is 'Y' in US layout, so even if i fudge the code to pass through the first lookup by using scancodes (which do remain the same) instead of virtual keycodes, the last check notes that DE layout is capable of producing 'Z' normally, and bails).

After that it dawned on me that i should check to see how other applications work.
Turns out, Firefox and Notepad work exactly the same way - Ctrl+Z in US and RU, Ctrl+Y in DE.

This is unexpectedly bad. I wonder, how shortcut i18n works in these applications then...Maybe they just explicitly define different shortcut maps for different UI languages...?

Tomorrow i'll check to see how Gnome is dealing with it.

Works the same way in Gnome. It appears that this is a feature, not a bug.

> It appears that this is a feature, not a bug.

Well, it makes sense as long as you don't expect the user to switch keyboard layouts...
Current behavior of Inkscape is similar: Set up shortcuts from a shortcuts file and translate the keys to the users "group 0" keyboard layout. That is done at startup - if the user switches to a different keyboard layout later physical keys should not change, though.


> Works the same way in Gnome.

Thanks for testing!

Could you also cross-check my observations from comment 65 (changing hardware keycodes for the same physical key)? Because I'm afraid this might be a bug (if so I can report it separately) but I know not enough about how it's supposed to work and the internals to make that judgment.

If you mean on Windows - that's an design decision (the use of virtual key codes instead of scancodes; virtual key codes change depending on the layout, as you have seen). We can't roll it back. It's possible to work around it (and i did, in my local version of GTK2), but it does not override the overall behaviour, so i'm not seeing any reason to push with this.

187 = VK_OEM_PLUS
192 = VK_OEM_3
221 = VK_OEM_6

> If you mean on Windows - that's an design decision

Is there *any* way to determine which physical key a user pressed on his/her keyboard then from gtk?
Because it sounds to me as if you had to live with virtual keycodes being "arbitrary" then (depending on keyboard layout and using some "gtk-internal" magic).

Sure, in GTK3 there's bug 765259. This was not backported to GTK2, but doing so is trivial (i know, i did).

Thanks!

I'm afraid a custom patched version is not really an option but fortunately Inkscape will be gtk3-only for newer versions anyway...

If I may ask (sorry If it was mentioned before and I didn't get it):
Was the design decision to use virtual key codes that change depending on the layout really necessary? To me it sounds a bit like a broken design if it makes such ugly workarounds like "gdk_event_get_scancode" necessary that basically duplicate functionality (getting a hardware keycode) and does even explicitly mention that it returns hardware_keycode on all OSs except Windows...
I mean: Wouldn't it be possible to create virtual keys that stay constant across layouts, so Linux-like behavior is achieved also on Windows?

(In reply to Eduard Braun from comment #73)
> If I may ask (sorry If it was mentioned before and I didn't get it):
> Was the design decision to use virtual key codes that change depending on
> the layout really necessary? To me it sounds a bit like a broken design if
> it makes such ugly workarounds like "gdk_event_get_scancode" necessary that
> basically duplicate functionality (getting a hardware keycode) and does even
> explicitly mention that it returns hardware_keycode on all OSs except
> Windows...
> I mean: Wouldn't it be possible to create virtual keys that stay constant
> across layouts, so Linux-like behavior is achieved also on Windows?

I was not there when the decision was made. I suspect that whoever made the decision just went along with the stuff that Windows usually delivered (VK codes) in its messages, thinking that it correlates directly to the hardware keycodes that X events have (VK codes are clearly not keyvals, as there's only, like, 255 of them or so; therefore they must be hardware keycodes!). Also, scancodes are probably not very useful by themselves (AFAIU, scancode -> VK code match is made by the device driver). I've seen scancode tables, but i have no idea how close they are to reality. VK codes, on the other hand, are more or less well-defined semantically.

If you wish, you can argue to make this transition in GTK4 :)