Open Bug 1182596 Opened 7 years ago Updated 2 years ago

[GTK] ResetIME cause gtk_im_context_reset is called to frequently when it's unnecessary

Categories

(Core :: DOM: UI Events & Focus Handling, defect, P3)

39 Branch
x86_64
Linux
defect

Tracking

()

People

(Reporter: wengxt, Unassigned)

References

Details

(Keywords: inputmethod, Whiteboard: tpi:+)

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:39.0) Gecko/20100101 Firefox/39.0
Build ID: 20150704182459

Steps to reproduce:

Press key "A" in input method (even when key is not filtered.)


Actual results:

"A" appears in the text box, and ResetIME is being called.


Expected results:

"A" appears in the text box, ResetIME is not being called.
Attached file log when press key
Component: Untriaged → Widget: Gtk
Product: Firefox → Core
This causes issue in input method that relies on keeping input state after committing a string.
E.G. https://github.com/fcitx/fcitx-unikey/issues/6

fcitx-unikey may do the spell check based on what user just types in. When user types in gioongs, fcitx-unikey holds "g" in the internal buffer for spell-checking purpose even after "g" is committed. But firefox will send reset to input method after a input method commit, so input method will discard this internal buffer based on firefox request. In that way, spell-checking in fcitx thinks that user only types in "ioongs" instead of "gioongs", which is not an intended behavior and causing the issue above.
Priority: -- → P3
Whiteboard: tpi:+
There's another issue related to this: https://github.com/fcitx/fcitx-unikey/issues/13
Sorry for the delay to catch this bug.

According to the GTK's document, our behavior must be correct:
https://developer.gnome.org/gtk3/stable/GtkIMContext.html#gtk-im-context-reset
> Notify the input method that a change such as a change in cursor position has been made.
> This will typically cause the input method to clear the preedit state.

Because "cursor position" is changed by inputting a character. Why do you think this is an our bug?

I think that notifying IME of selection change is important especially when IME wants surrounding text. If IME retrieves surrounding text with a selection range, they need to update the surrounding text cache at every selection change.

We had similar issue, bug 1138159, by our side. Does this help you? Could you try to check with Nightly build? <https://nightly.mozilla.org/>

Note that we know some complicated web apps resetting selection range at each input. Therefore, even if the bug fix helps your IME, there could be same issue with some web apps.
Flags: needinfo?(wengxt)
Keywords: inputmethod
> We had similar issue, bug 1138159, by our side.

I meant we had the issue bug we fixed it by our side.
Well, the problem is gtk_im_context_reset shouldn't be called when cursor moves if this movement is caused by typing text. If cursor moves because of arrow key, or mouse click, then it's fine to reset the input method state. And there's separate function for handling surrounding text: https://developer.gnome.org/gtk3/stable/GtkIMContext.html#gtk-im-context-set-surrounding .

I was just comparing the behavior in gtk-demo and firefox.

Whenever I type anything (key bypass input method), gtk_im_context_reset is called in firefox, but not in gtk-demo's text entry case. Namely, firefox and gtk entry behaves differently (And firefox is the wrong one).

BTW, this specific problem is not resolved in nightly.
Flags: needinfo?(wengxt)
I think gtk documentation is not clear on what's "cursor move" (https://developer.gnome.org/gtk3/stable/GtkIMContext.html#gtk-im-context-reset). In reality (or gtk implementation), only "real" movements (click, arrow key, etc) will trigger gtk-im-context-reset.

If the text is from input method, then firefox will not call gtk_im_context_reset (this is correct) and cursor indeed moves in this case.
It only happens when input method don't want to handle the key and this key will produce some text (e.g. key A,B, C...).
Sorry for the long delay due to sick and busy...

