From 5a87fa3f92cfc29944b8dc66c2320e3062b532ee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= <mgraesslin@kde.org>
Date: Sun, 22 Jan 2017 09:44:15 +0100
Subject: [PATCH] Support modifier-only-shortcuts when capslock is on

Summary:
So far we didn't trigger modifier-only-shortcuts when capslock was
enabled. In fact we even ensured that the shortcuts did not trigger.
This seems not to be what our users expect. Meta should still trigger
if capslock is on.

This change modifies the logic to determine which modifier is currently
pressed by using the modifiersRelevantForGlobalShortcuts. The difference
to the "normal" modifiers is that this excludes capslock from modifiers
and excludes consumed modifiers. The latter is not really relevant as
modifier-only-shortcuts do not trigger if multiple keys are pressed,
which is required to have a modifier consumed.

BUG: 375355
FIXED-IN: 5.8.6

Test Plan: Only with adjusted autotest

Reviewers: #kwin, #plasma

Subscribers: plasma-devel, kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D4241
---
 .../modifier_only_shortcut_test.cpp           | 22 ++++++++++++-------
 keyboard_input.cpp                            |  6 ++---
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/autotests/integration/modifier_only_shortcut_test.cpp b/autotests/integration/modifier_only_shortcut_test.cpp
index 1b304d86ce..d610c7456f 100644
--- a/autotests/integration/modifier_only_shortcut_test.cpp
+++ b/autotests/integration/modifier_only_shortcut_test.cpp
@@ -251,7 +251,7 @@ void ModifierOnlyShortcutTest::testTrigger()
 void ModifierOnlyShortcutTest::testCapsLock()
 {
     // this test verifies that Capslock does not trigger the shift shortcut
-    // and that the shift modifier on capslock does not trigger either
+    // but other shortcuts still trigger even when Capslock is on
     Target target;
     QSignalSpy triggeredSpy(&target, &Target::shortcutTriggered);
     QVERIFY(triggeredSpy.isValid());
@@ -278,13 +278,13 @@ void ModifierOnlyShortcutTest::testCapsLock()
     QCOMPARE(triggeredSpy.count(), 1);
 
     // currently caps lock is on
-    // shift is ignored
+    // shift still triggers
     kwinApp()->platform()->keyboardKeyPressed(modifier, timestamp++);
     kwinApp()->platform()->keyboardKeyReleased(modifier, timestamp++);
     QCOMPARE(input()->keyboardModifiers(), Qt::ShiftModifier);
-    QCOMPARE(triggeredSpy.count(), 1);
+    QCOMPARE(triggeredSpy.count(), 2);
 
-    // meta on the other hand should trigger
+    // meta should also trigger
     group.writeEntry("Meta", QStringList{s_serviceName, s_path, s_serviceName, QStringLiteral("shortcut")});
     group.writeEntry("Alt", QStringList());
     group.writeEntry("Shift", QStringList{});
@@ -295,15 +295,21 @@ void ModifierOnlyShortcutTest::testCapsLock()
     QCOMPARE(input()->keyboardModifiers(), Qt::ShiftModifier | Qt::MetaModifier);
     QCOMPARE(input()->keyboard()->xkb()->modifiersRelevantForGlobalShortcuts(), Qt::MetaModifier);
     kwinApp()->platform()->keyboardKeyReleased(KEY_LEFTMETA, timestamp++);
-    QEXPECT_FAIL("", "BUG 375355", Continue);
-    QCOMPARE(triggeredSpy.count(), 2);
+    QCOMPARE(triggeredSpy.count(), 3);
+
+    // set back to shift to ensure we don't trigger with capslock
+    group.writeEntry("Meta", QStringList());
+    group.writeEntry("Alt", QStringList());
+    group.writeEntry("Shift", QStringList{s_serviceName, s_path, s_serviceName, QStringLiteral("shortcut")});
+    group.writeEntry("Control", QStringList());
+    group.sync();
+    workspace()->slotReconfigure();
 
     // release caps lock
     kwinApp()->platform()->keyboardKeyPressed(KEY_CAPSLOCK, timestamp++);
     kwinApp()->platform()->keyboardKeyReleased(KEY_CAPSLOCK, timestamp++);
     QCOMPARE(input()->keyboardModifiers(), Qt::NoModifier);
-    QEXPECT_FAIL("", "BUG 375355", Continue);
-    QCOMPARE(triggeredSpy.count(), 2);
+    QCOMPARE(triggeredSpy.count(), 3);
 }
 
 void ModifierOnlyShortcutTest::testGlobalShortcutsDisabled_data()
diff --git a/keyboard_input.cpp b/keyboard_input.cpp
index 9a1c3f4f95..d1866ae4eb 100644
--- a/keyboard_input.cpp
+++ b/keyboard_input.cpp
@@ -286,7 +286,7 @@ void Xkb::updateKey(uint32_t key, InputRedirection::KeyboardKeyState state)
     if (!m_keymap || !m_state) {
         return;
     }
-    const auto oldMods = m_modifiers;
+    const auto oldMods = modifiersRelevantForGlobalShortcuts();
     xkb_state_update_key(m_state, key + 8, static_cast<xkb_key_direction>(state));
     if (state == InputRedirection::KeyboardKeyPressed) {
         const auto sym = toKeysym(key);
@@ -314,14 +314,14 @@ void Xkb::updateKey(uint32_t key, InputRedirection::KeyboardKeyState state)
             !ScreenLockerWatcher::self()->isLocked() &&
             oldMods == Qt::NoModifier &&
             m_input->qtButtonStates() == Qt::NoButton) {
-            m_modOnlyShortcut.modifier = Qt::KeyboardModifier(int(m_modifiers));
+            m_modOnlyShortcut.modifier = Qt::KeyboardModifier(int(modifiersRelevantForGlobalShortcuts()));
         } else {
             m_modOnlyShortcut.modifier = Qt::NoModifier;
         }
     } else {
         m_modOnlyShortcut.pressCount--;
         if (m_modOnlyShortcut.pressCount == 0 &&
-            m_modifiers == Qt::NoModifier &&
+            modifiersRelevantForGlobalShortcuts() == Qt::NoModifier &&
             !workspace()->globalShortcutsDisabled()) {
             if (m_modOnlyShortcut.modifier != Qt::NoModifier) {
                 const auto list = options->modifierOnlyDBusShortcut(m_modOnlyShortcut.modifier);