keybindings: Improve consistency during modal states, fix layout

switching.

- Move ime pre-edit workaround from the menu applet to the StEntry
  class so it can benefit all users of the class.
- Remove another old keybinding workaround from the menu applet.
  (8a0d2d9)
- Make non-XKB-option layout-switching usable in all Action modes
- Handle the RunDialog's super-to-cancel on key-release instead of
  key-press.
- Have Main._stageEventHandler trigger modifier-only keybindings on
  key-release instead of -press.

The last item ends up recreating modifier-only keybinding handling
that currently exists in muffin's keybindings.c.

The most important part is that it identifies a modifier as the
keyval on key-press but only stores the keybinding action if there's
a match.

If there's a subsequent key-press of a normal key, that keybinding
is triggered if one exists. If the next event is a release, the
stored action is triggered instead.

Author: mtwebster

files/usr/share/cinnamon/applets/menu@cinnamon.org/applet.js

@@ -1833,19 +1833,6 @@ class CinnamonMenuApplet extends Applet.TextIconApplet {
             }
         }
 
-        /* check for a keybinding and quit early, otherwise we get a double hit
-           of the keybinding callback */
-        let action = global.display.get_keybinding_action(keyCode, modifierState);
-
-        if (action == Meta.KeyBindingAction.CUSTOM) {
-            return Clutter.EVENT_STOP;
-        }
-
-        if (this.searchEntryText.has_preedit()) {
-            // There is an uncommitted text in the search box, let the input method to handle this.
-            return Clutter.EVENT_PROPAGATE;
-        }
-
         let ctrlKey = modifierState & Clutter.ModifierType.CONTROL_MASK;
 
         // If a context menu is open, hijack keyboard navigation and concentrate on the context menu.

js/ui/keyboardManager.js

