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

Proposed by William Hua on 2015-12-16
Status: Merged
Approved by: Sebastien Bacher on 2016-01-07
Approved revision: 4123
Merged at revision: 4125
Proposed branch: lp:~attente/unity-settings-daemon/1226962
Merge into: lp:unity-settings-daemon
Diff against target: 323 lines (+139/-23)
2 files modified
debian/changelog (+9/-0)
plugins/keyboard/gsd-keyboard-manager.c (+130/-23)
To merge this branch: bzr merge lp:~attente/unity-settings-daemon/1226962
Reviewer Review Type Date Requested Status
Sebastien Bacher 2015-12-16 Approve on 2016-01-07
Review via email: mp+280783@code.launchpad.net

Commit message

* 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)

Description of the change

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.

To post a comment you must log in.
William Hua (attente) wrote :

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) wrote :

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) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2015-12-14 10:02:41 +0000
3+++ debian/changelog 2015-12-16 23:50:53 +0000
4@@ -1,3 +1,12 @@
5+unity-settings-daemon (15.04.1+16.04.20151214-0ubuntu2) UNRELEASED; urgency=medium
6+
7+ * plugins/keyboard/gsd-keyboard-manager.c:
8+ - Fix shortcuts for non-latin keyboard layouts in certain programs,
9+ but only when the user's layout switching shortcuts are not
10+ Shift_L, Shift_R, <Shift>Shift_L, or <Shift>Shift_R (LP: #1226962)
11+
12+ -- William Hua <william.hua@canonical.com> Wed, 16 Dec 2015 18:34:53 -0500
13+
14 unity-settings-daemon (15.04.1+16.04.20151214-0ubuntu1) xenial; urgency=medium
15
16 [ David Henningsson ]
17
18=== modified file 'plugins/keyboard/gsd-keyboard-manager.c'
19--- plugins/keyboard/gsd-keyboard-manager.c 2015-10-28 17:52:16 +0000
20+++ plugins/keyboard/gsd-keyboard-manager.c 2015-12-16 23:50:53 +0000
21@@ -108,6 +108,11 @@
22 #define DEFAULT_LANGUAGE "en_US"
23 #define DEFAULT_LAYOUT "us"
24
25+#define SWITCH_SETTINGS_DIR "org.gnome.desktop.wm.keybindings"
26+
27+#define SWITCH_INPUT_SOURCE_KEY "switch-input-source"
28+#define SWITCH_INPUT_SOURCE_BACKWARD_KEY "switch-input-source-backward"
29+
30 #define GSD_KEYBOARD_DBUS_NAME "org.gnome.SettingsDaemon.Keyboard"
31 #define GSD_KEYBOARD_DBUS_PATH "/org/gnome/SettingsDaemon/Keyboard"
32
33@@ -118,6 +123,7 @@
34 GSettings *gsettings;
35 GSettings *input_sources_settings;
36 GSettings *interface_settings;
37+ GSettings *switch_settings;
38 GnomeXkbInfo *xkb_info;
39 GDBusProxy *localed;
40 GCancellable *cancellable;
41@@ -138,6 +144,9 @@
42 GdkDeviceManager *device_manager;
43 guint device_added_id;
44 guint device_removed_id;
45+ guint n_xkb_layouts;
46+ gboolean latin_first;
47+ gboolean shift_switch;
48
49 GDBusConnection *dbus_connection;
50 GDBusNodeInfo *dbus_introspection;
51@@ -623,8 +632,8 @@
52 XkbSelectEventDetails (dpy,
53 XkbUseCoreKbd,
54 XkbStateNotify,
55- XkbModifierLockMask,
56- XkbModifierLockMask);
57+ XkbModifierLockMask | XkbGroupLockMask,
58+ XkbModifierLockMask | XkbGroupLockMask);
59 }
60
61 static unsigned
62@@ -691,6 +700,13 @@
63 }
64 }
65
66+ if (manager->priv->latin_first && (xkbev->state.changed & XkbGroupLockMask) && manager->priv->n_xkb_layouts > 0) {
67+ Display *display = GDK_DISPLAY_XDISPLAY (gdk_display_get_default ());
68+
69+ /* Fix the locked group to the last group if it was changed by a grp: option. */
70+ XkbLockGroup (display, XkbUseCoreKbd, manager->priv->n_xkb_layouts - 1);
71+ }
72+
73 return GDK_FILTER_CONTINUE;
74 }
75
76@@ -728,16 +744,20 @@
77 static void
78 upload_xkb_description (const gchar *rules_file_path,
79 XkbRF_VarDefsRec *var_defs,
80- XkbComponentNamesRec *comp_names)
81+ XkbComponentNamesRec *comp_names,
82+ gint n_xkb_layouts,
83+ gboolean latin_first)
84 {
85 Display *display = GDK_DISPLAY_XDISPLAY (gdk_display_get_default ());
86 XkbDescRec *xkb_desc;
87 gchar *rules_file;
88
89- /* The layout we want is always in the first XKB group index
90- * so we should enforce it to make sure we never end up with
91- * the wrong one. */
92- XkbLockGroup (display, XkbUseCoreKbd, 0);
93+ if (!latin_first) {
94+ /* The layout we want is always in the first XKB group index
95+ * so we should enforce it to make sure we never end up with
96+ * the wrong one. */
97+ XkbLockGroup (display, XkbUseCoreKbd, 0);
98+ }
99
100 /* Upload it to the X server using the same method as setxkbmap */
101 xkb_desc = XkbGetKeyboardByName (display,
102@@ -758,13 +778,21 @@
103 if (!XkbRF_SetNamesProp (display, rules_file, var_defs))
104 g_warning ("Couldn't update the XKB root window property");
105
106+ if (latin_first && n_xkb_layouts > 0) {
107+ /* The layout we want is always in the last XKB group index
108+ * so we should enforce it to make sure we never end up with
109+ * the wrong one. */
110+ XkbLockGroup (display, XkbUseCoreKbd, n_xkb_layouts - 1);
111+ }
112+
113 g_free (rules_file);
114 }
115
116 static gchar *
117 build_xkb_group_string (const gchar *user,
118 const gchar *locale,
119- const gchar *latin)
120+ const gchar *latin,
121+ gboolean shift_switch)
122 {
123 gchar *string;
124 gsize length = 0;
125@@ -784,14 +812,25 @@
126
127 string = malloc (length);
128
129- if (locale && latin)
130- sprintf (string, "%s,%s,%s", user, locale, latin);
131- else if (locale)
132- sprintf (string, "%s,%s", user, locale);
133- else if (latin)
134- sprintf (string, "%s,%s", user, latin);
135- else
136- sprintf (string, "%s", user);
137+ if (shift_switch) {
138+ if (locale && latin)
139+ sprintf (string, "%s,%s,%s", user, locale, latin);
140+ else if (locale)
141+ sprintf (string, "%s,%s", user, locale);
142+ else if (latin)
143+ sprintf (string, "%s,%s", user, latin);
144+ else
145+ sprintf (string, "%s", user);
146+ } else {
147+ if (locale && latin)
148+ sprintf (string, "%s,%s,%s", latin, locale, user);
149+ else if (locale)
150+ sprintf (string, "%s,%s", locale, user);
151+ else if (latin)
152+ sprintf (string, "%s,%s", latin, user);
153+ else
154+ sprintf (string, "%s", user);
155+ }
156
157 return string;
158 }
159@@ -839,7 +878,7 @@
160 variant);
161 }
162
163-static void
164+static gint
165 replace_layout_and_variant (GsdKeyboardManager *manager,
166 XkbRF_VarDefsRec *xkb_var_defs,
167 const gchar *layout,
168@@ -855,9 +894,10 @@
169 const gchar *latin_variant = "";
170 const gchar *locale_layout = NULL;
171 const gchar *locale_variant = NULL;
172+ gint n_xkb_layouts = 0;
173
174 if (!layout)
175- return;
176+ return n_xkb_layouts;
177
178 if (!variant)
179 variant = "";
180@@ -887,10 +927,19 @@
181 }
182
183 free (xkb_var_defs->layout);
184- xkb_var_defs->layout = build_xkb_group_string (layout, locale_layout, latin_layout);
185+ xkb_var_defs->layout = build_xkb_group_string (layout, locale_layout, latin_layout, manager->priv->shift_switch);
186
187 free (xkb_var_defs->variant);
188- xkb_var_defs->variant = build_xkb_group_string (variant, locale_variant, latin_variant);
189+ xkb_var_defs->variant = build_xkb_group_string (variant, locale_variant, latin_variant, manager->priv->shift_switch);
190+
191+ n_xkb_layouts = 1;
192+
193+ if (latin_layout)
194+ n_xkb_layouts++;
195+ if (locale_layout)
196+ n_xkb_layouts++;
197+
198+ return n_xkb_layouts;
199 }
200
201 static gchar *
202@@ -1018,13 +1067,14 @@
203 XkbRF_RulesRec *xkb_rules;
204 XkbRF_VarDefsRec *xkb_var_defs;
205 gchar *rules_file_path;
206+ gint n_xkb_layouts;
207
208 gsd_xkb_get_var_defs (&rules_file_path, &xkb_var_defs);
209
210 free (xkb_var_defs->options);
211 xkb_var_defs->options = options;
212
213- replace_layout_and_variant (manager, xkb_var_defs, layout, variant);
214+ n_xkb_layouts = replace_layout_and_variant (manager, xkb_var_defs, layout, variant);
215
216 gdk_error_trap_push ();
217
218@@ -1034,10 +1084,14 @@
219 xkb_comp_names = g_new0 (XkbComponentNamesRec, 1);
220
221 XkbRF_GetComponents (xkb_rules, xkb_var_defs, xkb_comp_names);
222- upload_xkb_description (rules_file_path, xkb_var_defs, xkb_comp_names);
223+ upload_xkb_description (rules_file_path, xkb_var_defs, xkb_comp_names,
224+ n_xkb_layouts, !manager->priv->shift_switch);
225+ manager->priv->latin_first = !manager->priv->shift_switch;
226
227 free_xkb_component_names (xkb_comp_names);
228 XkbRF_Free (xkb_rules, True);
229+
230+ manager->priv->n_xkb_layouts = n_xkb_layouts;
231 } else {
232 g_warning ("Couldn't load XKB rules");
233 }
234@@ -2393,6 +2447,54 @@
235 #endif
236
237 static gboolean
238+is_shift (const gchar *shortcut)
239+{
240+ return shortcut && (g_str_equal (shortcut, "Shift_L") || g_str_equal (shortcut, "<Shift>Shift_L")
241+ || g_str_equal (shortcut, "Shift_R") || g_str_equal (shortcut, "<Shift>Shift_R"));
242+}
243+
244+static gboolean
245+is_shift_switch (GSettings *settings)
246+{
247+ gboolean shift_switch = FALSE;
248+ gchar **shortcuts;
249+ gchar **i;
250+
251+ shortcuts = g_settings_get_strv (settings, SWITCH_INPUT_SOURCE_KEY);
252+
253+ for (i = shortcuts; !shift_switch && *i; i++)
254+ shift_switch = is_shift (*i);
255+
256+ g_strfreev (shortcuts);
257+
258+ if (shift_switch)
259+ return shift_switch;
260+
261+ shortcuts = g_settings_get_strv (settings, SWITCH_INPUT_SOURCE_BACKWARD_KEY);
262+
263+ for (i = shortcuts; !shift_switch && *i; i++)
264+ shift_switch = is_shift (*i);
265+
266+ g_strfreev (shortcuts);
267+
268+ return shift_switch;
269+}
270+
271+static void
272+switch_settings_changed (GSettings *settings,
273+ gchar *key,
274+ gpointer user_data)
275+{
276+ GsdKeyboardManager *manager = user_data;
277+ GsdKeyboardManagerPrivate *priv = manager->priv;
278+
279+ if (is_shift_switch (settings) != priv->shift_switch) {
280+ priv->shift_switch = !priv->shift_switch;
281+ apply_input_source (manager, priv->active_input_source);
282+ }
283+}
284+
285+static gboolean
286 start_keyboard_idle_cb (GsdKeyboardManager *manager)
287 {
288 const gchar *module;
289@@ -2419,6 +2521,8 @@
290
291 manager->priv->input_sources_settings = g_settings_new (GNOME_DESKTOP_INPUT_SOURCES_DIR);
292 manager->priv->interface_settings = g_settings_new (GNOME_DESKTOP_INTERFACE_DIR);
293+ manager->priv->switch_settings = g_settings_new (SWITCH_SETTINGS_DIR);
294+ manager->priv->shift_switch = is_shift_switch (manager->priv->switch_settings);
295 manager->priv->xkb_info = gnome_xkb_info_new ();
296
297 #ifdef HAVE_FCITX
298@@ -2473,7 +2577,8 @@
299 G_CALLBACK (settings_changed), manager);
300 g_signal_connect (G_OBJECT (manager->priv->gsettings), "changed",
301 G_CALLBACK (gsettings_changed), manager);
302-
303+ g_signal_connect (G_OBJECT (manager->priv->switch_settings), "changed",
304+ G_CALLBACK (switch_settings_changed), manager);
305 g_signal_connect (G_OBJECT (manager->priv->input_sources_settings), "change-event",
306 G_CALLBACK (apply_input_sources_settings), manager);
307
308@@ -2527,6 +2632,7 @@
309 g_clear_object (&p->cancellable);
310
311 g_clear_object (&p->settings);
312+ g_clear_object (&p->switch_settings);
313 g_clear_object (&p->input_sources_settings);
314 g_clear_object (&p->interface_settings);
315 g_clear_object (&p->xkb_info);
316@@ -2577,6 +2683,7 @@
317 {
318 manager->priv = GSD_KEYBOARD_MANAGER_GET_PRIVATE (manager);
319 manager->priv->active_input_source = -1;
320+ manager->priv->n_xkb_layouts = -1;
321 }
322
323 static void

Subscribers

People subscribed via source and target branches