1516323 - At least ibus won't set a modifier flag which indicates a key event is synthesized asynchronously in some environments

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

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

According to the log of bug 1498823, ibus won't set IBUS_IGNORED_MASK to modifier flags when it synthesizes the event for asynchronous handling in some environments.  Currently, we assume that iBus and Fcitx set IBUS_IGNORED_MASK or FcitxKeyState_IgnoredMask.  So, we put both real key events and synthesized key events into the posting event queue and that causes using a lot of memory until the editor is blurred.  Fortunately, timestamp of synthesized key events are always same as their original events.  Therefore, we can look for original event from the positing event queue.

Comment 1

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

Attachment: Bug 1516323 - Make IMContextWrapper::OnKeyEvent() check whether coming key event is already in the posting event queue

According to the log of bug 1498823, ibus won't set IBUS_IGNORED_MASK to
modifier flags when it synthesizes the event for asynchronous handling in
some environments.

Currently, we assume that iBus and Fcitx set IBUS_IGNORED_MASK or
FcitxKeyState_IgnoredMask.  So, we put both real key events and synthesized
key events into the posting event queue and that causes using a lot of
memory until the editor is blurred.  Fortunately, timestamp of synthesized
key events are always same as their original events.  Therefore, we can look
for original event from the positing event queue.

Although we have gotten no bug reports about this issue of Fcitx, but this
patch adds same hack for Fcitx too because the runtime cost is not
expensive but the symptom is really serious.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/90370fd4bc2a
Make IMContextWrapper::OnKeyEvent() check whether coming key event is already in the posting event queue r=m_kato

Comment 3

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/90370fd4bc2a

Comment 4

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

Comment on attachment 9033226 [details]
Bug 1516323 - Make IMContextWrapper::OnKeyEvent() check whether coming key event is already in the posting event queue

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: None

User impact if declined: This affects users perhaps only on some broken environments. Currently, we confirm that this occurs when you install debian-cinnamon from its ISO image. 

This bug causes dispatching redundant keydown/keyup events on such environment because of broken synthesized key events. I.e., one key press may cause 2 actions, and this causes every key events into our queue of posted events until new editor gets focus.  Therefore, this is also a memory leak bug while user types characters in an editor (the memory leak bug is a regression of bug 1443421).

Additionally, this is necessary to uplift the patch for bug 1498823, that's more important than this bug.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: Bug 1498823

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): This patch checks whether coming key event is a broken key event or not, with timestamp of native event. Typically, actual key events do not have same timestamp. So, this shouldn't break actual key event handling.

String changes made/needed: no

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9033226 [details]
Bug 1516323 - Make IMContextWrapper::OnKeyEvent() check whether coming key event is already in the posting event queue

[Triage Comment]
Low-risk patch which blocks the uplift of the more-severe bug 1498823. Approved for 65.0b8.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/4cb9d2d57e7d