(In reply to Weng Xuetian from comment #6)
> Well, the problem is gtk_im_context_reset shouldn't be called when cursor
> moves if this movement is caused by typing text. If cursor moves because of
> arrow key, or mouse click, then it's fine to reset the input method state.

I don't think so, it depends on editor's implementation where cursor moves to or not move.

For example, some web apps may replace inputted text, like forcibly converts inputted text to upper case character.

Note that it's very difficult to distinguish if browser's editor change is caused only by a printable key because web apps can do anything with keydown event listeners.

> And there's separate function for handling surrounding text:
> https://developer.gnome.org/gtk3/stable/GtkIMContext.html#gtk-im-context-set-
> surrounding .

Yes, but it is not notification for IM from application.

In Nightly, if IME hasn't retrieved surrounding text, we don't send the notification at every selection change. So, if your IME retrieves surrounding text, it should check new surrounding text.  If new surrounding text is expected, your IME doesn't need to do anything special.

> I was just comparing the behavior in gtk-demo and firefox.
> 
> Whenever I type anything (key bypass input method), gtk_im_context_reset is
> called in firefox, but not in gtk-demo's text entry case. Namely, firefox
> and gtk entry behaves differently (And firefox is the wrong one).

Why our behavior is wrong? We respect the explanation of the reference. Although, the explanation is too unclear.

The most complicated problem is, the name, gtk_im_context_"reset"(), is too odd. According to the GTK document, it should be called when selection is changed. We do it for conforming to the explation of the reference.  Then, it depends on active IME what will do. The reference documents just an example, "This will typically cause the input method to clear the preedit state".  This means that applications can NOT use this method to cancel active composition because it's not the purpose of this API. So, it's NOT our request to reset any state of your IME. (Actually, applications use focusout/focusin for cancelling active composition.)

> BTW, this specific problem is not resolved in nightly.

On the other hand, why doesn't our behavior cause any trouble with other IMEs?
Flags: needinfo?(wengxt)
But let me tell you the truth, Gtk's own widget doesn't call gtk_im_context_reset when selection is changed. Either Gtk's text entry implementation is broken, or their documentation is broken.

Also, the concept of "reset" is not invented by Gtk. It is also shared by Qt, XIM, Wayland
http://doc.qt.io/qt-5/qinputmethod.html#reset
https://www.x.org/releases/X11R7.6/doc/libX11/specs/XIM/xim.html#Reset_IC
https://cgit.freedesktop.org/wayland/wayland-protocols/tree/unstable/text-input/text-input-unstable-v1.xml#n95

The wayland one probably is the most clear one and that's what we expected from IME point of view. I believe that Gtk's implementation also shares the same view of the purpose of reset. And yes, it IS depends on IME to clear the preedit state, IME basically could do anything upon this reset request. BUT, if an IME DOES NOT clear the preedit state, it is considered as flawed from an IM developer point of view.

This specific behavior, is not causing problem in other IME because those IM doesn't have such assumption.

Also, I don't think there's any real "Selection" change. Nothing is selected in the case that I described. In the firefox's case:
- If the string is committed by ime (via commit-string signal), gtk_im_context_"reset" is NOT called by firefox.
- If the key is ignored by IME, and it does cause a string to be committed (E.g. key A, or even key left/right to move cursor), gtk_im_context_"reset" IS called by firefox.

I don't think that could be taken as consistent behavior.

And just FYI, this specific bug does not exist in old firefox (I don't remember which, but I don't think I have such issue like 5 years ago).
Flags: needinfo?(wengxt)
Let me show an example for this.

In Chinese, the standard uses these character “” for quote. This characters are typed in pair. But there is only one " key on the keyboard. The input method remembers the state that whether " is pressed. When it's pressed for the first time, it produces “, next time it will produce ”.

This works flawlessly in any other program except firefox. When any raw key is passed to firefox, firefox call's the gtk_im_context_reset and input method reset the state. Because this is the only right thing to do. gtk_im_context_reset is only called any other gtk program in rare cases, for example, user click another place in the input box. But in firefox, firefox will even call this function whenever there's a raw key typed.

As I said before, firefox's implementation does not match the implement in gtk. Gtk's documentation might have some problem, but the behavior in firefox in wrong.
I guess it makes sense for us to follow what the reference implementation does on this. And it sounds reasonable that reset shouldn't be triggered if a cursor position change is from a printable key.

Based on the discussion above, I guess the relevant code is: https://searchfox.org/mozilla-central/rev/6326724982c66aaeaf70bb7c7ee170f7a38ca226/widget/gtk/IMContextWrapper.cpp#1015-1033 which happens when the cursor changes position (which is basically a selection change). And I agree that it seems hard to distinguish between the cursor move (selection change) from typing and from script changing.
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #11)
> And I agree that it seems hard to distinguish
> between the cursor move (selection change) from typing and from script
> changing.

Yeah. Perhaps, IMContextWrapper needs to compute expected selection and when NOTIFY_IME_OF_SELECTION_CHANGE comes and matches with the expected result, IMContextWrapper shouldn't report the selection change to IME.  On the other hand, we are now handling text input asynchronously due to e10s. So, IMContextWrapper needs to wait all pending selection changes if there are. This needs a lot of changes in IMContextWrapper like TSFTextStore (for Windows).

That's really related to bug 1284415 and bug 1173694. Both of them are long standing bug, though...
See Also: → 1284415, 1173694

Moving all open keyboard/IME handling bugs to DOM: UI Events & Focus Handling component.

Component: Widget: Gtk → DOM: UI Events & Focus Handling
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Unspecified → Linux
Hardware: Unspecified → x86_64
Summary: ResetIME cause gtk_im_context_reset is called to frequently when it's unnecessary → [GTK] ResetIME cause gtk_im_context_reset is called to frequently when it's unnecessary
You need to log in before you can comment on or make changes to this bug.