Merge lp:~attente/unity-settings-daemon/1226962 into lp:unity-settings-daemon

[William Hua]

* plugins/keyboard/gsd-keyboard-manager.c:
  - Fix shortcuts for non-latin keyboard layouts in certain programs,
    but only when the user's layout switching shortcuts are not
    Shift_L, Shift_R, <Shift>Shift_L, or <Shift>Shift_R (LP: #1226962)

I'm currently maintaining a PPA (https://launchpad.net/~attente/+archive/ubuntu/java-non-latin-shortcuts) for this long-standing annoying bug. The PPA works for probably 99% of cases, however, it could not be merged because it causes a severe regression when the user chooses Shift_L, Shift_R, <Shift>Shift_L, or <Shift>Shift_R as a layout-switching shortcut: the user is unable to type uppercase text. This change contains the behaviour of that PPA, but monitors the switching shortcut to decide whether to do it or not.

[William Hua (attente)]

Simple test case:

- add Greek keyboard layout in unity-control-center Text Entry panel
- launch Inkscape
- switch to Greek using the keyboard indicator
- try some shortcuts
  - Ctrl+Shift+D (normally opens the Document Preferences dialog)
  - Ctrl+Shift+F (normally opens the Fill and Stroke panel)
  - Ctrl+Shift+A (normally opens the Align and Distribute panel)

The shortcuts don't work with the current version of unity-settings-daemon, this patch should fix it.

[Sebastien Bacher (seb128)]

I'm not going to claim to understand the details of thosse keyboard layout and hotkeys but the approach and code look fine to me

review: Approve

[Iain Lane (laney)]

It broke shortcuts in terminator for me, so I reverted this for now - sorry.

=== modified file 'debian/changelog'
--- debian/changelog	2015-12-14 10:02:41 +0000
+++ debian/changelog	2015-12-16 23:50:53 +0000
@@ -1,3 +1,12 @@
+unity-settings-daemon (15.04.1+16.04.20151214-0ubuntu2) UNRELEASED; urgency=medium
+
+  * plugins/keyboard/gsd-keyboard-manager.c:
+    - Fix shortcuts for non-latin keyboard layouts in certain programs,
+      but only when the user's layout switching shortcuts are not
+      Shift_L, Shift_R, <Shift>Shift_L, or <Shift>Shift_R (LP: #1226962)
+
+ -- William Hua <[e-mail]>  Wed, 16 Dec 2015 18:34:53 -0500
+
 unity-settings-daemon (15.04.1+16.04.20151214-0ubuntu1) xenial; urgency=medium
 
   [ David Henningsson ]
 
=== modified file 'plugins/keyboard/gsd-keyboard-manager.c'
--- plugins/keyboard/gsd-keyboard-manager.c	2015-10-28 17:52:16 +0000
+++ plugins/keyboard/gsd-keyboard-manager.c	2015-12-16 23:50:53 +0000
@@ -108,6 +108,11 @@
 #define DEFAULT_LANGUAGE "en_US"
 #define DEFAULT_LAYOUT "us"
 
+#define SWITCH_SETTINGS_DIR "org.gnome.desktop.wm.keybindings"
+
+#define SWITCH_INPUT_SOURCE_KEY          "switch-input-source"
+#define SWITCH_INPUT_SOURCE_BACKWARD_KEY "switch-input-source-backward"
+
 #define GSD_KEYBOARD_DBUS_NAME  "org.gnome.SettingsDaemon.Keyboard"
 #define GSD_KEYBOARD_DBUS_PATH  "/org/gnome/SettingsDaemon/Keyboard"
 
@@ -118,6 +123,7 @@
         GSettings *gsettings;
         GSettings *input_sources_settings;
         GSettings *interface_settings;
+        GSettings *switch_settings;
         GnomeXkbInfo *xkb_info;
         GDBusProxy *localed;
         GCancellable *cancellable;
@@ -138,6 +144,9 @@
         GdkDeviceManager *device_manager;
         guint device_added_id;
         guint device_removed_id;
+        guint n_xkb_layouts;
+        gboolean latin_first;
+        gboolean shift_switch;
 
         GDBusConnection *dbus_connection;
         GDBusNodeInfo *dbus_introspection;
@@ -623,8 +632,8 @@
         XkbSelectEventDetails (dpy,
                                XkbUseCoreKbd,
                                XkbStateNotify,
-                               XkbModifierLockMask,
-                               XkbModifierLockMask);
+                               XkbModifierLockMask | XkbGroupLockMask,
+                               XkbModifierLockMask | XkbGroupLockMask);
 }
 
 static unsigned
@@ -691,6 +700,13 @@
 		}
 	}
 
