Refine IME switch keybinding related code.

BUG=None
TEST=Manually

Review URL: https://codereview.appspot.com/6295047

3 files changed, 107 insertions, 64 deletions

diff --git a/ui/gtk3/keybindingmanager.vala b/ui/gtk3/keybindingmanager.vala
index dd3c7bdb..d2821ad4 100644
--- a/ui/gtk3/keybindingmanager.vala
+++ b/ui/gtk3/keybindingmanager.vala
@@ -47,17 +47,14 @@ public class KeybindingManager : GLib.Object {
      * Helper class to store keybinding
      */
     private class Keybinding {
-        public Keybinding(string accelerator,
-                          uint keysym,
+        public Keybinding(uint keysym,
                           Gdk.ModifierType modifiers,
                           KeybindingHandlerFunc handler) {
-            this.accelerator = accelerator;
             this.keysym = keysym;
             this.modifiers = modifiers;
             this.handler = handler;
         }
 
-        public string accelerator { get; set; }
         public uint keysym { get; set; }
         public Gdk.ModifierType modifiers { get; set; }
         public unowned KeybindingHandlerFunc handler { get; set; }
@@ -78,18 +75,13 @@ public class KeybindingManager : GLib.Object {
     /**
      * Bind accelerator to given handler
      *
-     * @param accelerator accelerator parsable by Gtk.accelerator_parse
+     * @param keysym
+     * @param modifiers
      * @param handler handler called when given accelerator is pressed
      */
-    public bool bind(string accelerator,
+    public bool bind(uint keysym,
+                     Gdk.ModifierType modifiers,
                      KeybindingHandlerFunc handler) {
-        debug("Binding key " + accelerator);
-
-        // convert accelerator
-        uint keysym;
-        Gdk.ModifierType modifiers;
-        Gtk.accelerator_parse(accelerator, out keysym, out modifiers);
-
         unowned X.Display display = Gdk.x11_get_default_xdisplay();
 
         int keycode = display.keysym_to_keycode(keysym);
@@ -100,27 +92,24 @@ public class KeybindingManager : GLib.Object {
         grab_keycode (Gdk.Display.get_default(), keysym, modifiers);
 
         // store binding
-        Keybinding binding = new Keybinding(accelerator,
-                                            keysym, modifiers,
-                                            handler);
+        Keybinding binding = new Keybinding(keysym, modifiers, handler);
         m_bindings.append(binding);
 
-        debug("Successfully binded key " + accelerator);
         return true;
     }
 
     /**
      * Unbind given accelerator.
      *
-     * @param accelerator accelerator parsable by Gtk.accelerator_parse
+     * @param keysym
+     * @param modifiers
      */
-    public void unbind (string accelerator) {
-        debug("Unbinding key " + accelerator);
-
+    public void unbind(uint keysym,
+                       Gdk.ModifierType modifiers) {
         // unbind all keys with given accelerator
         GLib.List<Keybinding> remove_bindings = new GLib.List<Keybinding>();
         foreach(Keybinding binding in m_bindings) {
-            if(str_equal(accelerator, binding.accelerator)) {
+            if (binding.keysym == keysym && binding.modifiers == modifiers) {
                 ungrab_keycode (Gdk.Display.get_default(),
                                 binding.keysym,
                                 binding.modifiers);
@@ -139,8 +128,8 @@ public class KeybindingManager : GLib.Object {
         return m_instance;
     }
 
-    public static uint get_primary_modifier (uint binding_mask) {
-        const uint[] masks = {
+    public static Gdk.ModifierType get_primary_modifier (uint binding_mask) {
+        const Gdk.ModifierType[] masks = {
             Gdk.ModifierType.MOD5_MASK,
             Gdk.ModifierType.MOD4_MASK,
             Gdk.ModifierType.MOD3_MASK,
@@ -150,7 +139,7 @@ public class KeybindingManager : GLib.Object {
             Gdk.ModifierType.SHIFT_MASK,
             Gdk.ModifierType.LOCK_MASK
         };
-        foreach (uint mask in masks) {
+        foreach (Gdk.ModifierType mask in masks) {
             if ((binding_mask & mask) == mask)
                 return mask;
         }
diff --git a/ui/gtk3/panel.vala b/ui/gtk3/panel.vala
index b494b362..d539dde0 100644
--- a/ui/gtk3/panel.vala
+++ b/ui/gtk3/panel.vala
@@ -42,7 +42,9 @@ class Panel : IBus.PanelService {
     private Gtk.AboutDialog m_about_dialog;
     private Gtk.CssProvider m_css_provider;
     private const string ACCELERATOR_SWITCH_IME_FOREWARD = "<Control>space";
-    private const string ACCELERATOR_SWITCH_IME_BACKWARD = "<Control><Shift>space";
+
+    private uint m_switch_keysym = 0;
+    private Gdk.ModifierType m_switch_modifiers = 0;
 
     public Panel(IBus.Bus bus) {
         GLib.assert(bus.is_connected());
@@ -65,13 +67,7 @@ class Panel : IBus.PanelService {
         m_candidate_panel.page_down.connect((w) => this.page_down());
 
         m_switcher = new Switcher();
-
-        var keybinding_manager = KeybindingManager.get_instance();
-        keybinding_manager.bind(ACCELERATOR_SWITCH_IME_FOREWARD,
-                (e) => handle_engine_switch(e, false));
-
-        keybinding_manager.bind(ACCELERATOR_SWITCH_IME_BACKWARD,
-                (e) => handle_engine_switch(e, true));
+        bind_switch_shortcut();
 
         m_property_manager = new PropertyManager();
         m_property_manager.property_activate.connect((k, s) => {
@@ -82,9 +78,45 @@ class Panel : IBus.PanelService {
     }
 
     ~Panel() {
+        unbind_switch_shortcut();
+    }
+
+    private void bind_switch_shortcut() {
         var keybinding_manager = KeybindingManager.get_instance();
-        keybinding_manager.unbind(ACCELERATOR_SWITCH_IME_FOREWARD);
-        keybinding_manager.unbind(ACCELERATOR_SWITCH_IME_BACKWARD);
+
+        Gtk.accelerator_parse(ACCELERATOR_SWITCH_IME_FOREWARD,
+                out m_switch_keysym, out m_switch_modifiers);
+
+        if (m_switch_keysym == 0 && m_switch_modifiers == 0) {
+            warning("Parse accelerator '%s' failed!",
+                    ACCELERATOR_SWITCH_IME_FOREWARD);
+            return;
+        }
+
+        keybinding_manager.bind(m_switch_keysym, m_switch_modifiers,
+                (e) => handle_engine_switch(e, false));
+
+        // accelerator already has Shift mask
+        if ((m_switch_modifiers & Gdk.ModifierType.SHIFT_MASK) != 0)
+            return;
+
+        keybinding_manager.bind(m_switch_keysym,
+                m_switch_modifiers | Gdk.ModifierType.SHIFT_MASK,
+                (e) => handle_engine_switch(e, true));
+    }
+
+    private void unbind_switch_shortcut() {
+        var keybinding_manager = KeybindingManager.get_instance();
+
+        if (m_switch_keysym == 0 && m_switch_modifiers == 0)
+            return;
+
+        keybinding_manager.unbind(m_switch_keysym, m_switch_modifiers);
+        keybinding_manager.unbind(m_switch_keysym,
+                m_switch_modifiers | Gdk.ModifierType.SHIFT_MASK);
+
+        m_switch_keysym = 0;
+        m_switch_modifiers = 0;
     }
 
     private void set_custom_font() {
@@ -220,7 +252,8 @@ class Panel : IBus.PanelService {
                 event, primary_modifiers);
         if (pressed) {
             int i = revert ? m_engines.length - 1 : 1;
-            i = m_switcher.run(event, m_engines, i);
+            i = m_switcher.run(m_switch_keysym, m_switch_modifiers, event,
+                    m_engines, i);
             if (i < 0) {
                 debug("switch cancelled");
             } else {
diff --git a/ui/gtk3/switcher.vala b/ui/gtk3/switcher.vala
index 1fb04310..b543a8f9 100644
--- a/ui/gtk3/switcher.vala
+++ b/ui/gtk3/switcher.vala
@@ -76,7 +76,9 @@ class Switcher : Gtk.Window {
     private IBusEngineButton[] m_buttons = {};
     private IBus.EngineDesc[] m_engines;
     private uint m_selected_engine;
-    private uint m_primary_modifier;
+    private uint m_keyval;
+    private uint m_modifiers;
+    private Gdk.ModifierType m_primary_modifier;
     private GLib.MainLoop m_loop;
     private int m_result;
 
@@ -111,13 +113,19 @@ class Switcher : Gtk.Window {
         grab_focus();
     }
 
-    public int run(Gdk.Event event, IBus.EngineDesc[] engines, int index) {
+    public int run(uint keyval,
+                   uint state,
+                   Gdk.Event event,
+                   IBus.EngineDesc[] engines,
+                   int index) {
         assert (m_loop == null);
         assert (index < engines.length);
 
+        m_keyval = keyval;
+        m_modifiers = state;
         m_primary_modifier =
             KeybindingManager.get_primary_modifier(
-                event.key.state & KeybindingManager.MODIFIER_FILTER);
+                state & KeybindingManager.MODIFIER_FILTER);
 
         update_engines(engines);
         m_selected_engine = index;
@@ -134,7 +142,8 @@ class Switcher : Gtk.Window {
 #if VALA_0_16
             device = device_manager.list_devices(Gdk.DeviceType.MASTER).data;
 #else
-            unowned GLib.List<Gdk.Device> devices = device_manager.list_devices(Gdk.DeviceType.MASTER);
+            unowned GLib.List<Gdk.Device> devices =
+                    device_manager.list_devices(Gdk.DeviceType.MASTER);
             device = devices.data;
 #endif
         }
@@ -298,34 +307,46 @@ class Switcher : Gtk.Window {
     }
 
     public override bool key_press_event(Gdk.EventKey e) {
+        bool retval = true;
         Gdk.EventKey *pe = &e;
-        switch (pe->keyval) {
-            case 0x0020: /* space */
-            case 0xff80: /* KP_Space */
-                if ((pe->state & Gdk.ModifierType.SHIFT_MASK) == 0)
+
+        do {
+            uint modifiers = KeybindingManager.MODIFIER_FILTER & pe->state;
+
+            if ((modifiers != m_modifiers) &&
+                (modifiers != (m_modifiers | Gdk.ModifierType.SHIFT_MASK))) {
+                break;
+            }
+
+            if (pe->keyval == m_keyval) {
+                if (modifiers == m_modifiers)
                     next_engine();
-                else
+                else // modififers == m_modifiers | SHIFT_MASK
                     previous_engine();
                 break;
-            case 0x08fb: /* leftarrow */
-            case 0xff51: /* Left */
-                previous_engine();
-                break;
-            case 0x08fc: /* uparrow */
-            case 0xff52: /* Up */
-                break;
-            case 0x08fd: /* rightarrow */
-            case 0xff53: /* Right */
-                next_engine();
-                break;
-            case 0x08fe: /* downarrow */
-            case 0xff54: /* Down */
-                break;
-            default:
-                debug("0x%04x", pe->keyval);
-                break;
-        }
-        return true;
+            }
+
+            switch (pe->keyval) {
+                case 0x08fb: /* leftarrow */
+                case 0xff51: /* Left */
+                    previous_engine();
+                    break;
+                case 0x08fc: /* uparrow */
+                case 0xff52: /* Up */
+                    break;
+                case 0x08fd: /* rightarrow */
+                case 0xff53: /* Right */
+                    next_engine();
+                    break;
+                case 0x08fe: /* downarrow */
+                case 0xff54: /* Down */
+                    break;
+                default:
+                    debug("0x%04x", pe->keyval);
+                    break;
+            }
+        } while (false);
+        return retval;
     }
 
     public override bool key_release_event(Gdk.EventKey e) {