From fdc78e4146952e9c4e24554098b97ec9caa3a3fa Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 1 Dec 2021 00:49:44 -0800 Subject: [PATCH] [Win32, keyboard] Fix dead key events that don't have the dead key mask (flutter/engine#30004) This PR fixes flutter/flutter#92654, a rare case where dead key events are not properly handled. --- .../ci/licenses_golden/licenses_flutter | 1 + .../flutter/shell/platform/windows/BUILD.gn | 1 + .../windows/keyboard_key_channel_handler.cc | 14 +-- .../windows/keyboard_key_embedder_handler.cc | 14 +-- .../platform/windows/keyboard_key_handler.cc | 3 +- .../platform/windows/keyboard_unittests.cc | 107 +++++++++++++++++- .../platform/windows/keyboard_win32_common.h | 29 +++++ .../shell/platform/windows/window_win32.cc | 19 ++-- 8 files changed, 155 insertions(+), 33 deletions(-) create mode 100644 engine/src/flutter/shell/platform/windows/keyboard_win32_common.h diff --git a/engine/src/flutter/ci/licenses_golden/licenses_flutter b/engine/src/flutter/ci/licenses_golden/licenses_flutter index d06e2485d9..9fc5b5dc5c 100755 --- a/engine/src/flutter/ci/licenses_golden/licenses_flutter +++ b/engine/src/flutter/ci/licenses_golden/licenses_flutter @@ -1759,6 +1759,7 @@ FILE: ../../../flutter/shell/platform/windows/keyboard_key_handler.cc FILE: ../../../flutter/shell/platform/windows/keyboard_key_handler.h FILE: ../../../flutter/shell/platform/windows/keyboard_key_handler_unittests.cc FILE: ../../../flutter/shell/platform/windows/keyboard_unittests.cc +FILE: ../../../flutter/shell/platform/windows/keyboard_win32_common.h FILE: ../../../flutter/shell/platform/windows/platform_handler.cc FILE: ../../../flutter/shell/platform/windows/platform_handler.h FILE: ../../../flutter/shell/platform/windows/platform_handler_unittests.cc diff --git a/engine/src/flutter/shell/platform/windows/BUILD.gn b/engine/src/flutter/shell/platform/windows/BUILD.gn index f8968126f0..5b5f468865 100644 --- a/engine/src/flutter/shell/platform/windows/BUILD.gn +++ b/engine/src/flutter/shell/platform/windows/BUILD.gn @@ -72,6 +72,7 @@ source_set("flutter_windows_source") { "keyboard_key_embedder_handler.h", "keyboard_key_handler.cc", "keyboard_key_handler.h", + "keyboard_win32_common.h", "platform_handler.cc", "platform_handler.h", "sequential_id_generator.cc", diff --git a/engine/src/flutter/shell/platform/windows/keyboard_key_channel_handler.cc b/engine/src/flutter/shell/platform/windows/keyboard_key_channel_handler.cc index a3fefd800e..61a9e7c8a1 100644 --- a/engine/src/flutter/shell/platform/windows/keyboard_key_channel_handler.cc +++ b/engine/src/flutter/shell/platform/windows/keyboard_key_channel_handler.cc @@ -14,6 +14,7 @@ #include #include "flutter/shell/platform/common/json_message_codec.h" +#include "flutter/shell/platform/windows/keyboard_win32_common.h" namespace flutter { @@ -146,17 +147,6 @@ int GetModsForKeyState() { #endif } -// Revert the "character" for a dead key to its normal value, or the argument -// unchanged otherwise. -// -// When a dead key is pressed, the WM_KEYDOWN's lParam is mapped to a special -// value: the "normal character" | 0x80000000. For example, when pressing -// "dead key caret" (one that makes the following e into ê), its mapped -// character is 0x8000005E. "Reverting" it gives 0x5E, which is character '^'. -uint32_t _UndeadChar(uint32_t ch) { - return ch & ~0x80000000; -} - } // namespace KeyboardKeyChannelHandler::KeyboardKeyChannelHandler( @@ -184,7 +174,7 @@ void KeyboardKeyChannelHandler::KeyboardHook( event.AddMember(kKeyCodeKey, key, allocator); event.AddMember(kScanCodeKey, scancode | (extended ? kScancodeExtended : 0), allocator); - event.AddMember(kCharacterCodePointKey, _UndeadChar(character), allocator); + event.AddMember(kCharacterCodePointKey, UndeadChar(character), allocator); event.AddMember(kKeyMapKey, kWindowsKeyMap, allocator); event.AddMember(kModifiersKey, GetModsForKeyState(), allocator); diff --git a/engine/src/flutter/shell/platform/windows/keyboard_key_embedder_handler.cc b/engine/src/flutter/shell/platform/windows/keyboard_key_embedder_handler.cc index abca2003c6..d28ee0a38b 100644 --- a/engine/src/flutter/shell/platform/windows/keyboard_key_embedder_handler.cc +++ b/engine/src/flutter/shell/platform/windows/keyboard_key_embedder_handler.cc @@ -12,6 +12,7 @@ #include #include +#include "flutter/shell/platform/windows/keyboard_win32_common.h" #include "flutter/shell/platform/windows/string_conversion.h" namespace flutter { @@ -28,17 +29,6 @@ constexpr SHORT kStateMaskPressed = 0x80; const char* empty_character = ""; -// Revert the "character" for a dead key to its normal value, or the argument -// unchanged otherwise. -// -// When a dead key is pressed, the WM_KEYDOWN's lParam is mapped to a special -// value: the "normal character" | 0x80000000. For example, when pressing -// "dead key caret" (one that makes the following e into ê), its mapped -// character is 0x8000005E. "Reverting" it gives 0x5E, which is character '^'. -uint32_t _UndeadChar(uint32_t ch) { - return ch & ~0x80000000; -} - // Get some bits of the char, from the start'th bit from the right (excluded) // to the end'th bit from the right (included). // @@ -185,7 +175,7 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl( uint64_t eventual_logical_record; char character_bytes[kCharacterCacheSize]; - character = _UndeadChar(character); + character = UndeadChar(character); if (is_physical_down) { if (had_record) { diff --git a/engine/src/flutter/shell/platform/windows/keyboard_key_handler.cc b/engine/src/flutter/shell/platform/windows/keyboard_key_handler.cc index 60149f3617..f2d4ce94af 100644 --- a/engine/src/flutter/shell/platform/windows/keyboard_key_handler.cc +++ b/engine/src/flutter/shell/platform/windows/keyboard_key_handler.cc @@ -9,6 +9,7 @@ #include #include "flutter/shell/platform/common/json_message_codec.h" +#include "flutter/shell/platform/windows/keyboard_win32_common.h" namespace flutter { @@ -20,7 +21,7 @@ static constexpr int kMaxPendingEvents = 1000; // Returns if a character sent by Win32 is a dead key. bool _IsDeadKey(uint32_t ch) { - return (ch & 0x80000000) != 0; + return (ch & kDeadKeyCharMask) != 0; } // Returns true if this key is a key down event of ShiftRight. diff --git a/engine/src/flutter/shell/platform/windows/keyboard_unittests.cc b/engine/src/flutter/shell/platform/windows/keyboard_unittests.cc index a58704acee..8ee9c1d914 100644 --- a/engine/src/flutter/shell/platform/windows/keyboard_unittests.cc +++ b/engine/src/flutter/shell/platform/windows/keyboard_unittests.cc @@ -349,6 +349,7 @@ constexpr uint64_t kScanCodeKeyE = 0x12; constexpr uint64_t kScanCodeKeyQ = 0x10; constexpr uint64_t kScanCodeKeyW = 0x11; constexpr uint64_t kScanCodeDigit1 = 0x02; +constexpr uint64_t kScanCodeDigit6 = 0x07; // constexpr uint64_t kScanCodeNumpad1 = 0x4f; // constexpr uint64_t kScanCodeNumLock = 0x45; constexpr uint64_t kScanCodeControl = 0x1d; @@ -860,8 +861,112 @@ TEST(KeyboardTest, DeadKeyThatCombines) { EXPECT_EQ(key_calls.size(), 0); } +// This tests dead key ^ then E on a US INTL keyboard, which should be combined +// into ê. +// +// It is different from French AZERTY because the character that the ^ key is +// mapped to does not contain the dead key character somehow. +TEST(KeyboardTest, DeadKeyWithoutDeadMaskThatCombines) { + KeyboardTester tester; + tester.Responding(false); + + // Press ShiftLeft + tester.SetKeyState(VK_LSHIFT, true, true); + tester.InjectMessages( + 1, + WmKeyDownInfo{VK_SHIFT, kScanCodeShiftLeft, kNotExtended, kWasUp}.Build( + kWmResultZero)); + + EXPECT_EQ(key_calls.size(), 1); + EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeDown, + kPhysicalShiftLeft, kLogicalShiftLeft, "", + kNotSynthesized); + clear_key_calls(); + + tester.InjectPendingEvents(); + EXPECT_EQ(key_calls.size(), 0); + clear_key_calls(); + + // Press 6^ + tester.InjectMessages( + 2, + WmKeyDownInfo{'6', kScanCodeDigit6, kNotExtended, kWasUp}.Build( + kWmResultZero), + WmDeadCharInfo{'^', kScanCodeDigit6, kNotExtended, kWasUp}.Build( + kWmResultZero)); + + EXPECT_EQ(key_calls.size(), 1); + EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeDown, kPhysicalDigit6, + kLogicalDigit6, "6", kNotSynthesized); + clear_key_calls(); + + EXPECT_EQ(tester.InjectPendingEvents(), 0); + EXPECT_EQ(key_calls.size(), 0); + clear_key_calls(); + + // Release 6^ + tester.InjectMessages( + 1, WmKeyUpInfo{'6', kScanCodeDigit6, kNotExtended}.Build(kWmResultZero)); + + EXPECT_EQ(key_calls.size(), 1); + EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeUp, kPhysicalDigit6, + kLogicalDigit6, "", kNotSynthesized); + clear_key_calls(); + + tester.InjectPendingEvents(); + EXPECT_EQ(key_calls.size(), 0); + clear_key_calls(); + + // Release ShiftLeft + tester.SetKeyState(VK_LSHIFT, false, true); + tester.InjectMessages( + 1, WmKeyUpInfo{VK_SHIFT, kScanCodeShiftLeft, kNotExtended}.Build( + kWmResultZero)); + + EXPECT_EQ(key_calls.size(), 1); + EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeUp, kPhysicalShiftLeft, + kLogicalShiftLeft, "", kNotSynthesized); + clear_key_calls(); + + tester.InjectPendingEvents(); + EXPECT_EQ(key_calls.size(), 0); + clear_key_calls(); + + // Press E + tester.InjectMessages( + 2, + WmKeyDownInfo{kVirtualKeyE, kScanCodeKeyE, kNotExtended, kWasUp}.Build( + kWmResultZero), + WmCharInfo{0xEA, kScanCodeKeyE, kNotExtended, kWasUp}.Build( + kWmResultZero)); + + EXPECT_EQ(key_calls.size(), 1); + EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeDown, kPhysicalKeyE, + kLogicalKeyE, "ê", kNotSynthesized); + clear_key_calls(); + + tester.InjectPendingEvents( + 0xEA); // The redispatched event uses unmodified 'e' + EXPECT_EQ(key_calls.size(), 1); + EXPECT_CALL_IS_TEXT(key_calls[0], u"ê"); + clear_key_calls(); + + // Release E + tester.InjectMessages( + 1, WmKeyUpInfo{kVirtualKeyE, kScanCodeKeyE, kNotExtended}.Build( + kWmResultZero)); + + EXPECT_EQ(key_calls.size(), 1); + EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeUp, kPhysicalKeyE, + kLogicalKeyE, "", kNotSynthesized); + clear_key_calls(); + + tester.InjectPendingEvents(); + EXPECT_EQ(key_calls.size(), 0); +} + // This tests dead key ^ then & (US: 1) on a French keyboard, which do not -// combine and should output "^$". +// combine and should output "^&". TEST(KeyboardTest, DeadKeyThatDoesNotCombine) { KeyboardTester tester; tester.Responding(false); diff --git a/engine/src/flutter/shell/platform/windows/keyboard_win32_common.h b/engine/src/flutter/shell/platform/windows/keyboard_win32_common.h new file mode 100644 index 0000000000..5bd6a25c56 --- /dev/null +++ b/engine/src/flutter/shell/platform/windows/keyboard_win32_common.h @@ -0,0 +1,29 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_SHELL_PLATFORM_WINDOWS_KEYBOARD_WIN32_COMMON_H_ +#define FLUTTER_SHELL_PLATFORM_WINDOWS_KEYBOARD_WIN32_COMMON_H_ + +#include + +namespace flutter { + +// The bit of a mapped character in a WM_KEYDOWN message that indicates the +// character is a dead key. +// +// When a dead key is pressed, the WM_KEYDOWN's lParam is mapped to a special +// value: the "normal character" | 0x80000000. For example, when pressing +// "dead key caret" (one that makes the following e into ê), its mapped +// character is 0x8000005E. "Reverting" it gives 0x5E, which is character '^'. +constexpr int kDeadKeyCharMask = 0x80000000; + +// Revert the "character" for a dead key to its normal value, or the argument +// unchanged otherwise. +inline uint32_t UndeadChar(uint32_t ch) { + return ch & ~kDeadKeyCharMask; +} + +} // namespace flutter + +#endif // FLUTTER_SHELL_PLATFORM_WINDOWS_KEYBOARD_WIN32_COMMON_H_ diff --git a/engine/src/flutter/shell/platform/windows/window_win32.cc b/engine/src/flutter/shell/platform/windows/window_win32.cc index a9e37ef9ae..1a1d6ef57e 100644 --- a/engine/src/flutter/shell/platform/windows/window_win32.cc +++ b/engine/src/flutter/shell/platform/windows/window_win32.cc @@ -15,6 +15,7 @@ #include #include "dpi_utils_win32.h" +#include "keyboard_win32_common.h" namespace flutter { @@ -514,14 +515,18 @@ WindowWin32::HandleMessage(UINT const message, const bool extended = ((lparam >> 24) & 0x01) == 0x01; const bool was_down = lparam & 0x40000000; // Certain key combinations yield control characters as WM_CHAR's - // lParam. For example, 0x01 for Ctrl-A. Filter these characters. - // See + // lParam. For example, 0x01 for Ctrl-A. Filter these characters. See // https://docs.microsoft.com/en-us/windows/win32/learnwin32/accelerator-tables - const char32_t event_character = - (message == WM_DEADCHAR || message == WM_SYSDEADCHAR) - ? Win32MapVkToChar(keycode_for_char_message_) - : IsPrintable(code_point) ? code_point - : 0; + char32_t event_character; + if (message == WM_DEADCHAR || message == WM_SYSDEADCHAR) { + // Mask the resulting char with kDeadKeyCharMask anyway, because in + // rare cases the bit is *not* set (US INTL Shift-6 circumflex, see + // https://github.com/flutter/flutter/issues/92654 .) + event_character = + Win32MapVkToChar(keycode_for_char_message_) | kDeadKeyCharMask; + } else { + event_character = IsPrintable(code_point) ? code_point : 0; + } bool handled = OnKey(keycode_for_char_message_, scancode, message == WM_SYSCHAR ? WM_SYSKEYDOWN : WM_KEYDOWN, event_character, extended, was_down);