[Feature] Send a dummy keycode to neutralize flashing modifiers in retro tap and key overrides (#20992)

docs/feature_key_overrides.md

@@ -225,3 +225,30 @@ The duration of the key repeat delay is controlled with the `KEY_OVERRIDE_REPEAT
 ## Difference to Combos :id=difference-to-combos
 
 Note that key overrides are very different from [combos](https://docs.qmk.fm/#/feature_combo). Combos require that you press down several keys almost _at the same time_ and can work with any combination of non-modifier keys. Key overrides work like keyboard shortcuts (e.g. `ctrl` + `z`): They take combinations of _multiple_ modifiers and _one_ non-modifier key to then perform some custom action. Key overrides are implemented with much care to behave just like normal keyboard shortcuts would in regards to the order of pressed keys, timing, and interacton with other pressed keys. There are a number of optional settings that can be used to really fine-tune the behavior of each key override as well. Using key overrides also does not delay key input for regular key presses, which inherently happens in combos and may be undesirable.
+
+## Solution to the problem of flashing modifiers :id=neutralize-flashing-modifiers
+
+If the programs you use bind an action to taps of modifier keys (e.g. tapping left GUI to bring up the applications menu or tapping left Alt to focus the menu bar), you may find that using key overrides with suppressed mods falsely triggers those actions. To counteract this, you can define a `DUMMY_MOD_NEUTRALIZER_KEYCODE` in `config.h` that will get sent in between the register and unregister events of a suppressed modifier. That way, the programs on your computer will no longer interpret the mod suppression induced by key overrides as a lone tap of a modifier key and will thus not falsely trigger the undesired action.
+
+Naturally, for this technique to be effective, you must choose a `DUMMY_MOD_NEUTRALIZER_KEYCODE` for which no keyboard shortcuts are bound to. Recommended values are: `KC_RIGHT_CTRL` or `KC_F18`. 
+Please note that `DUMMY_MOD_NEUTRALIZER_KEYCODE` must be a basic, unmodified, HID keycode so values like `KC_NO`, `KC_TRANSPARENT` or `KC_PIPE` aka `S(KC_BACKSLASH)` are not permitted.
+
+By default, only left Alt and left GUI are neutralized. If you want to change the list of applicable modifier masks, use the following in your `config.h`:
+
+```c
+#define MODS_TO_NEUTRALIZE { <mod_mask_1>, <mod_mask_2>, ... }
+```
+
+Examples:
+
+```c
+#define DUMMY_MOD_NEUTRALIZER_KEYCODE KC_RIGHT_CTRL
+
+// Neutralize left alt and left GUI (Default value)
+#define MODS_TO_NEUTRALIZE { MOD_BIT(KC_LEFT_ALT), MOD_BIT(KC_LEFT_GUI) }
+
+// Neutralize left alt, left GUI, right GUI and left Control+Shift
+#define MODS_TO_NEUTRALIZE { MOD_BIT(KC_LEFT_ALT), MOD_BIT(KC_LEFT_GUI), MOD_BIT(KC_RIGHT_GUI), MOD_BIT(KC_LEFT_CTRL)|MOD_BIT(KC_LEFT_SHIFT) }
+```
+
+!> Do not use `MOD_xxx` constants like `MOD_LSFT` or `MOD_RALT`, since they're 5-bit packed bit-arrays while `MODS_TO_NEUTRALIZE` expects a list of 8-bit packed bit-arrays. Use `MOD_BIT(<kc>)` or `MOD_MASK_xxx` instead.

docs/tap_hold.md

@@ -460,6 +460,31 @@ bool get_retro_tapping(uint16_t keycode, keyrecord_t *record) {
 }
 ```
 
+If the programs you use bind an action to taps of modifier keys (e.g. tapping left GUI to bring up the applications menu or tapping left Alt to focus the menu bar), you may find that using retro-tapping falsely triggers those actions. To counteract this, you can define a `DUMMY_MOD_NEUTRALIZER_KEYCODE` in `config.h` that will get sent in between the register and unregister events of a held mod-tap key. That way, the programs on your computer will no longer interpret the mod suppression induced by retro-tapping as a lone tap of a modifier key and will thus not falsely trigger the undesired action.
+
+Naturally, for this technique to be effective, you must choose a `DUMMY_MOD_NEUTRALIZER_KEYCODE` for which no keyboard shortcuts are bound to. Recommended values are: `KC_RIGHT_CTRL` or `KC_F18`. 
+Please note that `DUMMY_MOD_NEUTRALIZER_KEYCODE` must be a basic, unmodified, HID keycode so values like `KC_NO`, `KC_TRANSPARENT` or `KC_PIPE` aka `S(KC_BACKSLASH)` are not permitted.
+
+By default, only left Alt and left GUI are neutralized. If you want to change the list of applicable modifier masks, use the following in your `config.h`:
+
+```c
+#define MODS_TO_NEUTRALIZE { <mod_mask_1>, <mod_mask_2>, ... }
+```
+
+Examples:
+
+```c
+#define DUMMY_MOD_NEUTRALIZER_KEYCODE KC_RIGHT_CTRL
+
+// Neutralize left alt and left GUI (Default value)
+#define MODS_TO_NEUTRALIZE { MOD_BIT(KC_LEFT_ALT), MOD_BIT(KC_LEFT_GUI) }
+
+// Neutralize left alt, left GUI, right GUI and left Control+Shift
+#define MODS_TO_NEUTRALIZE { MOD_BIT(KC_LEFT_ALT), MOD_BIT(KC_LEFT_GUI), MOD_BIT(KC_RIGHT_GUI), MOD_BIT(KC_LEFT_CTRL)|MOD_BIT(KC_LEFT_SHIFT) }
+```
+
+!> Do not use `MOD_xxx` constants like `MOD_LSFT` or `MOD_RALT`, since they're 5-bit packed bit-arrays while `MODS_TO_NEUTRALIZE` expects a list of 8-bit packed bit-arrays. Use `MOD_BIT(<kc>)` or `MOD_MASK_xxx` instead.
+
 ### Retro Shift
 
 [Auto Shift,](feature_auto_shift.md) has its own version of `retro tapping` called `retro shift`. It is extremely similar to `retro tapping`, but holding the key past `AUTO_SHIFT_TIMEOUT` results in the value it sends being shifted. Other configurations also affect it differently; see [here](feature_auto_shift.md#retro-shift) for more information.

quantum/action.c

@@ -528,6 +528,13 @@ void process_action(keyrecord_t *record, action_t action) {
                             unregister_code(action.key.code);
                         } else {
                             ac_dprintf("MODS_TAP: No tap: add_mods\n");
+#    if defined(RETRO_TAPPING) && defined(DUMMY_MOD_NEUTRALIZER_KEYCODE)
+                            // Send a dummy keycode to neutralize flashing modifiers
+                            // if the key was held and then released with no interruptions.
+                            if (retro_tapping_counter == 2) {
+                                neutralize_flashing_modifiers(get_mods());
+                            }
+#    endif
                             unregister_mods(mods);
                         }
                     }

quantum/action_util.c

@@ -500,3 +500,28 @@ __attribute__((weak)) void oneshot_layer_changed_kb(uint8_t layer) {
 uint8_t has_anymod(void) {
     return bitpop(real_mods);
 }
+
+#ifdef DUMMY_MOD_NEUTRALIZER_KEYCODE
+/** \brief Send a dummy keycode in between the register and unregister event of a modifier key, to neutralize the "flashing modifiers" phenomenon.
+ *
+ * \param active_mods 8-bit packed bit-array describing the currently active modifiers (in the format GASCGASC).
+ *
+ * Certain QMK features like  key overrides or retro tap must unregister a previously
+ * registered modifier before sending another keycode but this can trigger undesired
+ * keyboard shortcuts if the clean tap of a single modifier key is bound to an action
+ * on the host OS, as is for example the case for the left GUI key on Windows, which
+ * opens the Start Menu when tapped.
+ */
+void neutralize_flashing_modifiers(uint8_t active_mods) {
+    // In most scenarios, the flashing modifiers phenomenon is a problem
+    // only for a subset of modifier masks.
+    const static uint8_t mods_to_neutralize[] = MODS_TO_NEUTRALIZE;
+    const static uint8_t n_mods               = ARRAY_SIZE(mods_to_neutralize);
+    for (uint8_t i = 0; i < n_mods; ++i) {
+        if (active_mods == mods_to_neutralize[i]) {
+            tap_code(DUMMY_MOD_NEUTRALIZER_KEYCODE);
+            break;
+        }
+    }
+}
+#endif

quantum/action_util.h

@@ -102,6 +102,19 @@ void use_oneshot_swaphands(void);
 void clear_oneshot_swaphands(void);
 #endif
 
+#ifdef DUMMY_MOD_NEUTRALIZER_KEYCODE
+// KC_A is used as the lowerbound instead of QK_BASIC because the range QK_BASIC...KC_A includes
+// internal keycodes like KC_NO and KC_TRANSPARENT which are unsuitable for use with `tap_code(kc)`.
+#    if !(KC_A <= DUMMY_MOD_NEUTRALIZER_KEYCODE && DUMMY_MOD_NEUTRALIZER_KEYCODE <= QK_BASIC_MAX)
+#        error "DUMMY_MOD_NEUTRALIZER_KEYCODE must be a basic, unmodified, HID keycode!"
+#    endif
+void neutralize_flashing_modifiers(uint8_t active_mods);
+#endif
+#ifndef MODS_TO_NEUTRALIZE
+#    define MODS_TO_NEUTRALIZE \
+        { MOD_BIT(KC_LEFT_ALT), MOD_BIT(KC_LEFT_GUI) }
+#endif
+
 #ifdef __cplusplus
 }
 #endif

quantum/process_keycode/process_key_override.c

@@ -322,6 +322,15 @@ static bool try_activating_override(const uint16_t keycode, const uint8_t layer,
 
         clear_active_override(false);
 
+#ifdef DUMMY_MOD_NEUTRALIZER_KEYCODE
+        // Send a dummy keycode before unregistering the modifier(s)
+        // so that suppressing the modifier(s) doesn't falsely get interpreted
+        // by the host OS as a tap of a modifier key.
+        // For example, unintended activations of the start menu on Windows when
+        // using a GUI+<kc> key override with suppressed mods.
+        neutralize_flashing_modifiers(active_mods);
+#endif
+
         active_override                 = override;
         active_override_trigger_is_down = true;
 

tests/tap_hold_configurations/retro_tapping/config.h

@@ -19,3 +19,6 @@
 #include "test_common.h"
 
 #define RETRO_TAPPING
+#define DUMMY_MOD_NEUTRALIZER_KEYCODE KC_RIGHT_CTRL
+#define MODS_TO_NEUTRALIZE \
+    { MOD_BIT(KC_LEFT_GUI) }

tests/tap_hold_configurations/retro_tapping/test_neutralization.cpp

@@ -0,0 +1,201 @@
+/* Copyright 2023 Vladislav Kucheriavykh
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "keyboard_report_util.hpp"
+#include "keycode.h"
+#include "test_common.hpp"
+#include "action_tapping.h"
+#include "test_keymap_key.hpp"
+
+using testing::_;
+using testing::InSequence;
+
+class RetroTapNeutralization : public TestFixture {};
+
+TEST_F(RetroTapNeutralization, neutralize_retro_tapped_left_gui_mod_tap) {
+    TestDriver driver;
+    InSequence s;
+    auto       mod_tap_hold_key = KeymapKey(0, 7, 0, LGUI_T(KC_P));
+
+    set_keymap({mod_tap_hold_key});
+
+    EXPECT_NO_REPORT(driver);
+    mod_tap_hold_key.press();
+    idle_for(TAPPING_TERM);
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_LGUI));
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (DUMMY_MOD_NEUTRALIZER_KEYCODE, KC_LGUI));
+    EXPECT_REPORT(driver, (KC_LGUI));
+    EXPECT_EMPTY_REPORT(driver);
+    EXPECT_REPORT(driver, (KC_P));
+    EXPECT_EMPTY_REPORT(driver);
+    mod_tap_hold_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(RetroTapNeutralization, do_not_neutralize_retro_tapped_left_shift_mod_tap) {
+    TestDriver driver;
+    InSequence s;
+    auto       mod_tap_hold_key = KeymapKey(0, 7, 0, LSFT_T(KC_P));
+
+    set_keymap({mod_tap_hold_key});
+
+    EXPECT_NO_REPORT(driver);
+    mod_tap_hold_key.press();
+    idle_for(TAPPING_TERM);
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_LEFT_SHIFT));
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_EMPTY_REPORT(driver);
+    EXPECT_REPORT(driver, (KC_P));
+    EXPECT_EMPTY_REPORT(driver);
+    mod_tap_hold_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(RetroTapNeutralization, do_not_neutralize_retro_tapped_right_gui_mod_tap) {
+    TestDriver driver;
+    InSequence s;
+    auto       mod_tap_hold_key = KeymapKey(0, 7, 0, RGUI_T(KC_P));
+
+    set_keymap({mod_tap_hold_key});
+
+    EXPECT_NO_REPORT(driver);
+    mod_tap_hold_key.press();
+    idle_for(TAPPING_TERM);
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_RGUI));
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_EMPTY_REPORT(driver);
+    EXPECT_REPORT(driver, (KC_P));
+    EXPECT_EMPTY_REPORT(driver);
+    mod_tap_hold_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(RetroTapNeutralization, do_not_neutralize_retro_tapped_left_gui_shift_mod_tap) {
+    TestDriver driver;
+    InSequence s;
+    auto       mod_tap_hold_key = KeymapKey(0, 7, 0, MT(MOD_LGUI | MOD_LSFT, KC_P));
+
+    set_keymap({mod_tap_hold_key});
+
+    EXPECT_NO_REPORT(driver);
+    mod_tap_hold_key.press();
+    idle_for(TAPPING_TERM);
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_REPORT(driver, (KC_LSFT, KC_LGUI));
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    EXPECT_EMPTY_REPORT(driver);
+    EXPECT_REPORT(driver, (KC_P));
+    EXPECT_EMPTY_REPORT(driver);
+    mod_tap_hold_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(RetroTapNeutralization, do_not_neutralize_roll_of_regular_and_mod_tap_keys) {
+    TestDriver driver;
+    InSequence s;
+    auto       mod_tap_hold_key = KeymapKey(0, 1, 0, LGUI_T(KC_P));
+    auto       regular_key      = KeymapKey(0, 2, 0, KC_A);
+
+    set_keymap({mod_tap_hold_key, regular_key});
+
+    /* Press mod-tap-hold key. */
+    EXPECT_NO_REPORT(driver);
+    mod_tap_hold_key.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    /* Press regular key. */
+    EXPECT_NO_REPORT(driver);
+    regular_key.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    /* Release regular key. */
+    EXPECT_NO_REPORT(driver);
+    regular_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    /* Release mod-tap-hold key. */
+    EXPECT_REPORT(driver, (KC_P));
+    EXPECT_REPORT(driver, (KC_P, KC_A));
+    EXPECT_REPORT(driver, (KC_P));
+    EXPECT_EMPTY_REPORT(driver);
+    mod_tap_hold_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    /* Idle for tapping term of mod tap hold key. */
+    idle_for(TAPPING_TERM - 3);
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(RetroTapNeutralization, do_not_neutralize_tap_regular_key_while_mod_tap_is_held) {
+    TestDriver driver;
+    InSequence s;
+    auto       mod_tap_hold_key = KeymapKey(0, 1, 0, LGUI_T(KC_P));
+    auto       regular_key      = KeymapKey(0, 2, 0, KC_A);
+
+    set_keymap({mod_tap_hold_key, regular_key});
+
+    /* Press and hold mod-tap key. */
+    EXPECT_REPORT(driver, (KC_LEFT_GUI));
+    mod_tap_hold_key.press();
+    idle_for(TAPPING_TERM + 1);
+    VERIFY_AND_CLEAR(driver);
+
+    /* Press regular key. */
+    EXPECT_REPORT(driver, (KC_A, KC_LEFT_GUI));
+    regular_key.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    /* Release regular key. */
+    EXPECT_REPORT(driver, (KC_LEFT_GUI));
+    regular_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    /* Release mod-tap-hold key. */
+    EXPECT_EMPTY_REPORT(driver);
+    mod_tap_hold_key.release();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+
+    /* Idle for tapping term of mod tap hold key. */
+    idle_for(TAPPING_TERM - 3);
+    VERIFY_AND_CLEAR(driver);
+}