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

RESOLVED FIXED in Firefox 65

Status

()

RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

Trunk
mozilla66
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox65 fixed, firefox66 fixed)

Details

Attachments

(1 attachment)

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.
Blocks: 1443421
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

3 months ago
Pushed by masayuki@d-toybox.com:
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

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/90370fd4bc2a
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
status-firefox66: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
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
Attachment #9033226 - Flags: approval-mozilla-beta?
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.
Attachment #9033226 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 6

3 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/4cb9d2d57e7d
status-firefox65: --- → fixed
You need to log in before you can comment on or make changes to this bug.