[Windows] Fix Space input after Alt+Space with previous Microsoft IME - #192105

@JIWON1923 Pinging you in case this got lost!

@JIWON1923 Pinging you in case this got lost!

Thanks for the ping, and sorry for the delayed response!

If I remember correctly, I kept this check outside the sequence_logical_key == VK_PROCESSKEY block because there was a case where the stale pressed record might not be released otherwise.

It's been a while, so I'd like to test this again to make sure. I'll take another look and get back to you!

@mattkae

Thanks for the suggestion! I tried it locally and tested it with Use previous version of Microsoft IME enabled.

It turns out the suggested placement breaks the fix, and it also makes the new regression test FreshImeKeyDownReleasesStalePressedKey fail. I'd like to keep the current placement, and here is why.

Why the check can't move inside if (sequence_logical_key == VK_PROCESSKEY)

sequence_logical_key is taken from the existing pressed record whenever had_record is true. In this scenario that record still holds the stale Space entry, so when the fresh VK_PROCESSKEY arrives, sequence_logical_key is Space (32), not VK_PROCESSKEY (229):

[entry] key=229 scancode=57 had_record=1
        last_logical_record=32 sequence_logical_key=32
[dispatch] logical=229 eventual_record=229
[entry] key=32 last_logical_record=229 sequence_logical_key=229
[ime-filter] key=32
[text-suppressed] key=32 character=32 framework_handled=1

(Log tags are shortened here; these are the same log points used in the PR description.)

Because sequence_logical_key is 32, the sequence_logical_key == VK_PROCESSKEY branch is never entered for this event. Neither the synthesized key-up nor the IME filtering runs, so VK_PROCESSKEY is dispatched as an ordinary key-down and recorded as 229. The translated VK_SPACE event that follows then inherits 229 as its sequence logical key, is filtered as an IME event, and its WM_CHAR is suppressed.

In short, the branch we would move the check into is exactly the branch that the stale record prevents us from reaching. The check has to run before the sequence logical key is derived from the record, which is why it sits ahead of the IME filtering branch.

Manual verification

The same input sequence was used in all three recordings:

가나다 → Alt+Space → Esc → 가나다 → Space → Space

The decisive detail is how many of the two trailing spaces actually get inserted.

1. Notepad (baseline) — both spaces are inserted. The status bar ends at column 9 / 8 characters (6 Hangul syllables + 2 spaces).

2. Current implementation in this PR — matches Notepad. Both spaces are inserted.

3. With the suggested change — the first Space only commits the composing text and no space character is inserted. The second Space works normally, so the field ends with one space instead of two.

Given the above, would it be okay to keep the current implementation as it is? Please let me know if you would prefer a different approach.

Thanks for the contribution!

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #192105 at sha eccd612

autosubmit label was removed for flutter/flutter/192105, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

autosubmit label was removed for flutter/flutter/192105, because - The status or check suite Mac_arm64_verify_binaries has failed. Please fix the issues identified (or deflake) before re-applying this label.

@mattkae Could you re-apply autosubmit? Checks are all green now. Thanks!