Bug 30389 – [gtk] Input method does not work in Epiphany

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: WebKitGTK

[Duy Nguyen]

The original report is here: https://bugzilla.gnome.org/show_bug.cgi?id=598563

Epiphany used to recognize GTK_IM_MODULE and set input method
for every textbox correctly, changing input method from urlbar also affected
all textboxes. With recent Webkit, it seems input method is set to default
input method, no way I can change it to XIM or Vietnames VIQR.

Xan suspected the culprit was r48964.

[Martin Robinson]

I can't seem to reproduce this behavior using GtkLauncher. Setting GTK_IM_MODULE seems to change the IM context properly. Also, I can't find an example of a GTK+ app which shares the selected IM context between text fields (libgtk-x11-2.0.so.0.1600.1). Duy, any tips on reproducing this?

[Duy Nguyen]

Funny GTK_IM_MODULE works with GtkLauncher for me too. But you can also start GtkLauncher without GTK_IM_MODULE and set IM in url bar. New IM does not affect textboxes. At least Epiphany used to support this way.

The way I tested it is starting epiphany www.google.com with GTK_IM_MODULE, then type something. I also confirm that epiphany broke with 48964 and worked with 48963.

I'm using Gtk+ 2.18.2, Glib 2.22.2, libsoup near to master, LANG=vi_VN.UTF-8.

[Duy Nguyen]

Another observation. Gedit (and GtkEntry) seems to react well with XSettings "gtk-im-module" changes. I change /desktop/gnome/interface/gtk-im-module (then gnome-settings-daemon will update xsettings accordingly) and Gedit/GtkEntry react well with the new input method. Webview does not though. After a view is created, it sticks with the selected input method at that moment forever.

[Duy Nguyen]

Ohh.. and GtkLauncher will pick up the new input method if you use context menu to change IM to something else, then back to system input method. Looks like some hook is missing.

[Duy Nguyen]

To sum it up:
 - Looking at the source, I don't know how the old Epiphany could share input method too, but that would not matter if we had a way to change input method.
 - Speaking of changing system input method via XSettings, WebKit does not call gtk_im_context_focus_{in,out} frequent enough (which explains my last comment). The only place it calls those is in EditorClient::setInputMethodState(), which does not get called if I switch windows, though switching text entry does the trick. Focus problem.
 - A more important bug with input methods. It seems composition in webkit is not right. With input method VIQR, I can type "am" in a GtkEntry and get "am" ('a' will put it to preedit mode, 'a^m' for example will produce 'âm'). With WebKit "am" produces "m". The preedit 'a' is eaten. The only way I can have 'am' is typing 'a\m' -- 'a\' will produce a complete 'a'.

[Duy Nguyen]

Created attachment 45632 [details]
gtkim.patch

This addresses my last point in the last comment. It seems GtkIMContext can emit multiple commit and preedit-changed signals in one row.

I tested it with VIQR input method. You can try to type these in GtkLauncher and a normal GtkEntry to see if the behaviour matches:

aan
bba<move left>d (this does not work correctly due to cursor position, but at least it does not emit "badba")

My first attempt to make a proper patch, take it easy :)

[WebKit Review Bot]

Attachment 45632 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp:78:  Tab found; better to use spaces  [whitespace/tab] [1]
WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp:78:  Use 0 instead of NULL.  [readability/null] [5]
WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp:80:  Tab found; better to use spaces  [whitespace/tab] [1]
WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp:507:  Tab found; better to use spaces  [whitespace/tab] [1]
WebKit/gtk/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 5

[Duy Nguyen]

Created attachment 45633 [details]
gtkim.patch

Tab-damaged

[WebKit Review Bot]

Attachment 45633 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp:78:  Use 0 instead of NULL.  [readability/null] [5]
Total errors found: 1

[Eric Seidel (no email)]

Comment on attachment 45633 [details]
gtkim.patch

How do we test this?  Does this already have a manual test?  Can we make a layout test?

[Duy Nguyen]

(In reply to comment #10)
> (From update of attachment 45633 [details])
> How do we test this?  Does this already have a manual test?  Can we make a
> layout test?

A simple manual test was given in comment #6 (reference behavior is from GtkEntry). I don't know what a layout test is. But I think I can simulate signals sent from an input method (in this case, VIQR) and check the result.

[Duy Nguyen]

I think I can extend testkeyevents.c to cover this bug, but how can I get text from a text field? Document::getElementById() is C++ and thus not accessible from testkeyevents.c.

[Gustavo Noronha (kov)]

(In reply to comment #12)
> I think I can extend testkeyevents.c to cover this bug, but how can I get text
> from a text field? Document::getElementById() is C++ and thus not accessible
> from testkeyevents.c.

This should be easily doable with JavaScriptCore. Look at how Epiphany implements saving form passwords currently (commits should be easy to find, because they are close to HEAD).

[Martin Robinson]

Hey Dan (and others having trouble). I've taken your patch here and incorporated it into my work at 32290. It feels like these fixes belong together. I'd love it if you could confirm this fixes your issues (except the focus_(in/out) issue which I think deserves another patch -- forthcoming!). Do you mind checking out my latest patch on that bug and letting me know if this improve the behavior?

I can handle writing the tests for these issues. The tests for the two behaviors will be very similar.

[Duy Nguyen]

Looks like the patch on bug 32290 solves this issue well.

*** This bug has been marked as a duplicate of bug 32290 ***

[Oliver Hunt]

Comment on attachment 45633 [details]
gtkim.patch

clearing review flag based on prior comments