1505147 - onKeyDown fires the keycode for space twice when using dead keys

Status: VERIFIED FIXED

(Core :: DOM: Events, defect, P3)

Description

[Travis R]

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.

Comment 1

[Travis R]

for clarity: this was specifically tested on Ubuntu 18.10 with Firefox 63 using the US International (with dead keys) keyboard layout.

Comment 2

[Henri Sivonen (:hsivonen)]

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.

Comment 3

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

Well, got it and must be easy to fix.

Comment 4

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

Attachment: Bug 1505147 - nsWindow::OnKeyPressEvent() shouldn't dispatch eKeyDown event when IMContextWrapper::OnKeyEvent() has already dispatched it for the event

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.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
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

Comment 6

[Natalia Csoregi [:nataliaCs]]

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

Comment 7

[Cristian Fogel [:cfogel]]

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.