Skip to content
This repository has been archived by the owner on Feb 25, 2025. It is now read-only.

[Window, Keyboard] Fix repeat events of modifier keys #35046

Merged
merged 5 commits into from
Aug 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions shell/platform/windows/keyboard_key_embedder_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl(

const bool is_event_down = action == WM_KEYDOWN || action == WM_SYSKEYDOWN;

bool event_key_can_be_repeat = true;
UpdateLastSeenCritialKey(key, physical_key, sequence_logical_key);
// Synchronize the toggled states of critical keys (such as whether CapsLocks
// is enabled). Toggled states can only be changed upon a down event, so if
Expand All @@ -197,14 +198,15 @@ void KeyboardKeyEmbedderHandler::KeyboardHookImpl(
// updated to the true state, while the critical keys whose toggled state have
// been changed will be pressed regardless of their true pressed state.
// Updating the pressed state will be done by SynchronizeCritialPressedStates.
SynchronizeCritialToggledStates(key, is_event_down);
SynchronizeCritialToggledStates(key, is_event_down, &event_key_can_be_repeat);
// Synchronize the pressed states of critical keys (such as whether CapsLocks
// is pressed).
//
// After this function, all critical keys except for the target key will have
// their toggled state and pressed state matched with their true states. The
// target key's pressed state will be updated immediately after this.
SynchronizeCritialPressedStates(key, physical_key, is_event_down);
SynchronizeCritialPressedStates(key, physical_key, is_event_down,
event_key_can_be_repeat);

// The resulting event's `type`.
FlutterKeyEventType type;
Expand Down Expand Up @@ -340,7 +342,8 @@ void KeyboardKeyEmbedderHandler::UpdateLastSeenCritialKey(

void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates(
int event_virtual_key,
bool is_event_down) {
bool is_event_down,
bool* event_key_can_be_repeat) {
// NowState ----------------> PreEventState --------------> TrueState
// Synchronization Event
for (auto& kv : critical_keys_) {
Expand Down Expand Up @@ -385,6 +388,7 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates(
key_info.physical_key,
key_info.logical_key, empty_character),
nullptr, nullptr);
*event_key_can_be_repeat = false;
}
key_info.toggled_on = true_toggled;
}
Expand All @@ -394,7 +398,8 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialToggledStates(
void KeyboardKeyEmbedderHandler::SynchronizeCritialPressedStates(
int event_virtual_key,
int event_physical_key,
bool is_event_down) {
bool is_event_down,
bool event_key_can_be_repeat) {
// During an incoming event, there might be a synthesized Flutter event for
// each key of each pressing goal, followed by an eventual main Flutter
// event.
Expand Down Expand Up @@ -424,8 +429,18 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialPressedStates(
if (is_event_down) {
// For down events, this key is the event key if they have the same
// virtual key, because virtual key represents "functionality."
//
// In that case, normally Flutter should synthesize nothing since the
// resulting event can adapt to the current state by dispatching either
// a down or a repeat event. However, in certain cases (when Flutter has
// just synchronized the key's toggling state) the event must not be a
// repeat event.
if (virtual_key == event_virtual_key) {
pre_event_pressed = false;
if (event_key_can_be_repeat) {
continue;
} else {
pre_event_pressed = false;
}
}
} else {
// For up events, this key is the event key if they have the same
Expand Down
6 changes: 4 additions & 2 deletions shell/platform/windows/keyboard_key_embedder_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,14 @@ class KeyboardKeyEmbedderHandler
// Check each key's state from |get_key_state_| and synthesize events
// if their toggling states have been desynchronized.
void SynchronizeCritialToggledStates(int event_virtual_key,
bool is_event_down);
bool is_event_down,
bool* event_key_can_be_repeat);
// Check each key's state from |get_key_state_| and synthesize events
// if their pressing states have been desynchronized.
void SynchronizeCritialPressedStates(int event_virtual_key,
int event_physical_key,
bool is_event_down);
bool is_event_down,
bool event_key_can_be_repeat);

// Wraps perform_send_event_ with state tracking. Use this instead of
// |perform_send_event_| to send events to the framework.
Expand Down
12 changes: 12 additions & 0 deletions shell/platform/windows/keyboard_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,18 @@ TEST(KeyboardTest, ShiftLeftUnhandled) {
clear_key_calls();
EXPECT_EQ(tester.RedispatchedMessageCountAndClear(), 1);

// Hold ShiftLeft
tester.InjectKeyboardChanges(std::vector<KeyboardChange>{
WmKeyDownInfo{VK_SHIFT, kScanCodeShiftLeft, kNotExtended, kWasDown}.Build(
kWmResultZero)});

EXPECT_EQ(key_calls.size(), 1);
EXPECT_CALL_IS_EVENT(key_calls[0], kFlutterKeyEventTypeRepeat,
kPhysicalShiftLeft, kLogicalShiftLeft, "",
kNotSynthesized);
clear_key_calls();
EXPECT_EQ(tester.RedispatchedMessageCountAndClear(), 1);

// Release ShiftLeft
tester.InjectKeyboardChanges(std::vector<KeyboardChange>{
KeyStateChange{VK_LSHIFT, false, true},
Expand Down