From ec98f498e8209152533ce06e08b97bf13d456430 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Wed, 17 Aug 2016 08:39:51 +0200 Subject: [PATCH] Ensure modifier locks and latches don't trigger the mod only shortcut Summary: If caps lock is on the shift key should not trigger. Similar pressing caps lock should neither on activation press nor on deactivation press trigger the shortcut. Related to that are latched modifiers aka sticky modifiers: if the modifier is still on after releasing the key the shortcut should not trigger. We must assume the user wanted to use the modifier to activate the modifier, not to activate the shortcut. This change ensures that we don't track for modifier only shortcuts if a modifier is active before press or after release. The added test case demonstrates for caps lock, latched modifiers is currently still untested. (Needs a way to mock it). Test Plan: See test case for caps lock. Reviewers: #kwin, #plasma Subscribers: kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D2467 --- .../modifier_only_shortcut_test.cpp | 44 +++++++++++++++++++ keyboard_input.cpp | 10 ++++- keyboard_input.h | 1 + 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/autotests/integration/modifier_only_shortcut_test.cpp b/autotests/integration/modifier_only_shortcut_test.cpp index b613276f8..f38318551 100644 --- a/autotests/integration/modifier_only_shortcut_test.cpp +++ b/autotests/integration/modifier_only_shortcut_test.cpp @@ -48,6 +48,7 @@ private Q_SLOTS: void testTrigger_data(); void testTrigger(); + void testCapsLock(); }; class Target : public QObject @@ -244,5 +245,48 @@ void ModifierOnlyShortcutTest::testTrigger() QVERIFY(Test::unlockScreen()); } +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 + Target target; + QSignalSpy triggeredSpy(&target, &Target::shortcutTriggered); + QVERIFY(triggeredSpy.isValid()); + + KConfigGroup group = kwinApp()->config()->group("ModifierOnlyShortcuts"); + 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(); + + // first test that the normal shortcut triggers + quint32 timestamp = 1; + const int modifier = KEY_LEFTSHIFT; + kwinApp()->platform()->keyboardKeyPressed(modifier, timestamp++); + kwinApp()->platform()->keyboardKeyReleased(modifier, timestamp++); + QCOMPARE(triggeredSpy.count(), 1); + + // now capslock + kwinApp()->platform()->keyboardKeyPressed(KEY_CAPSLOCK, timestamp++); + kwinApp()->platform()->keyboardKeyReleased(KEY_CAPSLOCK, timestamp++); + QCOMPARE(input()->keyboardModifiers(), Qt::ShiftModifier); + QCOMPARE(triggeredSpy.count(), 1); + + // currently caps lock is on + // shift is ignored + kwinApp()->platform()->keyboardKeyPressed(modifier, timestamp++); + kwinApp()->platform()->keyboardKeyReleased(modifier, timestamp++); + QCOMPARE(input()->keyboardModifiers(), Qt::ShiftModifier); + QCOMPARE(triggeredSpy.count(), 1); + + // release caps lock + kwinApp()->platform()->keyboardKeyPressed(KEY_CAPSLOCK, timestamp++); + kwinApp()->platform()->keyboardKeyReleased(KEY_CAPSLOCK, timestamp++); + QCOMPARE(input()->keyboardModifiers(), Qt::NoModifier); + QCOMPARE(triggeredSpy.count(), 1); +} + WAYLANDTEST_MAIN(ModifierOnlyShortcutTest) #include "modifier_only_shortcut_test.moc" diff --git a/keyboard_input.cpp b/keyboard_input.cpp index 1ce949c2c..e695795fd 100644 --- a/keyboard_input.cpp +++ b/keyboard_input.cpp @@ -85,6 +85,7 @@ Xkb::Xkb(InputRedirection *input) , m_keymap(NULL) , m_state(NULL) , m_shiftModifier(0) + , m_capsModifier(0) , m_controlModifier(0) , m_altModifier(0) , m_metaModifier(0) @@ -191,6 +192,7 @@ void Xkb::updateKeymap(xkb_keymap *keymap) m_state = state; m_shiftModifier = xkb_keymap_mod_get_index(m_keymap, XKB_MOD_NAME_SHIFT); + m_capsModifier = xkb_keymap_mod_get_index(m_keymap, XKB_MOD_NAME_CAPS); m_controlModifier = xkb_keymap_mod_get_index(m_keymap, XKB_MOD_NAME_CTRL); m_altModifier = xkb_keymap_mod_get_index(m_keymap, XKB_MOD_NAME_ALT); m_metaModifier = xkb_keymap_mod_get_index(m_keymap, XKB_MOD_NAME_LOGO); @@ -250,12 +252,14 @@ void Xkb::updateKey(uint32_t key, InputRedirection::KeyboardKeyState state) if (!m_keymap || !m_state) { return; } + const auto oldMods = m_modifiers; xkb_state_update_key(m_state, key + 8, static_cast(state)); updateModifiers(); if (state == InputRedirection::KeyboardKeyPressed) { m_modOnlyShortcut.pressCount++; if (m_modOnlyShortcut.pressCount == 1 && !ScreenLockerWatcher::self()->isLocked() && + oldMods == Qt::NoModifier && m_input->qtButtonStates() == Qt::NoButton) { m_modOnlyShortcut.modifier = Qt::KeyboardModifier(int(m_modifiers)); } else { @@ -263,7 +267,8 @@ void Xkb::updateKey(uint32_t key, InputRedirection::KeyboardKeyState state) } } else { m_modOnlyShortcut.pressCount--; - if (m_modOnlyShortcut.pressCount == 0) { + if (m_modOnlyShortcut.pressCount == 0 && + m_modifiers == Qt::NoModifier) { if (m_modOnlyShortcut.modifier != Qt::NoModifier) { const auto list = options->modifierOnlyDBusShortcut(m_modOnlyShortcut.modifier); if (list.size() >= 4) { @@ -284,7 +289,8 @@ void Xkb::updateKey(uint32_t key, InputRedirection::KeyboardKeyState state) void Xkb::updateModifiers() { Qt::KeyboardModifiers mods = Qt::NoModifier; - if (xkb_state_mod_index_is_active(m_state, m_shiftModifier, XKB_STATE_MODS_EFFECTIVE) == 1) { + if (xkb_state_mod_index_is_active(m_state, m_shiftModifier, XKB_STATE_MODS_EFFECTIVE) == 1 || + xkb_state_mod_index_is_active(m_state, m_capsModifier, XKB_STATE_MODS_EFFECTIVE) == 1) { mods |= Qt::ShiftModifier; } if (xkb_state_mod_index_is_active(m_state, m_altModifier, XKB_STATE_MODS_EFFECTIVE) == 1) { diff --git a/keyboard_input.h b/keyboard_input.h index 1e79e9b10..1609d81fc 100644 --- a/keyboard_input.h +++ b/keyboard_input.h @@ -76,6 +76,7 @@ private: xkb_keymap *m_keymap; xkb_state *m_state; xkb_mod_index_t m_shiftModifier; + xkb_mod_index_t m_capsModifier; xkb_mod_index_t m_controlModifier; xkb_mod_index_t m_altModifier; xkb_mod_index_t m_metaModifier; -- GitLab