1879765 - Menu hotkeys interfere with Windows Unicode input

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[cyaugin]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:109.0) Gecko/20100101 Firefox/115.0

Steps to reproduce:

1. Enable general Unicode input in Windows using the Alt-+ input method. See http://www.johndcook.com/blog/2008/08/17/three-ways-to-enter-unicode-characters-in-windows/
2. Load a page containing a text editing box, e.g. https://en.wikipedia.org/w/index.php?title=Wikipedia:Sandbox&action=edit
3. In the edit box, hold down the Alt key and type the sequence "+274e" to generate the respective glyph.

Actual results:

Upon typing "e" the browser's Edit menu is activated, interfering with the Unicode input and preventing the glyph from appearing. This occurs with any hex letter that is both a possible Unicode value and a browser menu hotkey: (F)ile, (E)dit, (B)ookmarks

Expected results:

The browser menu should suppress Alt hotkey activation after "+" is input.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::DOM: Editor' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

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

I'll check this tomorrow.

Comment 3

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

Sorry for the long delay.

The website suggests wrong registry editing... The key should be in HKEY_CURRENT_USER\Control Panel\Input Method...

If I press Alt + e, I got following log:

[Parent 53452: Main Thread]: I/KeyboardHandler 54bdbfdeb8 NativeKey::NativeKey(aWidget=0x2141f112800 { GetWindowHandle()=0xe1100 }, aMessage={ message=WM_SYSKEYDOWN, virtual keycode=VK_E, repeat count=1, scancode=0x12, extended key=false, context code=true, previous key state=false, transition state=false, hwnd=0xe1100, aModKeyState={ Alt | NumLock }), sLatestInstance=0x0
[Parent 53452: Main Thread]: D/KeyboardHandler 54bdbfdeb8   NativeKey::GetFollowingCharMessage(), succeeded to retrieve next char message, aCharMsg={ message=WM_SYSCHAR, character code='e' (0x0065), repeat count=1, scancode=0x12, extended key=false, context code=true, previous key state=false, transition state=false, hwnd=0xe1100
[Parent 53452: Main Thread]: I/KeyboardHandler 54bdbfdeb8   NativeKey::InitWithKeyOrChar(), removed char message, { message=WM_SYSCHAR, character code='e' (0x0065), repeat count=1, scancode=0x12, extended key=false, context code=true, previous key state=false, transition state=false, hwnd=0xe1100

Then, if I press same key press during the Alt + + sequence, I got:

[Parent 53452: Main Thread]: I/KeyboardHandler 54bdbfdeb8 NativeKey::NativeKey(aWidget=0x2141f112800 { GetWindowHandle()=0xe1100 }, aMessage={ message=WM_SYSKEYDOWN, virtual keycode=VK_E, repeat count=1, scancode=0x12, extended key=false, context code=true, previous key state=false, transition state=false, hwnd=0xe1100, aModKeyState={ Alt | NumLock }), sLatestInstance=0x0
[Parent 53452: Main Thread]: D/KeyboardHandler 54bdbfdeb8   NativeKey::GetFollowingCharMessage(), there are no char messages
[Parent 53452: Main Thread]: I/KeyboardHandler 54bdbfdeb8   NativeKey::NativeKey(), mKeyboardLayout=0x4110411, mFocusedWndBeforeDispatch=0xe1100, mDOMKeyCode=VK_E, mKeyNameIndex=USE_STRING, mCodeNameIndex=KeyE, mModKeyState={ Alt | NumLock }, mVirtualKeyCode=VK_E, mOriginalVirtualKeyCode=VK_E, mCommittedCharsAndModifiers={ 'e' (0x0065) [Alt] }, mInputtingStringAndModifiers={ 'e' (0x0065) [Alt] }, mShiftedString={ 'E' (0x0045) [Shift] }, mUnshiftedString={ 'e' (0x0065) [none] }, mShiftedLatinChar=NULL (0x0000), mUnshiftedLatinChar=NULL (0x0000), mScanCode=0x0012, mIsExtended=false, mIsRepeat=false, mIsDeadKey=false, mIsPrintableKey=true, mIsSkippableInRemoteProcess=false, mCharMessageHasGone=false, mIsOverridingKeyboardLayout=false

It seems that we should not handle WM_SYSKEYDOWN as a mnemonic if it's not followed by WM_SYSCHAR.

Comment 4

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

Hmm, simply consuming Alt + E does not fix this bug...

I/KeyboardHandler 4a75bfdf08 NativeKey::NativeKey(aWidget=0x1533f415800 { GetWindowHandle()=0x90a72 }, aMessage={ message=WM_KEYUP, virtual keycode=VK_MENU, repeat count=1, scancode=0x38, extended key=false, context code=false, previous key state=true, transition state=true, hwnd=0x90a72, aModKeyState={ NumLock }), sLatestInstance=0x0
I/KeyboardHandler 4a75bfdf08   NativeKey::NativeKey(), mKeyboardLayout=0x4110411, mFocusedWndBeforeDispatch=0x90a72, mDOMKeyCode=VK_ALT, mKeyNameIndex=Alt, mCodeNameIndex=AltLeft, mModKeyState={ NumLock }, mVirtualKeyCode=VK_LMENU, mOriginalVirtualKeyCode=VK_MENU, mCommittedCharsAndModifiers={}, mInputtingStringAndModifiers={}, mShiftedString={}, mUnshiftedString={}, mShiftedLatinChar=NULL (0x0000), mUnshiftedLatinChar=NULL (0x0000), mScanCode=0x0038, mIsExtended=false, mIsRepeat=true, mIsDeadKey=false, mIsPrintableKey=false, mIsSkippableInRemoteProcess=false, mCharMessageHasGone=false, mIsOverridingKeyboardLayout=false
D/KeyboardHandler 4a75bfdf08   NativeKey::HandleKeyUpMessage(), initializing keyup event...
I/KeyboardHandler 4a75bfdf08   NativeKey::InitKeyEvent(), initialized, aKeyEvent={ mMessage=eKeyUp, mKeyNameIndex=Alt, mKeyValue="", mCodeNameIndex=AltLeft, mKeyCode=VK_ALT, mLocation=KEY_LOCATION_LEFT, mModifiers=NumLock, DefaultPrevented()=true }
I/KeyboardHandler 4a75bfdf08   NativeKey::HandleKeyUpMessage(), dispatching keyup event...
I/KeyboardHandler 4a75bfdf08   NativeKey::HandleKeyUpMessage(), dispatched keyup event, dispatched=true, consumed=true
D/KeyboardHandler 4a75bfdf08   NativeKey::~NativeKey(), destroyed
I/KeyboardHandler 4a75bfdf08 NativeKey::NativeKey(aWidget=0x1533f415800 { GetWindowHandle()=0x90a72 }, aMessage={ message=WM_CHAR, character code='❎' (0x274E), repeat count=1, scancode=0x38, extended key=false, context code=false, previous key state=true, transition state=true, hwnd=0x90a72, aModKeyState={ NumLock }), sLatestInstance=0x0
I/KeyboardHandler 4a75bfdf08   NativeKey::NativeKey(), mKeyboardLayout=0x4110411, mFocusedWndBeforeDispatch=0x90a72, mDOMKeyCode=VK_ALT, mKeyNameIndex=USE_STRING, mCodeNameIndex=AltLeft, mModKeyState={ NumLock }, mVirtualKeyCode=VK_LMENU, mOriginalVirtualKeyCode=VK_LMENU, mCommittedCharsAndModifiers={ '❎' (0x274E) [NumLock] }, mInputtingStringAndModifiers={}, mShiftedString={}, mUnshiftedString={}, mShiftedLatinChar=NULL (0x0000), mUnshiftedLatinChar=NULL (0x0000), mScanCode=0x0038, mIsExtended=false, mIsRepeat=true, mIsDeadKey=false, mIsPrintableKey=true, mIsSkippableInRemoteProcess=false, mCharMessageHasGone=false, mIsOverridingKeyboardLayout=false
D/KeyboardHandler 4a75bfdf08   NativeKey::HandleCharMessage(), initializing keypress event...
I/KeyboardHandler 4a75bfdf08   NativeKey::InitKeyEvent(), initialized, aKeyEvent={ mMessage=eKeyPress, mKeyNameIndex=USE_STRING, mKeyValue="❎", mCodeNameIndex=AltLeft, mKeyCode=Invalid DOM keyCode (0x00000000), mLocation=KEY_LOCATION_LEFT, mModifiers=NumLock, DefaultPrevented()=false }
I/KeyboardHandler 4a75bfdf08   NativeKey::HandleCharMessage(), dispatching keypress event...
I/KeyboardHandler 4a75bfdf08   NativeKey::HandleCharMessage(), dispatched keypress event, dispatched=true, consumed=false
D/KeyboardHandler 4a75bfdf08   NativeKey::~NativeKey(), destroyed

Comment 5

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

Ah, perhaps, caused by here: https://searchfox.org/mozilla-central/rev/c7df16ffad1f12a19c81c16bce0b65e4a15304d0/dom/ipc/BrowserChild.cpp#2000-2001,2014-2015

Comment 6

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

Attachment: Bug 1879765 - part 1: Make `NativeKey` consume printable `eKeyDown` event before dispatch if the key down is fired only with `WM_SYSKEYDOWN` r=m_kato!

When a printable key is pressed with Alt, Windows notify us of WM_SYSKEYDOWN
and WM_SYSCHAR. However, if the builtin IME for inputting a Unicode character
consumes some printable key presses, Windows does not notify us of WM_SYSCHAR.
Therefore, if WM_SYSKEYDOWN comes without WM_SYSCHAR, we should consume
the eKeyDown before dispatching the event so that the key press won't be
handled by access key of the menu nor the content.

Comment 7

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

Attachment: Bug 1879765 - part 2: Make `BrowserChild` store the last `code` value of consumed `eKeyDown` event r=smaug!,#dom-core

The builtin legacy IME of Windows to type a Unicode with typing a code point
causes consumed eKeyDown events while typing the code point, i.e., without
eKeyPress (FYI: The consumed state is not exposed to the web, it's used only
in chrome UI for the compatibility with Chrome). Then, BrowserChild store
whether the last eKeyDown was consumed or not to prevent the following
eKeyPress. Finally, a eKeyPress event is fired to input the Unicode
character after eKeyUp for Alt. The stored value is set to new value only
when another eKeyDown event is sent from the parent process. Therefore,
the last digit inputting eKeyDown causes BrowserChild thinking the last
eKeyDown is consumed so that the last eKeyPress is not dispatched.

This patch makes BrowserChild to store the code value of the last consumed
eKeyDown and check the code value to consider whether coming eKeyPress
should be or not be dispatched to PresShell and the DOM.

Depends on D207956

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/87e77d284071
part 1: Make `NativeKey` consume printable `eKeyDown` event before dispatch if the key down is fired only with `WM_SYSKEYDOWN` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/6214b15c2021
part 2: Make `BrowserChild` store the last `code` value of consumed `eKeyDown` event r=smaug

Comment 9

[tszentpeteri]

Backed out for causing failures on test_nsITextInputProcessor.xhtml.

• Backout link
• Push with failures
• Failure Log
• Failure log:

[task 2024-05-15T11:52:25.255Z] 11:52:25     INFO - TEST-PASS | dom/base/test/chrome/test_nsITextInputProcessor.xhtml | runInsertTextWithKeyPressTests(): insertTextWithPress with Alt keydown whose altKey is true should cause only a "keypress" event whose altKey is false" 
[task 2024-05-15T11:52:25.255Z] 11:52:25     INFO - Buffered messages finished
[task 2024-05-15T11:52:25.256Z] 11:52:25     INFO - TEST-UNEXPECTED-FAIL | dom/base/test/chrome/test_nsITextInputProcessor.xhtml | runInsertTextWithKeyPressTests(): insertTextWithKeyPress after Alt keydown should not cause inserting the string - got "X", expected ""
[task 2024-05-15T11:52:25.256Z] 11:52:25     INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:509:14
[task 2024-05-15T11:52:25.256Z] 11:52:25     INFO - is@chrome://mochitests/content/chrome/dom/base/test/chrome/window_nsITextInputProcessor.xhtml:42:14
[task 2024-05-15T11:52:25.256Z] 11:52:25     INFO - runInsertTextWithKeyPressTests@chrome://mochitests/content/chrome/dom/base/test/chrome/window_nsITextInputProcessor.xhtml:3750:5
[task 2024-05-15T11:52:25.256Z] 11:52:25     INFO - runTests@chrome://mochitests/content/chrome/dom/base/test/chrome/window_nsITextInputProcessor.xhtml:4975:3
[task 2024-05-15T11:52:25.256Z] 11:52:25     INFO - async*SimpleTest.waitForFocus/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1058:13
[task 2024-05-15T11:52:25.257Z] 11:52:25     INFO - TEST-PASS | dom/base/test/chrome/test_nsITextInputProcessor.xhtml | runInsertTextWithKeyPressTests(): insertTextWithPress after Alt keydown should cause only a "keypress" event whose altKey is true" 

Comment 10

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

Oh, macOS only failure... I forgot that Alt (option) key is a modifier for text input only on macOS...

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dd3310ad237f
part 1: Make `NativeKey` consume printable `eKeyDown` event before dispatch if the key down is fired only with `WM_SYSKEYDOWN` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/a3e49df724a5
part 2: Make `BrowserChild` store the last `code` value of consumed `eKeyDown` event r=smaug

Comment 12

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/dd3310ad237f
https://hg.mozilla.org/mozilla-central/rev/a3e49df724a5