onKeyDown fires the keycode for space twice when using dead keys

VERIFIED FIXED in Firefox 65

Status

()

defect
P3
normal
VERIFIED FIXED
9 months ago
6 months ago

People

(Reporter: travpc, Assigned: masayuki)

Tracking

({inputmethod})

63 Branch
mozilla65
All
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox65 verified)

Details

Attachments

(1 attachment)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.77 Safari/537.36

Steps to reproduce:

1. Find an input field (like the one here: https://codepen.io/anon/pen/YRyNpv)
2. Using a dead key enabled keyboard, activate a dead key without using the space bar (ie: hit the quote key twice to insert a quote. I used US International w/ dead keys)
3. Press the space bar
4. Note that two key down events were issued.


Actual results:

Two key down events were issued.


Expected results:

One key down event should be issued.
for clarity: this was specifically tested on Ubuntu 18.10 with Firefox 63 using the US International (with dead keys) keyboard layout.
Confirming in the sense that the allegation of behavior is correct (tested on Ubuntu 18.04 using the Finnish keyboard layout). It is not clear to me what the "correct" behavior from the Web compat point of view should be.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Well, got it and must be easy to fix.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Keywords: inputmethod
OS: Unspecified → Linux
Hardware: Unspecified → All
Currently, IMContextWrapper::OnKeyEvent() assumes that IME won't synthesize
keyboard event asynchronously again in some cases.  It's one of the cases
that user inputs text with a dead key sequence.  However, IME may synthesize
key event only in a few cases even in a dead key sequence.  For not losing
a chance to dispatch eKeyDown/eKeyUp event, we should keep dispatching
eKeyDown or eKeyUp event when we receive original event in dead key sequence.
Instead, if we receive unexpected async key event, we should stop dispatching
eKeyDown nor eKeyUp event from nsWindow again.

For doing that, IMContextWrapper::OnKeyEvent() needs to return whether
it (has already) dispatched an eKeyDown or eKeyUp and whether it was
consumed.  Therefore, this patch makes IMContextWrapper::OnKeyEvent()
return KeyHandlingState enum class.  And makes each caller don't dispatch
eKeyDown nor eKeyUp event when it retruns
KeyHandlingState::eNotHandledButDispatched or
KeyHandlingState::eNotHandledButConsumed.
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/f17b7ba6d0aa
nsWindow::OnKeyPressEvent() shouldn't dispatch eKeyDown event when IMContextWrapper::OnKeyEvent() has already dispatched it for the event r=m_kato
https://hg.mozilla.org/mozilla-central/rev/f17b7ba6d0aa
Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
See Also: → 1515321
Flags: qe-verify+

Confirmed the issue with 65.0a1 (2018-11-07) on Ubuntu16.04 after changing to US International (with dead keys) keys layout.
Fix verified with 65.0b12 on Ubuntu 16.04.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Depends on: 1523635
Depends on: 1524525
You need to log in before you can comment on or make changes to this bug.