[Win] Update KeyboardEvent as per the latest specification MDN: KeyboardEvent https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent W3C: UI Events https://www.w3.org/TR/uievents/#events-keyboardevents Bug 162852 – [Meta] Update KeyboardEvent as per the latest specification Bug 166759 – [GTK] Should support key and code properties on keyboard events
UI Events KeyboardEvent code Values https://www.w3.org/TR/uievents-code/ UI Events KeyboardEvent key Values https://www.w3.org/TR/uievents-key/ KeyboardEvent: code values - Web APIs | MDN https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/code/code_values Key Values - Web APIs | MDN https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key/Key_Values
Windows WM_KEYDONW has only key code, but charactor information which is needed for KeyboardEvent.key. In Chromium, PlatformKeyMap::DomKeyFromKeyboardCodeImpl converts a key code to a charactor code by using the current keyboard layout information. PlatformKeyMap::UpdateLayout builds mapping table by using ToUnicodeEx. Issue 1585193002: Build key map DomCodeToKey() for Windows - Code Review https://codereview.chromium.org/1585193002
Firefox implementations looks quite complicated. I can't understand them. Firefox also has the similer code to build a mapping table by using ToUnicodeEx in KeyboardLayout::LoadLayout. But, I can't understand how this table is used. It seems that Firefox is mapping a key code by using the following WM_CHAR events for printable keys, and a hard-coded table for non-printable keys. NativeKey::GetFollowingCharMessage retrieves queued WM_CHAR messages for printable keys. KeyboardLayout::ConvertNativeKeyCodeToKeyNameIndex is converting non-printable keys. https://dxr.mozilla.org/mozilla-central/source/widget/NativeKeyToDOMKeyName.h is the hard-coded table. 842927 - Implement DOM3 KeyboardEvent.key only for non-printable key, first https://bugzilla.mozilla.org/show_bug.cgi?id=842927
I think this problem can be solved more simply. The problem is: - TranslateMessage is using ToUnicodeEx - ToUnicodeEx stores a previous dead key - extra calling ToUnicodeEx consumes the stored dead key For example, typing '`' and 'a': - Types '`' - The first calling ToUnicodeEx from TranslateMessage stores '`' as a dead key - Types 'a' - The second calling ToUnicodeEx from TranslateMessage returns 'à' as expected The problem happens if ToUnicodeEx is used after calling TranslateMessage - Types '`' - The first calling ToUnicodeEx from TranslateMessage stores '`' as a dead key - Extra calling ToUnicodeEx with '`' consumes the stored dead key, and return '``' - Types 'a' - The second calling ToUnicodeEx from TranslateMessage unexpectedly returns 'a' This problem can be solved by one more extra calling ToUnicodeEx with VK_SPACE. - Types '`' - The first calling ToUnicodeEx from TranslateMessage stores '`' as a dead key - ToUnicodeEx(VK_SPACE) consums the dead key - ToUnicodeEx('`') stores a dead key again - Types 'a' - The second calling ToUnicodeEx from TranslateMessage returns 'à' as expected https://en.wikipedia.org/wiki/Dead_key
Created attachment 380407 [details] manual test to show key and code properties of keyboard events
I tested Firefox and Chrome on Windows by typing ^ and a with German keyboard. And, marked the differences with ★. Firefox 69 for Windows type:keydown key:Dead code:Backquote type:keyup key:Dead code:Backquote type:keydown key:â code:KeyA type:keypress key:â code:KeyA type:keyup key:a code:KeyA type:keydown key:Dead code:Backquote type:keyup key:Dead code:Backquote type:keydown key:^^ code:Backquote ★ type:keypress key:^ code:Backquote type:keypress key:^ code:Backquote type:keyup key:^ code:Backquote ★ Chrome 77 for Windows type:keydown key:Dead code:Backquote type:keyup key:Dead code:Backquote type:keydown key:â code:KeyA type:keypress key:â code:KeyA type:keyup key:a code:KeyA type:keydown key:Dead code:Backquote type:keyup key:Dead code:Backquote type:keydown key:Dead code:Backquote ★ type:keypress key:^ code:Backquote type:keypress key:^ code:Backquote type:keyup key:Dead code:Backquote ★
(In reply to Fujii Hironori from comment #6) > I tested Firefox and Chrome on Windows by typing ^ and a with German Oops. I typed [^] [a] [^] [^].
comment 4 is wrong. If I type [^][^][a]. It should be "^^a", but it becomes "^^â" because the second [^] also stores a dead key.
If I set bit 2 of wFlags for ToUnicodeEx, it seems that ToUnicodeEx doesn't change the stored dead key. https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-tounicodeex > If bit 2 is set, keyboard state is not changed (Windows 10, version 1607 and newer) Unfortunately, this behavior is supported only by Windows 10, version 1607 and newer. And, it's not obvious to me how should I use this flag. For example, If I type [^][a], TranslateMessage of [a] consumes the stored dead key. So, calling ToUnicodeEx with [a] and the flag returns "a" rather than "â" because no dead key is stored. Should I use ToUnicodeEx before TranslateMessage? Or, shouldn't I use TranslateMessage?
In Chromium, InputMethodWinBase::DispatchKeyEvent retrieves queued WM_CHAR events, and sets a char information from WM_CHAR to the keydown event. Callstack: > content.dll!ui::MakeWebKeyboardEventFromUiEvent(const ui::KeyEvent & event) Line 115 C++ > content.dll!ui::MakeWebKeyboardEvent(const ui::KeyEvent & event) Line 332 C++ > content.dll!content::NativeWebKeyboardEvent::NativeWebKeyboardEvent(const ui::KeyEvent & key_event, wchar_t character) Line 132 C++ > content.dll!content::RenderWidgetHostViewAura::InsertChar(const ui::KeyEvent & event) Line 1270 C++ > ui_base_ime_win.dll!ui::InputMethodWinBase::OnChar(HWND__ * window_handle, unsigned int message, unsigned __int64 wparam, __int64 lparam, const tagMSG & event, int * handled) Line 294 C++ > ui_base_ime_win.dll!ui::InputMethodWinBase::ProcessUnhandledKeyEvent(ui::KeyEvent * event, const std::__1::vector<tagMSG,std::__1::allocator<tagMSG>> * char_msgs) Line 503 C++ > ui_base_ime_win.dll!ui::InputMethodWinBase::DispatchKeyEvent(ui::KeyEvent * event) Line 255 C++ > aura.dll!aura::WindowEventDispatcher::PreDispatchKeyEvent(ui::KeyEvent * event) Line 1036 C++ > aura.dll!aura::WindowEventDispatcher::PreDispatchEvent(ui::EventTarget * target, ui::Event * event) Line 535 C++ > events.dll!ui::EventDispatcherDelegate::DispatchEvent(ui::EventTarget * target, ui::Event * event) Line 55 C++ > events.dll!ui::EventProcessor::OnEventFromSource(ui::Event * event) Line 49 C++ > events.dll!ui::EventSource::DeliverEventToSink(ui::Event * event) Line 112 C++ > events.dll!ui::EventSource::SendEventToSinkFromRewriter(const ui::Event * event, const ui::EventRewriter * rewriter) Line 137 C++ > events.dll!ui::EventSource::SendEventToSink(const ui::Event * event) Line 106 C++ > views.dll!views::DesktopWindowTreeHostWin::HandleKeyEvent(ui::KeyEvent * event) Line 934 C++ > views.dll!views::HWNDMessageHandler::OnKeyEvent(unsigned int message, unsigned __int64 w_param, __int64 l_param) Line 1912 C++ > views.dll!views::HWNDMessageHandler::_ProcessWindowMessage(HWND__ * hWnd, unsigned int uMsg, unsigned __int64 wParam, __int64 lParam, __int64 & lResult, unsigned long dwMsgMapID) Line 380 C++ > views.dll!views::HWNDMessageHandler::OnWndProc(unsigned int message, unsigned __int64 w_param, __int64 l_param) Line 1024 C++ > gfx.dll!gfx::WindowImpl::WndProc(HWND__ * hwnd, unsigned int message, unsigned __int64 w_param, __int64 l_param) Line 298 C++ > gfx.dll!base::win::WrappedWindowProc<&gfx::WindowImpl::WndProc>(HWND__ * hwnd, unsigned int message, unsigned __int64 wparam, __int64 lparam) Line 75 C++ > [External Code] > base.dll!base::MessagePumpForUI::ProcessMessageHelper(const tagMSG & msg) Line 476 C++ > base.dll!base::MessagePumpForUI::ProcessNextWindowsMessage() Line 445 C++ > base.dll!base::MessagePumpForUI::DoRunLoop() Line 212 C++ > base.dll!base::MessagePumpWin::Run(base::MessagePump::Delegate * delegate) Line 76 C++ > base.dll!base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool application_tasks_allowed, base::TimeDelta timeout) Line 467 C++ > base.dll!base::RunLoop::Run() Line 156 C++ > chrome.dll!ChromeBrowserMainParts::MainMessageLoopRun(int * result_code) Line 1854 C++ > content.dll!content::BrowserMainLoop::RunMainMessageLoopParts() Line 1026 C++ > content.dll!content::BrowserMainRunnerImpl::Run() Line 149 C++ > content.dll!content::BrowserMain(const content::MainFunctionParams & parameters) Line 47 C++ > content.dll!content::RunBrowserProcessMain(const content::MainFunctionParams & main_function_params, content::ContentMainDelegate * delegate) Line 556 C++ > content.dll!content::ContentMainRunnerImpl::RunServiceManager(content::MainFunctionParams & main_params, bool start_service_manager_only) Line 963 C++ > content.dll!content::ContentMainRunnerImpl::Run(bool start_service_manager_only) Line 871 C++ > content.dll!content::ContentServiceManagerMainDelegate::RunEmbedderProcess() Line 52 C++ > embedder.dll!service_manager::Main(const service_manager::MainParams & params) Line 423 C++ > content.dll!content::ContentMain(const content::ContentMainParams & params) Line 20 C++ > chrome.dll!ChromeMain(HINSTANCE__ * instance, sandbox::SandboxInterfaceInfo * sandbox_info, __int64 exe_entry_point_ticks) Line 110 C++ > chrome.exe!MainDllLoader::Launch(HINSTANCE__ * instance, base::TimeTicks exe_entry_point_ticks) Line 202 C++ > chrome.exe!wWinMain(HINSTANCE__ * instance, HINSTANCE__ * prev, wchar_t *, int) Line 234 C++ > [External Code] Issue 1267483003: Combine the WM_CHAR with WM_KEY* for key event flow. - Code Review https://codereview.chromium.org/1267483003
I typed [^][a][^z][^][^] in German keyboard. Firefox 69 for Windows type:keydown key:Dead code:Backquote type:keyup key:Dead code:Backquote type:keydown key:â code:KeyA type:keypress key:â code:KeyA type:keyup key:a code:KeyA type:keydown key:Dead code:Backquote type:keyup key:Dead code:Backquote type:keydown key:^y code:KeyZ type:keypress key:^ code:KeyZ type:keypress key:y code:KeyZ type:keyup key:y code:KeyZ type:keydown key:Dead code:Backquote type:keyup key:Dead code:Backquote type:keydown key:^^ code:Backquote type:keypress key:^ code:Backquote type:keypress key:^ code:Backquote type:keyup key:^ code:Backquote Chrome 77 for Windows type:keydown key:Dead code:Backquote type:keyup key:Dead code:Backquote type:keydown key:â code:KeyA type:keypress key:â code:KeyA type:keyup key:a code:KeyA type:keydown key:Dead code:Backquote type:keyup key:Dead code:Backquote type:keydown key:y code:KeyZ type:keypress key:^ code:KeyZ type:keypress key:y code:KeyZ type:keyup key:y code:KeyZ type:keydown key:Dead code:Backquote type:keyup key:Dead code:Backquote type:keydown key:Dead code:Backquote type:keypress key:^ code:Backquote type:keypress key:^ code:Backquote type:keyup key:Dead code:Backquote Microsoft Edge 44.18362.387.0 (Microsoft EdgeHTML 18.18362) type:keydown key:Unidentified code:undefined type:keyup key:Unidentified code:undefined type:keydown key:â code:undefined type:keypress key:â code:undefined type:keyup key:â code:undefined type:keydown key:Unidentified code:undefined type:keyup key:Unidentified code:undefined type:keydown key:^y code:undefined type:keypress key:^y code:undefined type:keyup key:^y code:undefined type:keydown key:Unidentified code:undefined type:keyup key:Unidentified code:undefined type:keydown key:^^ code:undefined type:keypress key:^^ code:undefined type:keyup key:^^ code:undefined
(In reply to Fujii Hironori from comment #11) (...) > type:keydown key:^y code:KeyZ (...) > type:keydown key:^y code:undefined > type:keypress key:^y code:undefined > type:keyup key:^y code:undefined Firefox and Edge set two base characters for 'key' property. This seems incompatible with the spec. https://www.w3.org/TR/uievents-key/#keys-unicode > A key string is a string containing a 0 or 1 non-control > characters ("base" characters) followed by 0 or more combining > characters. The string MUST be in Normalized Form C (NFC) as > described in [UnicodeNormalizationForms].
I tested on macOS with https://javascript.info/keyboard-events Chrome 77 on macOS 10.14 keydown key=Dead code=Equal keyup key=Dead code=Equal -------------------------------------------------------------------------------- keydown key=á code=KeyA keyup key=a code=KeyA -------------------------------------------------------------------------------- keydown key=Dead code=Equal keyup key=Dead code=Equal -------------------------------------------------------------------------------- keydown key=y code=KeyZ keyup key=y code=KeyZ -------------------------------------------------------------------------------- keydown key=Dead code=Equal keyup key=Dead code=Equal -------------------------------------------------------------------------------- keydown key=Dead code=Equal keyup key=Dead code=Equal Safari Technology Preview 93 on macOS 10.14 keydown key=Dead code=Equal keyup key=´ code=Equal -------------------------------------------------------------------------------- keydown key=á code=KeyA keyup key=a code=KeyA -------------------------------------------------------------------------------- keydown key=Dead code=Equal keyup key=´ code=Equal -------------------------------------------------------------------------------- keydown key=´y code=KeyZ keyup key=y code=KeyZ -------------------------------------------------------------------------------- keydown key=Dead code=Equal keyup key=´ code=Equal -------------------------------------------------------------------------------- keydown key=´ code=Equal keyup key=´ code=Equal
Created attachment 382570 [details] WIP patch
Created attachment 382816 [details] WIP patch
Created attachment 383018 [details] WIP patch
Created attachment 383338 [details] WIP patch
Created attachment 383438 [details] WIP patch
Created attachment 383542 [details] Patch
review?
Comment on attachment 383542 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383542&action=review > Source/WebCore/platform/win/WindowsKeyNames.cpp:39 > +typedef enum : uint8_t { > + EF_NONE = 0, > + EF_SHIFT_DOWN = 1 << 0, > + EF_CONTROL_DOWN = 1 << 1, > + EF_ALT_DOWN = 1 << 2, > + EF_CAPS_LOCK_ON = 1 << 3, > +} EventFlags; Might be neat to use OptionSet here...unless the goal is to keep this file as similar as possible to the Chromium copy, in which case it's fine as-is. > Source/WebCore/platform/win/WindowsKeyNames.cpp:209 > + return nullString(); Interesting, I didn't realize this existed. There are only 5 instances of `return nullString();` in Source/ compared to 731 of `return String();`... I guess it was introduced early this year for JSStrings: https://github.com/WebKit/webkit/commit/cb663b3dd21a5b9fbdd180625406226f851a1c7e According to Yusuke on IRC: "nullString is used almost only when you want `const String&` allocated in somewhere." > Source/WebCore/platform/win/WindowsKeyNames.cpp:311 > + > + Nit: extra newline > Source/WebCore/platform/win/WindowsKeyNames.cpp:341 > + } domKeyMap[] = { Seems odd that domKeyMap is defined *inside* domCodeFromLParam while cNonPrintableKeyMap is defined *outside* of nonPrintableVirtualKeyToDomKey. > Source/WebCore/platform/win/WindowsKeyNames.h:31 > +#pragma once > +#include <windows.h> Nit: missing newline > Source/WebCore/platform/win/WindowsKeyNames.h:49 > + typedef std::pair<unsigned /*VirtualKey*/, unsigned /*EventFlags*/> VirtualKeyEventFlagsPair; Would be nice to update typedefs to usings, again assuming we're not trying to minimize edits to this file.
Created attachment 384037 [details] Patch * Addressed the review feedbacks.
Comment on attachment 383542 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383542&action=review >> Source/WebCore/platform/win/WindowsKeyNames.cpp:39 >> +} EventFlags; > > Might be neat to use OptionSet here...unless the goal is to keep this file as similar as possible to the Chromium copy, in which case it's fine as-is. Good idea. >> Source/WebCore/platform/win/WindowsKeyNames.cpp:209 >> + return nullString(); > > Interesting, I didn't realize this existed. > There are only 5 instances of `return nullString();` in Source/ compared to 731 of `return String();`... > I guess it was introduced early this year for JSStrings: https://github.com/WebKit/webkit/commit/cb663b3dd21a5b9fbdd180625406226f851a1c7e > > According to Yusuke on IRC: "nullString is used almost only when you want `const String&` allocated in somewhere." Interesting. I think nullString() is just a nullptr of WTF::StringImpl. Replaced.
Comment on attachment 384037 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384037&action=review > Source/WebCore/platform/win/WindowsKeyNames.h:39 > +template<typename T> struct DefaultHash<OptionSet<T>> { Hmm, wouldn't this need to go in wtf/OptionSet.h?
Comment on attachment 384037 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384037&action=review >> Source/WebCore/platform/win/WindowsKeyNames.h:39 >> +template<typename T> struct DefaultHash<OptionSet<T>> { > > Hmm, wouldn't this need to go in wtf/OptionSet.h? Fair enough. I should do it. Filed: Bug 204562 – Add DefaultHash<OptionSet<T>> and HashTrait<OptionSet<T>> specializations
Created attachment 384320 [details] Patch
Comment on attachment 384320 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384320&action=review r=me > Source/WebCore/platform/win/WindowsKeyNames.cpp:194 > + // VK_KANA isn't generated by any modern layouts but is a listed value > + // that third-party apps might synthesize, so we handle it anyway. Style nit: indentation > Source/WebCore/platform/win/WindowsKeyNames.cpp:274 > + KeyModifierSet({KeyModifier::Shift, KeyModifier::CapsLock}) & modifiers, Style nit: spaces inside braces > Source/WebCore/platform/win/WindowsKeyNames.cpp:522 > + m_printableKeyCodeToKey.set(std::make_pair(virtualKey, modifiers), makeString(UChar(translatedChars[0]))); Seems like we should use WTF::ucharFrom...but then it's only one character. (Then again, I guess if it weren't just one character then you wouldn't need makeString either, since there's String(const wchar_t*). Oh well.)
Comment on attachment 384320 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384320&action=review Thank you very much for the review. >> Source/WebCore/platform/win/WindowsKeyNames.cpp:522 >> + m_printableKeyCodeToKey.set(std::make_pair(virtualKey, modifiers), makeString(UChar(translatedChars[0]))); > > Seems like we should use WTF::ucharFrom...but then it's only one character. > (Then again, I guess if it weren't just one character then you wouldn't need makeString either, since there's String(const wchar_t*). Oh well.) Yes, it's a single character. Will replace it with String(translatedChars, 1).
Created attachment 384321 [details] Patch for landing
Comment on attachment 384321 [details] Patch for landing Clearing flags on attachment: 384321 Committed r252873: <https://trac.webkit.org/changeset/252873>
All reviewed patches have been landed. Closing bug.
<rdar://problem/57482382>