Closed Bug 1511752 Opened 11 months ago Closed 10 months ago

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

Categories

(Core :: Widget: Gtk, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox-esr60 --- unaffected
firefox64 --- unaffected
firefox65 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: inputmethod, regression)

Attachments

(1 file)

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.
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.
Pushed by masayuki@d-toybox.com:
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
https://hg.mozilla.org/mozilla-central/rev/f7fafed0e4a9
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Is this supposed to be fixed in the latest Nightly? I still get duplicated characters.
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.
Aha, I failed to notice that this is a different ticket than the one that I opened. So me being confused, sorry.
No problem, sorry for the confusion.
You need to log in before you can comment on or make changes to this bug.