+	if (manager->priv->latin_first && (xkbev->state.changed & XkbGroupLockMask) && manager->priv->n_xkb_layouts > 0) {
+		Display *display = GDK_DISPLAY_XDISPLAY (gdk_display_get_default ());
+
+		/* Fix the locked group to the last group if it was changed by a grp: option. */
+		XkbLockGroup (display, XkbUseCoreKbd, manager->priv->n_xkb_layouts - 1);
+	}
+
         return GDK_FILTER_CONTINUE;
 }
 
@@ -728,16 +744,20 @@
 static void
 upload_xkb_description (const gchar          *rules_file_path,
                         XkbRF_VarDefsRec     *var_defs,
-                        XkbComponentNamesRec *comp_names)
+                        XkbComponentNamesRec *comp_names,
+                        gint                  n_xkb_layouts,
+                        gboolean              latin_first)
 {
         Display *display = GDK_DISPLAY_XDISPLAY (gdk_display_get_default ());
         XkbDescRec *xkb_desc;
         gchar *rules_file;
 
-        /* The layout we want is always in the first XKB group index
-         * so we should enforce it to make sure we never end up with
-         * the wrong one. */
-        XkbLockGroup (display, XkbUseCoreKbd, 0);
+        if (!latin_first) {
+                /* The layout we want is always in the first XKB group index
+                 * so we should enforce it to make sure we never end up with
+                 * the wrong one. */
+                XkbLockGroup (display, XkbUseCoreKbd, 0);
+        }
 
         /* Upload it to the X server using the same method as setxkbmap */
         xkb_desc = XkbGetKeyboardByName (display,
@@ -758,13 +778,21 @@
         if (!XkbRF_SetNamesProp (display, rules_file, var_defs))
                 g_warning ("Couldn't update the XKB root window property");
 
+        if (latin_first && n_xkb_layouts > 0) {
+                /* The layout we want is always in the last XKB group index
+                 * so we should enforce it to make sure we never end up with
+                 * the wrong one. */
+                XkbLockGroup (display, XkbUseCoreKbd, n_xkb_layouts - 1);
+        }
+
         g_free (rules_file);
 }
 
 static gchar *
 build_xkb_group_string (const gchar *user,
                         const gchar *locale,
-                        const gchar *latin)
+                        const gchar *latin,
+                        gboolean     shift_switch)
 {
         gchar *string;
         gsize length = 0;
@@ -784,14 +812,25 @@
 
         string = malloc (length);
 
-        if (locale && latin)
-                sprintf (string, "%s,%s,%s", user, locale, latin);
-        else if (locale)
-                sprintf (string, "%s,%s", user, locale);
-        else if (latin)
-                sprintf (string, "%s,%s", user, latin);
-        else
-                sprintf (string, "%s", user);
+        if (shift_switch) {
+                if (locale && latin)
+                        sprintf (string, "%s,%s,%s", user, locale, latin);
+                else if (locale)
+                        sprintf (string, "%s,%s", user, locale);
+                else if (latin)
+                        sprintf (string, "%s,%s", user, latin);
+                else
+                        sprintf (string, "%s", user);
+        } else {
+                if (locale && latin)
+                        sprintf (string, "%s,%s,%s", latin, locale, user);
+                else if (locale)
+                        sprintf (string, "%s,%s", locale, user);
+                else if (latin)
+                        sprintf (string, "%s,%s", latin, user);
+                else
+                        sprintf (string, "%s", user);
+        }
 
         return string;
 }
@@ -839,7 +878,7 @@
                                         variant);
 }
 
-static void
+static gint
 replace_layout_and_variant (GsdKeyboardManager *manager,
                             XkbRF_VarDefsRec   *xkb_var_defs,
                             const gchar        *layout,
@@ -855,9 +894,10 @@
         const gchar *latin_variant = "";
         const gchar *locale_layout = NULL;
         const gchar *locale_variant = NULL;
+        gint n_xkb_layouts = 0;
 
         if (!layout)
-                return;
+                return n_xkb_layouts;
 
         if (!variant)
                 variant = "";
@@ -887,10 +927,19 @@
         }
 
         free (xkb_var_defs->layout);
-        xkb_var_defs->layout = build_xkb_group_string (layout, locale_layout, latin_layout);
+        xkb_var_defs->layout = build_xkb_group_string (layout, locale_layout, latin_layout, manager->priv->shift_switch);
 
         free (xkb_var_defs->variant);
-        xkb_var_defs->variant = build_xkb_group_string (variant, locale_variant, latin_variant);
+        xkb_var_defs->variant = build_xkb_group_string (variant, locale_variant, latin_variant, manager->priv->shift_switch);
+
+        n_xkb_layouts = 1;
+
+        if (latin_layout)
+                n_xkb_layouts++;
+        if (locale_layout)
+                n_xkb_layouts++;
+
+        return n_xkb_layouts;
 }
 
 static gchar *
@@ -1018,13 +1067,14 @@
         XkbRF_RulesRec *xkb_rules;
         XkbRF_VarDefsRec *xkb_var_defs;
         gchar *rules_file_path;
