[windows] Sends complete key data to framework #17577
Conversation
…tter#16549)" This reverts commit 6aacf5e.
|
|
||
| // Variable used to store the key code preceding a WM_CHAR message. For more | ||
| // info, read the comments below in WM_KEYDOWN/UP. | ||
| static unsigned int keycode_for_char_message = 0; |
There was a problem hiding this comment.
This static makes me a little nervous, but I know it's common practice in Windows, and the changes should only happen on the UI thread, so there's no locking needed.
There was a problem hiding this comment.
Why is it static rather than an ivar?
There was a problem hiding this comment.
Yeah an ivar makes more sense. Done
| // is persisted in a static variable keycode_for_char_message obtained | ||
| // from WM_KEYDOWN. | ||
| const unsigned int scancode = (lparam >> 16) & 0xff; | ||
| window->OnKey(keycode_for_char_message, scancode, WM_KEYDOWN, code_point); |
There was a problem hiding this comment.
Should keycode_for_char_message be reset to zero here?
There was a problem hiding this comment.
I think it would always be updated at this point, since WM_KEYDOWN is called before WM_CHAR, but it doesn't hurt to be extra cautious. Added it.
|
|
||
| // Variable used to store the key code preceding a WM_CHAR message. For more | ||
| // info, read the comments below in WM_KEYDOWN/UP. | ||
| static unsigned int keycode_for_char_message = 0; |
There was a problem hiding this comment.
Why is it static rather than an ivar?
| // is persisted in a static variable keycode_for_char_message obtained | ||
| // from WM_KEYDOWN. | ||
| const unsigned int scancode = (lparam >> 16) & 0xff; | ||
| window->OnKey(keycode_for_char_message, scancode, WM_KEYDOWN, code_point); |
There was a problem hiding this comment.
I'm confused about the complex character flow. First we send the initial code point, then later we send the combined code point? And sending the first part twice, effectively, doesn't break anything?
If that's correct behavior, can you maybe add a comment pointing to the relevant framework code explaining/showing why?
There was a problem hiding this comment.
Just added a check to avoid sending the dead key code point, and some comments explaining the flow. Is this better?
There was a problem hiding this comment.
The dead key check seems different. Or is a dead key always a leading surrogate?
There was a problem hiding this comment.
Capturing from offline discussion: dead keys and surrogate pairs are orthogonal. The decision was that we should add a check to only send OnKey once even if we get multiple WM_CHAR messages, which should address my concern here.
| // is persisted in a static variable keycode_for_char_message obtained | ||
| // from WM_KEYDOWN. | ||
| const unsigned int scancode = (lparam >> 16) & 0xff; | ||
| window->OnKey(keycode_for_char_message, scancode, WM_KEYDOWN, code_point); |
There was a problem hiding this comment.
The dead key check seems different. Or is a dead key always a leading surrogate?
franciscojma86
added
the
waiting for tree to go green
Synchronizes the win32 key press messages to send a Flutter message with all the key information (key code, scan code, code point, modifiers).
This change depends on flutter/flutter#54227
Part of flutter/flutter#37710