Skip to content

Commit fde117b

Browse files
authored
Fix for win32 embedder failing to send all alt key downs to the flutter app (flutter#179097)
fixes [flutter#177822](flutter#177822) ## What's new? - The `FlutterWindow` now consumes any `WM_SYSKEYDOWN (VK_MENU)` message - Wrote a test to validate that this is the case - Make it so that the `KeyboardManager` can be provided This PR was made possible by flutter#178523 ## How to test? 1. Create a new flutter project 2. Add the following: ```dart import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; void main() { runApp(const MyApp()); HardwareKeyboard.instance.addHandler((event) { if (event is KeyDownEvent) { print('Key pressed: ${event.logicalKey.debugName}'); } return false; }); } // Rest of file unchanged from flutter create output ``` 3. Click alt. Note that the message is received. 4. Continue clicking alt and validate that the message is received. 5. Alt tab out and back and note that subsequent alts are correctly received. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
1 parent 9d96df2 commit fde117b

File tree

4 files changed

+69
-6
lines changed

4 files changed

+69
-6
lines changed

engine/src/flutter/shell/platform/windows/flutter_window.cc

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,13 @@ FlutterWindow::FlutterWindow(
7979
int height,
8080
std::shared_ptr<DisplayManagerWin32> const& display_manager,
8181
std::shared_ptr<WindowsProcTable> windows_proc_table,
82-
std::unique_ptr<TextInputManager> text_input_manager)
82+
std::unique_ptr<TextInputManager> text_input_manager,
83+
std::unique_ptr<KeyboardManager> keyboard_manager)
8384
: touch_id_generator_(kMinTouchDeviceId, kMaxTouchDeviceId),
8485
display_manager_(display_manager),
8586
windows_proc_table_(std::move(windows_proc_table)),
8687
text_input_manager_(std::move(text_input_manager)),
88+
keyboard_manager_(std::move(keyboard_manager)),
8789
ax_fragment_root_(nullptr) {
8890
// Get the DPI of the primary monitor as the initial DPI. If Per-Monitor V2 is
8991
// supported, |current_dpi_| should be updated in the
@@ -101,7 +103,9 @@ FlutterWindow::FlutterWindow(
101103
if (text_input_manager_ == nullptr) {
102104
text_input_manager_ = std::make_unique<TextInputManager>();
103105
}
104-
keyboard_manager_ = std::make_unique<KeyboardManager>(this);
106+
if (keyboard_manager_ == nullptr) {
107+
keyboard_manager_ = std::make_unique<KeyboardManager>(this);
108+
}
105109

106110
InitializeChild("FLUTTERVIEW", width, height);
107111
}
@@ -756,6 +760,17 @@ FlutterWindow::HandleMessage(UINT const message,
756760
if (keyboard_manager_->HandleMessage(message, wparam, lparam)) {
757761
return 0;
758762
}
763+
764+
// Fix for https://github.com/flutter/flutter/issues/177822
765+
// When a user clicks the alt key, Win32 will attempt to open up the
766+
// system menu by default. This causes the next key down to be consumed
767+
// by the event system and therefore never delivered to the app.
768+
// The workaround is to eat VK_MENU events. If the Flutter app wishes
769+
// to open a menu in response to alt down, they will now receive the
770+
// alt and can choose to do what they like in response.
771+
if (message == WM_SYSKEYDOWN && wparam == VK_MENU) {
772+
return 0;
773+
}
759774
break;
760775
}
761776

engine/src/flutter/shell/platform/windows/flutter_window.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ class FlutterWindow : public KeyboardManager::WindowDelegate,
4242
int height,
4343
std::shared_ptr<DisplayManagerWin32> const& display_manager,
4444
std::shared_ptr<WindowsProcTable> windows_proc_table = nullptr,
45-
std::unique_ptr<TextInputManager> text_input_manager = nullptr);
45+
std::unique_ptr<TextInputManager> text_input_manager = nullptr,
46+
std::unique_ptr<KeyboardManager> keyboard_manager = nullptr);
4647

4748
virtual ~FlutterWindow();
4849