+        gint n_xkb_layouts;
 
         gsd_xkb_get_var_defs (&rules_file_path, &xkb_var_defs);
 
         free (xkb_var_defs->options);
         xkb_var_defs->options = options;
 
-        replace_layout_and_variant (manager, xkb_var_defs, layout, variant);
+        n_xkb_layouts = replace_layout_and_variant (manager, xkb_var_defs, layout, variant);
 
         gdk_error_trap_push ();
 
@@ -1034,10 +1084,14 @@
                 xkb_comp_names = g_new0 (XkbComponentNamesRec, 1);
 
                 XkbRF_GetComponents (xkb_rules, xkb_var_defs, xkb_comp_names);
-                upload_xkb_description (rules_file_path, xkb_var_defs, xkb_comp_names);
+                upload_xkb_description (rules_file_path, xkb_var_defs, xkb_comp_names,
+                                        n_xkb_layouts, !manager->priv->shift_switch);
+                manager->priv->latin_first = !manager->priv->shift_switch;
 
                 free_xkb_component_names (xkb_comp_names);
                 XkbRF_Free (xkb_rules, True);
+
+                manager->priv->n_xkb_layouts = n_xkb_layouts;
         } else {
                 g_warning ("Couldn't load XKB rules");
         }
@@ -2393,6 +2447,54 @@
 #endif
 
 static gboolean
+is_shift (const gchar *shortcut)
+{
+        return shortcut && (g_str_equal (shortcut, "Shift_L") || g_str_equal (shortcut, "<Shift>Shift_L")
+                        ||  g_str_equal (shortcut, "Shift_R") || g_str_equal (shortcut, "<Shift>Shift_R"));
+}
+
+static gboolean
+is_shift_switch (GSettings *settings)
+{
+        gboolean shift_switch = FALSE;
+        gchar **shortcuts;
+        gchar **i;
+
+        shortcuts = g_settings_get_strv (settings, SWITCH_INPUT_SOURCE_KEY);
+
+        for (i = shortcuts; !shift_switch && *i; i++)
+                shift_switch = is_shift (*i);
+
+        g_strfreev (shortcuts);
+
+        if (shift_switch)
+                return shift_switch;
+
+        shortcuts = g_settings_get_strv (settings, SWITCH_INPUT_SOURCE_BACKWARD_KEY);
+
+        for (i = shortcuts; !shift_switch && *i; i++)
+                shift_switch = is_shift (*i);
+
+        g_strfreev (shortcuts);
+
+        return shift_switch;
+}
+
+static void
+switch_settings_changed (GSettings *settings,
+                         gchar     *key,
+                         gpointer   user_data)
+{
+        GsdKeyboardManager *manager = user_data;
+        GsdKeyboardManagerPrivate *priv = manager->priv;
+
+        if (is_shift_switch (settings) != priv->shift_switch) {
+                priv->shift_switch = !priv->shift_switch;
+                apply_input_source (manager, priv->active_input_source);
+        }
+}
+
+static gboolean
 start_keyboard_idle_cb (GsdKeyboardManager *manager)
 {
         const gchar *module;
@@ -2419,6 +2521,8 @@
 
         manager->priv->input_sources_settings = g_settings_new (GNOME_DESKTOP_INPUT_SOURCES_DIR);
         manager->priv->interface_settings = g_settings_new (GNOME_DESKTOP_INTERFACE_DIR);
+        manager->priv->switch_settings = g_settings_new (SWITCH_SETTINGS_DIR);
+        manager->priv->shift_switch = is_shift_switch (manager->priv->switch_settings);
         manager->priv->xkb_info = gnome_xkb_info_new ();
 
 #ifdef HAVE_FCITX
@@ -2473,7 +2577,8 @@
                           G_CALLBACK (settings_changed), manager);
         g_signal_connect (G_OBJECT (manager->priv->gsettings), "changed",
                           G_CALLBACK (gsettings_changed), manager);
-
+        g_signal_connect (G_OBJECT (manager->priv->switch_settings), "changed",
+                          G_CALLBACK (switch_settings_changed), manager);
         g_signal_connect (G_OBJECT (manager->priv->input_sources_settings), "change-event",
                           G_CALLBACK (apply_input_sources_settings), manager);
 
@@ -2527,6 +2632,7 @@
         g_clear_object (&p->cancellable);
 
         g_clear_object (&p->settings);
+        g_clear_object (&p->switch_settings);
         g_clear_object (&p->input_sources_settings);
         g_clear_object (&p->interface_settings);
         g_clear_object (&p->xkb_info);
@@ -2577,6 +2683,7 @@
 {
         manager->priv = GSD_KEYBOARD_MANAGER_GET_PRIVATE (manager);
         manager->priv->active_input_source = -1;
+        manager->priv->n_xkb_layouts = -1;
 }
 
 static void