1511752 - German keyboard layout which have dead keys sends odd hacky event to us and that causes unexpected keydown event

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I confirmed that this occurs with both iBus and Fcitx.

STR:
1. Add German keyboard layout which has dead key on Linux (e.g., German (QWERTY))
2. Load https://w3c.github.io/uievents/tools/key-event-viewer.html
3. Type "Equal" key of US keyboard layout (accent character of German (QWERTY))
4. Type "KeyA" key of US keyboard layout ("a" of German (QWERTY)

Then, you see redundant keydown event before committing a composed character with a set of composition events. The keydown event does not have any information, key is "Unidentified", keyCode etc are all 0, code is empty string, getModifierState() returns no modifiers. The reason of this bug is, German keyboard layout filters "KeyA" event for handling it asynchronously, and then, sends dummy key event to us to make us call gtk_im_context_filter_keypress() again, but it does not have any reasonable information:
> { type=GDK_KEY_PRESS, keyval=(null), unicode=0x0, state=, time=0, hardware_keycode=0, group=0 }
So, currently, we don't treat this event as synthesized by IME asynchronously.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1511752 - Make IMContextWrapper::OnKeyEvent() treat GDK_KEY_PRESS event without any key information in a dead key sequence as synthesized by current keyboard layout for asynchronous handling

Some keyboard layouts which have dead keys may handle dead key composition
asynchronously.  In this case, they don't rely on IME like iBus/Fcitx.

As far as I've tested, German (QWERTY) keyboard layout is one of such
keyboard layout.  This returns true when gtk_im_context_filter_keypress()
is called for a base character input (like "a").  Then, it synthesizes
GDK_KEY_PRESS event without any key information such as:
> { type=GDK_KEY_PRESS, keyval=(null), unicode=0x0, state=, time=0,
>   hardware_keycode=0, group=0 }
So, this is not marked as IBUS_IGNORED_MASK nor FcitxKeyState_IgnoredMask
by IME, but we should ignore this event since we should've already dispatched
"keydown" event for the preceding "a" key event, and anyway "keydown" event
for the synthesized event does not make sense for any web apps.

This patch makes IMContextWrapper::OnKeyEvent() ignore such key event, i.e.,
when it's in a dead key sequence, and GDK_KEY_PRESS does not have enough
information, e.g., hardware_keycode shouldn't be 0 especially for printable
keys.  Therefore, this patch make it check only hardware_keycode value and
gdk_keyval_to_unicode(aEvent->keyval) value.

If some keyboard layouts would send the original key event as is, we would
need to do more, but currently, this is enough and safe to land this timing.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4ffd9844c4d918ab9b5c35768d9500d70003018

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f7fafed0e4a9
Make IMContextWrapper::OnKeyEvent() treat GDK_KEY_PRESS event without any key information in a dead key sequence as synthesized by current keyboard layout for asynchronous handling r=m_kato

Comment 4

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/f7fafed0e4a9

Comment 5

[Markus Popp]

Is this supposed to be fixed in the latest Nightly? I still get duplicated characters.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Yes, it is, but this bug fixed only our illegal keydown event dispatching with German keyboard layout on Linux. It does not cause bug 1508200 directly but could be a blocker when Google fixes their bug.

Comment 7

[Markus Popp]

Aha, I failed to notice that this is a different ticket than the one that I opened. So me being confused, sorry.

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

No problem, sorry for the confusion.