engine/src/flutter/shell/platform/windows/flutter_window_unittests.cc

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,30 @@ using ::testing::Return;
2525
namespace {
2626
static constexpr int32_t kDefaultPointerDeviceId = 0;
2727

28+
class InjectableFlutterWindow : public FlutterWindow {
29+
public:
30+
InjectableFlutterWindow(
31+
int width,
32+
int height,
33+
std::shared_ptr<DisplayManagerWin32> const& display_manager,
34+
std::shared_ptr<WindowsProcTable> windows_proc_table = nullptr,
35+
std::unique_ptr<TextInputManager> text_input_manager = nullptr,
36+
std::unique_ptr<KeyboardManager> keyboard_manager = nullptr)
37+
: FlutterWindow(width,
38+
height,
39+
display_manager,
40+
std::move(windows_proc_table),
41+
std::move(text_input_manager),
42+
std::move(keyboard_manager)) {}
43+
44+
// Simulates a WindowProc message from the OS.
45+
LRESULT InjectWindowMessage(UINT const message,
46+
WPARAM const wparam,
47+
LPARAM const lparam) {
48+
return HandleMessage(message, wparam, lparam);
49+
}
50+
};
51+
2852
class MockFlutterWindow : public FlutterWindow {
2953
public:
3054
MockFlutterWindow(bool reset_view_on_exit = true)
@@ -111,6 +135,15 @@ class MockFlutterWindowsView : public FlutterWindowsView {
111135
FML_DISALLOW_COPY_AND_ASSIGN(MockFlutterWindowsView);
112136
};
113137

138+
class MockKeyboardManager : public KeyboardManager {
139+
public:
140+
MockKeyboardManager() : KeyboardManager(nullptr) {}
141+
MOCK_METHOD(bool,
142+
HandleMessage,
143+
(UINT const, WPARAM const, LPARAM const),
144+
(override));
145+
};
146+
114147
class FlutterWindowTest : public WindowsTest {};
115148

116149
} // namespace
@@ -310,6 +343,19 @@ TEST_F(FlutterWindowTest, OnThemeChange) {
310343
win32window.InjectWindowMessage(WM_THEMECHANGED, 0, 0);
311344
}
312345

346+
TEST_F(FlutterWindowTest, VkMenuSysKeyDownMessageIsAlwaysHandled) {
347+
std::unique_ptr<FlutterWindowsEngine> engine =
348+
FlutterWindowsEngineBuilder{GetContext()}.Build();
349+
std::unique_ptr<MockKeyboardManager> keyboard_manager =
350+
std::make_unique<MockKeyboardManager>();
351+
EXPECT_CALL(*keyboard_manager, HandleMessage)
352+
.WillOnce(testing::Return(false));
353+
InjectableFlutterWindow window(800, 600, engine->display_manager(), nullptr,
354+
nullptr, std::move(keyboard_manager));
355+
356+
EXPECT_EQ(window.InjectWindowMessage(WM_SYSKEYDOWN, VK_MENU, 0), 0);
357+
}
358+
313359
// The window should return no root accessibility node if
314360
// it isn't attached to a view.
315361
// Regression test for https://github.com/flutter/flutter/issues/129791

engine/src/flutter/shell/platform/windows/keyboard_manager.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ class KeyboardManager {
9393
using KeyEventCallback = WindowDelegate::KeyEventCallback;
9494

9595
explicit KeyboardManager(WindowDelegate* delegate);
96+
virtual ~KeyboardManager() = default;
9697

9798
// Processes Win32 messages related to keyboard and text.
9899
//
@@ -106,9 +107,9 @@ class KeyboardManager {
106107
// message here usually means that this message is a redispatched message,
107108
// but there are other rare cases too. |Window| should forward unhandled
108109
// messages to |DefWindowProc|.
109-
bool HandleMessage(UINT const message,
110-
WPARAM const wparam,
111-
LPARAM const lparam);
110+
virtual bool HandleMessage(UINT const message,
111+
WPARAM const wparam,
112+
LPARAM const lparam);
112113

113114
protected:
114115
struct Win32Message {

0 commit comments

Comments
 (0)