revert "xkbcommon: make shortcuts persistent across layouts"

This reverts commit 5e76a9569e37e6620a7ddf3a9d9620fbb9b9d04f.

The change's stated goal was to make shortcuts "stable",
i.e. work the same, regardless of which keyboard layout
the user has selected.

In doing so, it changed the semantics of shortcut handling
to depend on the order of the keyboard layouts reflected
by XKB, picking the first Latin layout in the list, instead
of prioritizing the currently selected/active keyboard
layout.

This change in semantics is a major behavior change, and
breaks common and valid setups such as having [en,fr] or
[en,de] layouts.

For example, the French layout uses an AZERTY layout,
where the Q and A keys are switched compared to QWERTY.
With the change in place, pressing the physical A key
on a French keyboard, with Control pressed, no longer
selects all text, but instead quits the application,
as the shortcut is interpreted based on the English
layout, which just happens to be first in the list.

Similar issues exist for German layouts, which use
QWERTZ, or more complex layouts such as the Neo layout.

The semantics of prioritizing the order of declared
layouts instead of the active one is inconsistent with
both macOS and Windows, as well as other toolkits on
Linux, including GTK and earlier versions of Qt.

It's also not discoverable by the user that the order
now matters. For example, there is no UX in the Gnome
setting that tells the user to ensure the order matches
their expectations for shortcut handling. And if there
was, this would only apply to Qt apps built with 6.6.0,
creating inconsistent behavior for users.

Worse, the X server is limited to four concurrent keyboard
layouts (groups), so if the user adds more layouts than
that, Gnome will replace the X server's view of layouts
only when switching to a layout beyond the first four.
And in that case, the X server's view of the layouts is
actually starting with the fifth layout declared in the
Gnome preferences. The logic in the reverted patch does
not take this into account, making it confusing for the
user which layout actually takes precedence.

Note that reverting this change does not affect our
fallback logic for layouts that do not produce Latin
symbols for the given key press, such as Greek or Russian.
Those layouts will continue to fall back to a Latin
layout for their QKeyEvent::key().

[ChangeLog][QtGui][X11/Wayland] A change in 6.6.0 that
resulted in keyboard shortcuts not respecting the user's
active layout has been reverted.

Pick-to: 6.6
Task-number: QTBUG-108761
Change-Id: Iec2897cd1541c0c125cc5b1078d0beec12b501c0
Reviewed-by: David Edmundson <davidedmundson@kde.org>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Gatis Paeglis <gatis.paeglis@qt.io>
Reviewed-by: Liang Qi <liang.qi@qt.io>

1 files changed, 36 insertions, 3 deletions

diff --git a/src/gui/platform/unix/qxkbcommon.cpp b/src/gui/platform/unix/qxkbcommon.cpp
index 162ba90efe..2fa2ae8911 100644
--- a/src/gui/platform/unix/qxkbcommon.cpp
+++ b/src/gui/platform/unix/qxkbcommon.cpp
@@ -488,9 +488,11 @@ int QXkbCommon::keysymToQtKey(xkb_keysym_t keysym, Qt::KeyboardModifiers modifie
         // With standard shortcuts we should prefer a latin character, this is
         // for checks like "some qkeyevent == QKeySequence::Copy" to work even
         // when using for example 'russian' keyboard layout.
-        xkb_keysym_t latinKeysym = QXkbCommon::lookupLatinKeysym(state, code);
-        if (latinKeysym != XKB_KEY_NoSymbol)
-            keysym = latinKeysym;
+        if (!QXkbCommon::isLatin1(keysym)) {
+            xkb_keysym_t latinKeysym = QXkbCommon::lookupLatinKeysym(state, code);
+            if (latinKeysym != XKB_KEY_NoSymbol)
+                keysym = latinKeysym;
+        }
     }
 
     return keysymToQtKey_internal(keysym, modifiers, state, code, superAsMeta, hyperAsMeta);
@@ -735,9 +737,12 @@ xkb_keysym_t QXkbCommon::lookupLatinKeysym(xkb_state *state, xkb_keycode_t keyco
         return sym;
     xkb_keymap *keymap = xkb_state_get_keymap(state);
     const xkb_layout_index_t layoutCount = xkb_keymap_num_layouts_for_key(keymap, keycode);
+    const xkb_layout_index_t currentLayout = xkb_state_key_get_layout(state, keycode);
     // Look at user layouts in the order in which they are defined in system
     // settings to find a latin keysym.
     for (layout = 0; layout < layoutCount; ++layout) {
+        if (layout == currentLayout)
+            continue;
         const xkb_keysym_t *syms = nullptr;
         xkb_level_index_t level = xkb_state_key_get_level(state, keycode, layout);
         if (xkb_keymap_key_get_syms_by_level(keymap, keycode, layout, level, &syms) != 1)
@@ -748,6 +753,34 @@ xkb_keysym_t QXkbCommon::lookupLatinKeysym(xkb_state *state, xkb_keycode_t keyco
         }
     }
 
+    if (sym == XKB_KEY_NoSymbol)
+        return sym;
+
+    xkb_mod_mask_t latchedMods = xkb_state_serialize_mods(state, XKB_STATE_MODS_LATCHED);
+    xkb_mod_mask_t lockedMods = xkb_state_serialize_mods(state, XKB_STATE_MODS_LOCKED);
+
+    // Check for uniqueness, consider the following setup:
+    // setxkbmap -layout us,ru,us -variant dvorak,, -option 'grp:ctrl_alt_toggle' (set 'ru' as active).
+    // In this setup, the user would expect to trigger a ctrl+q shortcut by pressing ctrl+<physical x key>,
+    // because "US dvorak" is higher up in the layout settings list. This check verifies that an obtained
+    // 'sym' can not be acquired by any other layout higher up in the user's layout list. If it can be acquired
+    // then the obtained key is not unique. This prevents ctrl+<physical q key> from generating a ctrl+q
+    // shortcut in the above described setup. We don't want ctrl+<physical x key> and ctrl+<physical q key> to
+    // generate the same shortcut event in this case.
+    const xkb_keycode_t minKeycode = xkb_keymap_min_keycode(keymap);
+    const xkb_keycode_t maxKeycode = xkb_keymap_max_keycode(keymap);
+    ScopedXKBState queryState(xkb_state_new(keymap));
+    for (xkb_layout_index_t prevLayout = 0; prevLayout < layout; ++prevLayout) {
+        xkb_state_update_mask(queryState.get(), 0, latchedMods, lockedMods, 0, 0, prevLayout);
+        for (xkb_keycode_t code = minKeycode; code < maxKeycode; ++code) {
+            xkb_keysym_t prevSym = xkb_state_key_get_one_sym(queryState.get(), code);
+            if (prevSym == sym) {
+                sym = XKB_KEY_NoSymbol;
+                break;
+            }
+        }
+    }
+
     return sym;
 }