Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Kanji switch in Japanese IME on WIndows #29761

Merged
merged 1 commit into from
Nov 17, 2021

Conversation

gspencergoog
Copy link
Contributor

Description

This fixes flutter/flutter#93422 by sending any WM_SYSKEYDOWN/WM_SYSKEYUP messages to the Windows default window proc regardless of whether the framework handles it or not. It will also send them to the Framework, but if the framework handles them, they will already have been sent to the default window proc, and so will also still be propagated.

This is because the Kanji switch is a system key that can't be synthesized, so the mode switch is lost when Flutter responded with "handled" (the previous behavior), and then it couldn't synthesize one that works when the framework said it didn't handle it.

Related Issues

Tests

  • Added tests for WM_SYSKEYDOWN/WM_SYSKEYUP to several of the unit tests that already test for WM_KEYDOWN/WM_KEYUP.

@google-cla google-cla bot added the cla: yes label Nov 16, 2021
@gspencergoog gspencergoog changed the title Fix japanese ime Fix Kanji switch in Japanese ime Nov 16, 2021
@gspencergoog gspencergoog changed the title Fix Kanji switch in Japanese ime Fix Kanji switch in Japanese IME on WIndows Nov 16, 2021
Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@gspencergoog gspencergoog added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Nov 17, 2021
@fluttergithubbot fluttergithubbot merged commit 7a58963 into flutter:main Nov 17, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 17, 2021
const bool was_down = lparam & 0x40000000;
bool is_syskey = message == WM_SYSKEYDOWN || message == WM_SYSKEYUP;
const int action = is_keydown_message
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this way action would be identical to message. There is no reason to do this any more.

.RetiresOnSaturation();
EXPECT_CALL(*flutter_windows_view.text_input_plugin,
KeyboardHook(_, _, _, _, _, _, _))
.Times(1)
Copy link
Contributor

@dkwingsmt dkwingsmt Dec 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this will work? Is it based on actual tests? Because SendInput should not be able to send SYS key events, therefore the redispatching process, which text_input_plugin relies on, will not work correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes platform-windows waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
4 participants