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

Commit 7bb9b9a

Browse files
gspencergooggspencergoog
committed
Review changes
1 parent d3346cc commit 7bb9b9a

File tree

3 files changed

+94
-74
lines changed

3 files changed

+94
-74
lines changed

‎ci/licenses_golden/licenses_flutter‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1273,6 +1273,7 @@ FILE: ../../../flutter/shell/platform/linux/fl_json_method_codec.cc
12731273
FILE: ../../../flutter/shell/platform/linux/fl_json_method_codec_test.cc
12741274
FILE: ../../../flutter/shell/platform/linux/fl_key_event_plugin.cc
12751275
FILE: ../../../flutter/shell/platform/linux/fl_key_event_plugin.h
1276+
FILE: ../../../flutter/shell/platform/linux/fl_key_event_plugin_test.cc
12761277
FILE: ../../../flutter/shell/platform/linux/fl_message_codec.cc
12771278
FILE: ../../../flutter/shell/platform/linux/fl_message_codec_test.cc
12781279
FILE: ../../../flutter/shell/platform/linux/fl_method_call.cc

‎shell/platform/linux/fl_key_event_plugin.cc‎

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -81,42 +81,53 @@ void fl_key_event_plugin_send_key_event(FlKeyEventPlugin* self,
8181
int64_t scan_code = event->hardware_keycode;
8282
int64_t unicodeScalarValues = gdk_keyval_to_unicode(event->keyval);
8383

84-
// Remove lock states from state mask.
85-
guint state = event->state & ~(GDK_LOCK_MASK | GDK_MOD2_MASK);
86-
87-
// GTK keeps track of the state of the locks themselves, not the "down" state
88-
// of the key. Flutter expects the "down" state of the modifier key, so we
89-
// keep track of the down state here, and send it to the framework. This code
90-
// has the flaw that if a key event is missed due to the app losing focus,
91-
// then this state will still think the key is down when it isn't, but that is
92-
// no worse than for other keys until we implement the sync/cancel events.
84+
// For most modifier keys, GTK keeps track of the "pressed" state of the
85+
// modifier keys. Flutter uses this information to keep modifier keys from
86+
// being "stuck" when a key-up event is lost because it happens after the app
87+
// loses focus.
88+
//
89+
// For Lock keys (ShiftLock, CapsLock, NumLock), however, GTK keeps track of
90+
// the state of the locks themselves, not the "pressed" state of the key.
91+
//
92+
// Since Flutter expects the "pressed" state of the modifier keys, the lock
93+
// state for these keys is discarded here, and it is substituted for the
94+
// pressed state of the key.
95+
//
96+
// This code has the flaw that if a key event is missed due to the app losing
97+
// focus, then this state will still think the key is pressed when it isn't,
98+
// but that is no worse than for "regular" keys until we implement the
99+
// sync/cancel events on app focus changes.
93100
//
94101
// This is necessary to do here instead of in the framework because Flutter
95102
// does modifier key syncing in the framework, and will turn on/off these keys
96103
// as being "pressed" whenever the lock is on, which breaks a lot of
97-
// interactions.
104+
// interactions (for example, if shift-lock is on, tab traversal is broken).
98105
//
99106
// TODO(gspencergoog): get rid of this tracked state when we are tracking the
100107
// state of all keys and sending sync/cancel events when focus is gained/lost.
101-
static bool shift_lock_down = false;
102-
static bool caps_lock_down = false;
103-
static bool num_lock_down = false;
108+
109+
// Remove lock states from state mask.
110+
guint state = event->state & ~(GDK_LOCK_MASK | GDK_MOD2_MASK);
111+
112+
static bool shift_lock_pressed = false;
113+
static bool caps_lock_pressed = false;
114+
static bool num_lock_pressed = false;
104115
switch (event->keyval) {
105116
case GDK_KEY_Num_Lock:
106-
num_lock_down = event->type == GDK_KEY_PRESS;
117+
num_lock_pressed = event->type == GDK_KEY_PRESS;
107118
break;
108119
case GDK_KEY_Caps_Lock:
109-
caps_lock_down = event->type == GDK_KEY_PRESS;
120+
caps_lock_pressed = event->type == GDK_KEY_PRESS;
110121
break;
111122
case GDK_KEY_Shift_Lock:
112-
shift_lock_down = event->type == GDK_KEY_PRESS;
123+
shift_lock_pressed = event->type == GDK_KEY_PRESS;
113124
break;
114125
}
115126

116-
// Make the state match the actual pressed state of the lock keys, not the
117-
// lock states themselves.
118-
state |= (shift_lock_down || caps_lock_down) ? GDK_LOCK_MASK : 0x0;
119-
state |= num_lock_down ? GDK_MOD2_MASK : 0x0;
127+
// Add back in the state matching the actual pressed state of the lock keys,
128+
// not the lock states.
129+
state |= (shift_lock_pressed || caps_lock_pressed) ? GDK_LOCK_MASK : 0x0;
130+
state |= num_lock_pressed ? GDK_MOD2_MASK : 0x0;
120131

121132
g_autoptr(FlValue) message = fl_value_new_map();
122133
fl_value_set_string_take(message, kTypeKey, fl_value_new_string(type));

‎shell/platform/linux/fl_key_event_plugin_test.cc‎

Lines changed: 62 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44

55
#include "flutter/shell/platform/linux/fl_key_event_plugin.h"
66

7-
#include "gtest/gtest.h"
87
#include <iostream>
8+
#include "gtest/gtest.h"
99

1010
#include "flutter/shell/platform/linux/fl_binary_messenger_private.h"
1111
#include "flutter/shell/platform/linux/fl_engine_private.h"
@@ -51,69 +51,77 @@ TEST(FlKeyEventPluginTest, SendKeyEvent) {
5151

5252
g_autoptr(FlEngine) engine = make_mock_engine();
5353
FlBinaryMessenger* messenger = fl_binary_messenger_new(engine);
54-
g_autoptr(FlKeyEventPlugin) plugin = fl_key_event_plugin_new(messenger, echo_response_cb, "test/echo");
54+
g_autoptr(FlKeyEventPlugin) plugin =
55+
fl_key_event_plugin_new(messenger, echo_response_cb, "test/echo");
5556

5657
char string[] = "A";
5758
GdkEventKey key_event = GdkEventKey{
58-
GDK_KEY_PRESS, // event type
59-
nullptr, // window (not needed)
60-
FALSE, // event was sent explicitly
61-
12345, // time
62-
0x0, // modifier state
63-
GDK_KEY_A, // key code
64-
1, // length of string representation
65-
reinterpret_cast<gchar*>(&string[0]), // string representation
66-
0x04, // scan code
67-
0, // keyboard group
68-
0, // is a modifier
59+
GDK_KEY_PRESS, // event type
60+
nullptr, // window (not needed)
61+
FALSE, // event was sent explicitly
62+
12345, // time
63+
0x0, // modifier state
64+
GDK_KEY_A, // key code
65+
1, // length of string representation
66+
reinterpret_cast<gchar*>(&string[0]), // string representation
67+
0x04, // scan code
68+
0, // keyboard group
69+
0, // is a modifier
6970
};
7071

71-
expected_value = "{type: keydown, keymap: linux, scanCode: 4, toolkit: gtk, keyCode: 65, modifiers: 0, unicodeScalarValues: 65}";
72+
expected_value =
73+
"{type: keydown, keymap: linux, scanCode: 4, toolkit: gtk, keyCode: 65, "
74+
"modifiers: 0, unicodeScalarValues: 65}";
7275
fl_key_event_plugin_send_key_event(plugin, &key_event, loop);
7376

7477
// Blocks here until echo_response_cb is called.
7578
g_main_loop_run(loop);
7679

7780
key_event = GdkEventKey{
78-
GDK_KEY_RELEASE, // event type
79-
nullptr, // window (not needed)
80-
FALSE, // event was sent explicitly
81-
12345, // time
82-
0x0, // modifier state
83-
GDK_KEY_A, // key code
84-
1, // length of string representation
85-
reinterpret_cast<gchar*>(&string[0]), // string representation
86-
0x04, // scan code
87-
0, // keyboard group
88-
0, // is a modifier
81+
GDK_KEY_RELEASE, // event type
82+
nullptr, // window (not needed)
83+
FALSE, // event was sent explicitly
84+
12345, // time
85+
0x0, // modifier state
86+
GDK_KEY_A, // key code
87+
1, // length of string representation
88+
reinterpret_cast<gchar*>(&string[0]), // string representation
89+
0x04, // scan code
90+
0, // keyboard group
91+
0, // is a modifier
8992
};
9093

91-
expected_value = "{type: keyup, keymap: linux, scanCode: 4, toolkit: gtk, keyCode: 65, modifiers: 0, unicodeScalarValues: 65}";
94+
expected_value =
95+
"{type: keyup, keymap: linux, scanCode: 4, toolkit: gtk, keyCode: 65, "
96+
"modifiers: 0, unicodeScalarValues: 65}";
9297
fl_key_event_plugin_send_key_event(plugin, &key_event, loop);
9398

9499
// Blocks here until echo_response_cb is called.
95100
g_main_loop_run(loop);
96101
}
97102

98-
void test_lock_event(guint key_code, const char* down_expected, const char* up_expected) {
103+
void test_lock_event(guint key_code,
104+
const char* down_expected,
105+
const char* up_expected) {
99106
g_autoptr(GMainLoop) loop = g_main_loop_new(nullptr, 0);
100107

101108
g_autoptr(FlEngine) engine = make_mock_engine();
102109
FlBinaryMessenger* messenger = fl_binary_messenger_new(engine);
103-
g_autoptr(FlKeyEventPlugin) plugin = fl_key_event_plugin_new(messenger, echo_response_cb, "test/echo");
110+
g_autoptr(FlKeyEventPlugin) plugin =
111+
fl_key_event_plugin_new(messenger, echo_response_cb, "test/echo");
104112

105113
GdkEventKey key_event = GdkEventKey{
106-
GDK_KEY_PRESS, // event type
107-
nullptr, // window (not needed)
108-
FALSE, // event was sent explicitly
109-
12345, // time
110-
0x10, // modifier state
111-
key_code, // key code
112-
1, // length of string representation
113-
nullptr, // string representation
114-
0x04, // scan code
115-
0, // keyboard group
116-
0, // is a modifier
114+
GDK_KEY_PRESS, // event type
115+
nullptr, // window (not needed)
116+
FALSE, // event was sent explicitly
117+
12345, // time
118+
0x10, // modifier state
119+
key_code, // key code
120+
1, // length of string representation
121+
nullptr, // string representation
122+
0x04, // scan code
123+
0, // keyboard group
124+
0, // is a modifier
117125
};
118126

119127
expected_value = down_expected;
@@ -133,27 +141,27 @@ void test_lock_event(guint key_code, const char* down_expected, const char* up_e
133141

134142
// Test sending a "NumLock" keypress.
135143
TEST(FlKeyEventPluginTest, SendNumLockKeyEvent) {
136-
test_lock_event(
137-
GDK_KEY_Num_Lock,
138-
"{type: keydown, keymap: linux, scanCode: 4, toolkit: gtk, keyCode: 65407, modifiers: 16}",
139-
"{type: keyup, keymap: linux, scanCode: 4, toolkit: gtk, keyCode: 65407, modifiers: 0}"
140-
);
144+
test_lock_event(GDK_KEY_Num_Lock,
145+
"{type: keydown, keymap: linux, scanCode: 4, toolkit: gtk, "
146+
"keyCode: 65407, modifiers: 16}",
147+
"{type: keyup, keymap: linux, scanCode: 4, toolkit: gtk, "
148+
"keyCode: 65407, modifiers: 0}");
141149
}
142150

143151
// Test sending a "CapsLock" keypress.
144152
TEST(FlKeyEventPluginTest, SendCapsLockKeyEvent) {
145-
test_lock_event(
146-
GDK_KEY_Caps_Lock,
147-
"{type: keydown, keymap: linux, scanCode: 4, toolkit: gtk, keyCode: 65509, modifiers: 2}",
148-
"{type: keyup, keymap: linux, scanCode: 4, toolkit: gtk, keyCode: 65509, modifiers: 0}"
149-
);
153+
test_lock_event(GDK_KEY_Caps_Lock,
154+
"{type: keydown, keymap: linux, scanCode: 4, toolkit: gtk, "
155+
"keyCode: 65509, modifiers: 2}",
156+
"{type: keyup, keymap: linux, scanCode: 4, toolkit: gtk, "
157+
"keyCode: 65509, modifiers: 0}");
150158
}
151159

152160
// Test sending a "ShiftLock" keypress.
153161
TEST(FlKeyEventPluginTest, SendShiftLockKeyEvent) {
154-
test_lock_event(
155-
GDK_KEY_Shift_Lock,
156-
"{type: keydown, keymap: linux, scanCode: 4, toolkit: gtk, keyCode: 65510, modifiers: 2}",
157-
"{type: keyup, keymap: linux, scanCode: 4, toolkit: gtk, keyCode: 65510, modifiers: 0}"
158-
);
162+
test_lock_event(GDK_KEY_Shift_Lock,
163+
"{type: keydown, keymap: linux, scanCode: 4, toolkit: gtk, "
164+
"keyCode: 65510, modifiers: 2}",
165+
"{type: keyup, keymap: linux, scanCode: 4, toolkit: gtk, "
166+
"keyCode: 65510, modifiers: 0}");
159167
}

0 commit comments

Comments
 (0)