I'm confused. That file hasn't changed in many years, it used to work fine in 2016, and it works just fine now on my machine. How did it break?

More to the topic, we already handle a few not-quite-accelerators in OnKeyPress or a related event; Tab could go there too.

Actually, we already do that in

You're going to have to figure out why that code path is never reached, since Tab works fine for me locally.

Nothing happens. This is due to Gtk not considering Tab to to be a valid accelerator:

• OS: Arch Linux, X11,
• GTK: 3.24

JFTR, Strange, because toggling "Property Browser" via Tab key works for me:

• OS: MX Linux MX-19.1 (Debian 10 buster), Xfce4
• GTK: 3.24.5

You're going to have to figure out why that code path is never reached, since Tab works fine for me locally.

Good to know that there's another code path.

On my system apparently, state is GDK_MOD2_MASK even with no modifier pressed. Since the above code exits early if a modifier other than shift or ctrl is present GraphicsWindow::KeyboardEvent never gets called. Removing this check makes tab behave as intended.

On my system apparently, state is GDK_MOD2_MASK even with no modifier pressed.

Why does this happen?

Why does this happen?

On my system GDK_MOD2_MASK is set if Num Lock is on. If num lock is off the existing code works as intended.

As far as I can tell we can therefore safely ignore MOD2.

Does this help?

diff --git a/src/platform/guigtk.cpp b/src/platform/guigtk.cpp
index e68fd304..461ba60b 100644
--- a/src/platform/guigtk.cpp
+++ b/src/platform/guigtk.cpp
@@ -565,7 +565,7 @@ protected:
         KeyboardEvent event = {};
         event.type = type;
 
-        if(gdk_event->state & ~(GDK_SHIFT_MASK|GDK_CONTROL_MASK)) {
+        if(gdk_event->state & GDK_MOD1_MASK) {
             return false;
         }
 

Sorry, the earlier comment had a typo--I meant to use GDK_MOD1_MASK (fixed now).

Depends on what we want to achieve:

• Ignore MOD1 (Alt) specifically
• Ignore all unknown modifiers such as super

From my point of view, we want the latter option since Super-tab shouldn't trigger our accelerator:

diff --git a/src/platform/guigtk.cpp b/src/platform/guigtk.cpp
index e68fd30..fe31466 100644
--- a/src/platform/guigtk.cpp
+++ b/src/platform/guigtk.cpp
@@ -565,7 +565,7 @@ protected:
         KeyboardEvent event = {};
         event.type = type;
 
-        if(gdk_event->state & ~(GDK_SHIFT_MASK|GDK_CONTROL_MASK)) {
+        if(gdk_event->state & ~(GDK_SHIFT_MASK|GDK_CONTROL_MASK|GDK_MOD2_MASK)) {
             return false;
         }

What other bugs remain in that code? Is there the same issue with ScrollLock and CapsLock?

I had a closer look at the Gtk docs and I think I found what we need:

diff --git a/src/platform/guigtk.cpp b/src/platform/guigtk.cpp
index e68fd30..7ff0478 100644
--- a/src/platform/guigtk.cpp
+++ b/src/platform/guigtk.cpp
@@ -565,7 +565,8 @@ protected:
         KeyboardEvent event = {};
         event.type = type;
 
-        if(gdk_event->state & ~(GDK_SHIFT_MASK|GDK_CONTROL_MASK)) {
+        const auto mod_mask = get_modifier_mask(Gdk::MODIFIER_INTENT_DEFAULT_MOD_MASK);
+        if((gdk_event->state & mod_mask) & ~(GDK_SHIFT_MASK|GDK_CONTROL_MASK)) {
             return false;
         }

https://developer.gnome.org/gdk3/stable/gdk3-Windows.html#GdkModifierIntent

The set of modifier masks accepted as modifiers in accelerators. Needed because Command is mapped to MOD2 on OSX, which is widely used, but on X11 MOD2 is NumLock and using that for a mod key is problematic at best.

Thanks!