GNOME Bugzilla – Bug 698183
Text input is too slow and some keys are broken after changing input source
Last modified: 2013-06-20 15:32:05 UTC
Overview: After I switch input sources by Mac OS X's input menu, text input becomes too slow and some keys such as "delete" are disabled. In addition to it, many warning messages are output to console when I press a key. Steps to reproduce: 1. Open Mac OS X's "System Preferences" window. 2. Open "International" -> "Input source" page. 3. Enable "Vietnamese" -> "Simple Telex" 4. Enable "Show input menu in menu bar" 5. Run gtk3-demo from a console 6. Open a text entry from "Entry" -> "Entry Buffer" 7. Switch input source to "Simple Telex" from input menu of Mac OS X 8. Press normal keys to enter text into the entry 9. Press "delete" to delete some characters Actual results: * Text input becomes too slow. It's hard to enter text even one charater. * Some keys such as "delete" doesn't work * Many warning messages are output to console: (gtk3-demo:37734): GLib-CRITICAL **: g_hash_table_insert_internal: assertion `hash_table != NULL' failed (gtk3-demo:37734): GLib-CRITICAL **: g_hash_table_lookup: assertion `hash_table != NULL' failed (gtk3-demo:37734): GLib-CRITICAL **: g_hash_table_insert_internal: assertion `hash_table != NULL' failed (gtk3-demo:37734): GLib-CRITICAL **: g_hash_table_lookup: assertion `hash_table != NULL' failed .... Expected results: * Text input shound't become slow * All keys should work correctly * Runtime warning messages shouldn't be output Additional information: * I confirmed the bug on Japanese environment of Mac OS X. I'm not sure it is also reproduced on English environment. * gtk+-2.24.16 has same issue.
Created attachment 241720 [details] Stack trace with "--g-fatal-warnings"
Created attachment 241723 [details] [review] Proposed patch v1 I think main cause of this is that GdkQuartz tries to update keymap on every key event. It's too expesive and risky. This fix uses CFNotificationCenter to observe keymap switching so that GdkQuartz tries to update keymap only when it's really needed.
BTW I found this bug while I was developing an immodule for Mac OS X: https://github.com/ashie/gtkimcocoa Bug 694273
I don't dispute your diagnosis, and the change does seem more efficient, but I'm not able to replicate the problem even if I switch localization settings to Japanese & Japan. Are you perhaps not using a US keyboard?
I think the main cause of this problem is this comparison. https://github.com/GNOME/gtk/blob/d0af25f12c2c99ef0959ce19b1385a9f375ac19c/gdk/quartz/gdkkeys-quartz.c#L285 In maybe_update_keymap() function it is checked whether the keyboard layout has been changed or not. But the way of checking it have a problem. The return value of TISCopyCurrentKeyboardLayoutInputSource() is used to check it. But the value can differ even if the keyboard layout is not changed. I think InputSourceID should be compared which can be acquired with TISGetInputSourceProperty. (CFStringRef*) TISGetInputSourceProperty(new_layout, kTISPropertyInputSourceID)
Sorry, CFStringRef is correct. (CFStringRef) TISGetInputSourceProperty(new_layout, kTISPropertyInputSourceID)
Created attachment 244894 [details] [review] Compares InputSourceID in maybe_update_keymap().
I created a patch. 0001-Compares-InputSourceID-in-maybe_update_keymap.patch I think this patch and Ashie-san's patch should be applied.
Sorry for late response. (In reply to comment #4) > Are you perhaps not using a US keyboard? Yes, I'm using Japanese keyboard.
(In reply to comment #5) > I think the main cause of this problem is this comparison. > https://github.com/GNOME/gtk/blob/d0af25f12c2c99ef0959ce19b1385a9f375ac19c/gdk/quartz/gdkkeys-quartz.c#L285 Thank you for following the bug. It seems fine. I also suspected the comparison but I couldn't confirm & fix it by myself because I'm not familiar with Mac OS X.
Hmm. Does this manifest only with GtkIMCocoa? Or only with a non-US keyboard? I'm not able to replicate the problem. Since running update_keyboard is controlled by the notification, shouldn't all of the conditional stuff that made it maybe_update_keyboard be removed rather than tweaking which identifier is used to notice that the layout is changed?
> Hmm. Does this manifest only with GtkIMCocoa? Or only with a non-US keyboard? > I'm not able to replicate the problem. I don't know why, but the return value of TISCopyCurrentKeyboardLayoutInputSource differ only when we are using third party input method software. Strange to say, it does not always occur even if we are using third party input method software. I think this is depending on OS's internal state. You can build and try Apple's sample code of input method as the following steps. curl -o NumberInput_IMKit_Sample.zip https://developer.apple.com/library/mac/samplecode/NumberInput_IMKit_Sample/NumberInput_IMKit_Sample.zip xcodebuild -project "NumberInput_IMKit_Sample/trunk/NumberInput 4/NumberInput.xcodeproj" -configuration "Release" sudo cp -rf "NumberInput_IMKit_Sample/trunk/NumberInput 4/build/Release/NumberInput.app" "/Library/Input Methods/" Logout Login Open "System Preferences" Open "Language & Text" Select "Input Sources" Turn on "NumberInput" Select "Decimal" in input menu in menu bar. Type "123467" + Space -> "1,234,567" will be shown. > Since running update_keyboard is controlled by the notification, shouldn't all > of the conditional stuff that made it maybe_update_keyboard be removed rather > than tweaking which identifier is used to notice that the layout is changed? Oh yes. Removing this conditional stuff is good enough. By the way, this problem occurs while using GIMP with third party input method software. I think GIMP is using GTK2. It this fix will be applied to GTK2?
fix typo: Will this fix be applied to GTK2?
(In reply to comment #11) > Hmm. Does this manifest only with GtkIMCocoa? Or only with a non-US keyboard? > I'm not able to replicate the problem. I confirmed this issue without any immodules. I didn't install any third party immodules to reproduce it. It's still reproduced even if I disable all bundled immodules.
One of posibillity is target architecture? I confirmed it with x86_64 binary.
(In reply to comment #12) > I don't know why, but the return value of > TISCopyCurrentKeyboardLayoutInputSource differ only when we are using third > party input method software. I also confirmed it using third party input method (GoogleIME) at first. Since I ensured to simplify steps to reproduce, I looked for a system IME which causes the issue on my environment.
Created attachment 245658 [details] [review] A patch against GTK+2 to observe input source (In reply to comment #13) > Will this fix be applied to GTK2? Sorry, I forgot to attach the patch against GTK2 although I already prepared it.
> Sorry, I forgot to attach the patch against GTK2 although I already prepared it. Thank you very much!
I think that we should do two things: - update the keymap in the notification callback - remove the check from the callback, as said in comment 11 Even without having 3rd party input methods or japanese keyboards, both changes seem completely logical to me.
If there are no objections I will do exactly that, in separate patches because the check removal will cherry-pick without any conflict between the branches.
Sounds good to me.
I also agree with you.
Fixed in master, gtk-3-8 and gtk-2-24: commit f04fe99deb92e425acf1da95a0d8a3455bdc14b4 Author: Michael Natterer <mitch@gimp.org> Date: Thu Jun 20 17:21:25 2013 +0200 quartz: remove check for keymap changes from update_keymap() The function is now only called when the keymap has actually changed. bug #698183. (cherry picked from commit e5e17cf3612191c0497882a6a6b3206dc0ca96cd) gdk/quartz/gdkkeys-quartz.c | 425 ++++++++++++++++++++++++++---------------------------- 1 file changed, 207 insertions(+), 218 deletions(-) commit 7b08ca94cc5c9c7798b6f7541f409f3c7dd09c52 Author: Michael Natterer <mitch@gimp.org> Date: Thu Jun 20 17:09:07 2013 +0200 quartz: update the keymap only if the input method changed and not on each keystroke, which for some IMs apparently caused a full update on each keystroke, not just a check for changes. Patch from Takuro Ashie, bug #698183. (cherry picked from commit bbe3554fa9baefc875dcc3d62cd1a4bbbaf18b53) gdk/quartz/gdkkeys-quartz.c | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-)