Skip to content

Commit

Permalink
[Window, Keyboard] Fix repeat events of modifier keys (flutter#35046)
Browse files Browse the repository at this point in the history
  • Loading branch information
dkwingsmt authored and betrevisan committed Aug 5, 2022
1 parent 012a88e commit 28c9a26
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 7 deletions.
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

0 comments on commit 28c9a26

Please sign in to comment.