@@ -530,14 +530,30 @@ var InputSourceManager = class {
 
     _setupKeybindings() {
         let kb = this._kb_settings.get_strv("switch-input-source");
-        Main.keybindingManager.addHotKeyArray("switch-input-source", kb, () => this._modifiersSwitcher(false));
+        Main.keybindingManager.addHotKeyArray(
+            "switch-input-source", kb,
+            () => this._modifiersSwitcher(false),
+            undefined,
+            Cinnamon.ActionMode.ALL
+        );
+
         kb = this._kb_settings.get_strv("switch-input-source-backward");
-        Main.keybindingManager.addHotKeyArray("switch-input-source-backward", kb, () => this._modifiersSwitcher(true));
+        Main.keybindingManager.addHotKeyArray(
+            "switch-input-source-backward", kb,
+            () => this._modifiersSwitcher(true),
+            undefined,
+            Cinnamon.ActionMode.ALL
+        );
 
         for (let i = 0; i <= 3; i++) {
             const name = `switch-input-source-${i}`;
             kb = this._kb_settings.get_strv(name);
-            Main.keybindingManager.addHotKeyArray(name, kb, () => this.activateInputSourceIndex(i));
+            Main.keybindingManager.addHotKeyArray(
+                name, kb,
+                () => this.activateInputSourceIndex(i),
+                undefined,
+                Cinnamon.ActionMode.ALL
+            );
         }
     }
 

js/ui/main.js

@@ -1278,33 +1278,91 @@ function _shouldFilterKeybinding(entry) {
     return !allowed;
 }
 
-// This function encapsulates hacks to make certain global keybindings
-// work even when we are in one of our modes where global keybindings
-// are disabled with a global grab. (When there is a global grab, then
-// all key events will be delivered to the stage, so ::captured-event
-// on the stage can be used for global keybindings.)
+/* This mimics some of the behavior in muffin's keybindings.c, to allow multi-key keybindings
+ * to work properly while pushModal is active and our input events are bypassing muffin's normal
+ * event handling.
+ *
+ * In normal input modes, single key (modifier) keybindings are activated on key-release, and multi-
+ * key bindings are activated on key-press.
+ *
+ * This needs to work in our pushModal state also, for things like layout-switching. We check the
+ * keyval of the event - if it's a modifier key it can potentially be a single-key binding or part
+ * of a multi-key binding, so we defer activating any single-key modifier bindings until key-release,
+ * to give the multi-key binding a chance to activate on key-press.
+ */
+let _modifierOnlyAction = 0;
+
+function _isModifierKeyval(symbol) {
+    return symbol === Clutter.KEY_Super_L   || symbol === Clutter.KEY_Super_R   ||
+           symbol === Clutter.KEY_Control_L || symbol === Clutter.KEY_Control_R ||
+           symbol === Clutter.KEY_Alt_L     || symbol === Clutter.KEY_Alt_R     ||
+           symbol === Clutter.KEY_Shift_L   || symbol === Clutter.KEY_Shift_R;
+}
+
 function _stageEventHandler(actor, event) {
     if (modalCount == 0)
         return false;
 
-    if (event.type() != Clutter.EventType.KEY_PRESS) {
-        if(!popup_rendering_actor || event.type() != Clutter.EventType.BUTTON_RELEASE)
+    let eventType = event.type();
+
+    if (eventType !== Clutter.EventType.KEY_PRESS &&
+        eventType !== Clutter.EventType.KEY_RELEASE) {
+        if (!popup_rendering_actor || eventType !== Clutter.EventType.BUTTON_RELEASE)
             return false;
         return (event.get_source() && popup_rendering_actor.contains(event.get_source()));
     }
 
+    if (event.get_source() instanceof Clutter.Text &&
+        (event.get_flags() & Clutter.EventFlags.INPUT_METHOD)) {
+        return false;
+    }
+
     let keyCode = event.get_key_code();
     let modifierState = Cinnamon.get_event_state(event);
 
-    let action = global.display.get_keybinding_action(keyCode, modifierState);
-    if (!(event.get_source() instanceof Clutter.Text && (event.get_flags() & Clutter.EventFlags.INPUT_METHOD))) {
+    if (eventType === Clutter.EventType.KEY_PRESS) {
+        if (_isModifierKeyval(event.get_key_symbol())) {
+            let action = global.display.get_keybinding_action(keyCode, modifierState);
+            if (action > 0) {
+                let entry = keybindingManager.getBindingById(action);
+                if (!_shouldFilterKeybinding(entry)) {
+                    _modifierOnlyAction = action;
+                    return true;
+                }
+            }
+            return false;
+        }
+
+        // A non-modifier key was pressed while a modifier was held.
+        // Use -1 to indicate the modifier release should be consumed.
+        _modifierOnlyAction = -1;
+
+        let action = global.display.get_keybinding_action(keyCode, modifierState);
         if (action > 0) {
             let entry = keybindingManager.getBindingById(action);
             if (!_shouldFilterKeybinding(entry)) {
                 keybindingManager.invoke_keybinding_action_by_id(action);
                 return true;
             }
         }
+
+        return false;
+    }
+
+    // Release event - activate the single-key keybinding, or eat the release if
+    // a multi-key combo was used.
+    if (_isModifierKeyval(event.get_key_symbol())) {
+        if (_modifierOnlyAction > 0) {
+            let action = _modifierOnlyAction;
+            _modifierOnlyAction = 0;
+            keybindingManager.invoke_keybinding_action_by_id(action);
+            return true;
+        }
+
+        if (_modifierOnlyAction === -1) {
+            _modifierOnlyAction = 0;
+            return true;
+        }
     }
 
     return false;
@@ -1319,6 +1377,8 @@ function _findModal(actor) {
 }
 
 function _completeModalSetup(actor, mode) {
+    _modifierOnlyAction = 0;
+
     if (modalCount == 0)
         Meta.disable_unredirect_for_display(global.display);
 

js/ui/runDialog.js

@@ -187,6 +187,14 @@ class RunDialog extends ModalDialog.ModalDialog {
         this._commandError = false;
 
         this._entryText.connect('key-press-event', this._onKeyPress.bind(this));
+        this._entryText.connect('key-release-event', (o, e) => {
+            let symbol = e.get_key_symbol();
+            if (symbol === Clutter.KEY_Super_L || symbol === Clutter.KEY_Super_R) {
+                this.close();
+                return Clutter.EVENT_STOP;
+            }
+            return Clutter.EVENT_PROPAGATE;
+        });
 
         this._history = new History.HistoryManager({
             gsettingsKey: HISTORY_KEY,
@@ -230,7 +238,7 @@ class RunDialog extends ModalDialog.ModalDialog {
             }
             return true;
         }
-        if (symbol === Clutter.KEY_Escape || symbol === Clutter.KEY_Super_L || symbol === Clutter.KEY_Super_R) {
+        if (symbol === Clutter.KEY_Escape) {
             this.close();
             return true;
         }

src/st/st-entry.c

@@ -914,6 +914,31 @@ st_entry_key_press_event (ClutterActor    *actor,
   return CLUTTER_ACTOR_CLASS (st_entry_parent_class)->key_press_event (actor, event);
 }
 
+static gboolean
+st_entry_captured_event (ClutterActor *actor,
+                         ClutterEvent *event)
+{
+  StEntryPrivate *priv = ST_ENTRY_PRIV (actor);
+
+  if (clutter_event_type (event) != CLUTTER_KEY_PRESS)
+    return CLUTTER_EVENT_PROPAGATE;
+
+  // Instead of having to check this everywhere an StEntry is used, filter
+  // the event before it gets to the normal handlers.
+  //
+  // Original PR: https://github.com/linuxmint/cinnamon/pull/13317
+  // ref: https://github.com/linuxmint/mint22.3-beta/issues/60
+
+  if (clutter_text_has_preedit (CLUTTER_TEXT (priv->entry)))
+    {
+      CLUTTER_ACTOR_GET_CLASS (priv->entry)->key_press_event (priv->entry,
+                                                              (ClutterKeyEvent *) event);
+      return CLUTTER_EVENT_STOP;
+    }
+
+  return CLUTTER_EVENT_PROPAGATE;
+}
+
 static void
 st_entry_key_focus_in (ClutterActor *actor)
 {
@@ -997,6 +1022,7 @@ st_entry_class_init (StEntryClass *klass)
   actor_class->allocate = st_entry_allocate;
   actor_class->paint = st_entry_paint;
 
+  actor_class->captured_event = st_entry_captured_event;
   actor_class->key_press_event = st_entry_key_press_event;
   actor_class->key_focus_in = st_entry_key_focus_in;