Send a KeyUp sequence only once a key has been released (#15130)

When win32-input-mode is enabled, we generate an input sequence for both
key down and key up events. However, in the initial implementation the
key up sequence for most keypresses would be faked - they were generated
at the same time as the key down sequence, regardless of when the key
was actually released. After this PR, we'll only generate the key up
sequence once a key has actually been released.

## References and Relevant Issues

The initial implementation of win32-input-mode was in PR #6309.
The spec for win32-input-mode was in PR #5887.

## Validation Steps Performed

I've manually tested this with an app that polls `ReadConsoleInput` in a
loop and logs the results. With this PR applied, I can now see the key
up events as a key is released, rather than when it was first pressed.

When compared with conhost, though, there are some differences. When
typing a shifted key, e.g. `Shift`+`A`, WT generates key down and key up
events for both the `Shift` and the `A`, while conhost only generates
both events for the `Shift` - the `A` won't generate a key up event
unless you release the `Shift` before the `A`. That seems more like a
conhost flaw though.

Another case I tested was the Japanese Microsoft IME, which in conhost
will generate a key down event for the Japanese character being inserted
followed by a key up event for for `Return`. WT generates key up events
for the ASCII characters being typed in the IME, then both a key down
and key up event for the inserted Japanese character, and finally a key
up event for `Return`. Both of those seem weird, but they still appear
to work OK. 

The current version of WT actually produces the most sensible behavior
for the IME - it just generates key up and key down events for the
inserted character. But that's only because it's dropping most of the
system generated key up events.

Closes #8440
This commit is contained in:
James Holderness
2023-04-11 23:59:25 +01:00
committed by GitHub
parent b4f65030e3
commit 508adbb1ec

View File

@@ -666,7 +666,7 @@ std::optional<PointTree::interval> Terminal::GetHyperlinkIntervalFromViewportPos
// - Character events (e.g. WM_CHAR) are generally the best way to properly receive
// keyboard input on Windows though, as the OS is suited best at handling the
// translation of the current keyboard layout, dead keys, etc.
// As a result of this false is returned for all key events that contain characters.
// As a result of this false is returned for all key down events that contain characters.
// SendCharEvent may then be called with the data obtained from a character event.
// - As a special case we'll always handle VK_TAB key events.
// This must be done due to TermControl::_KeyDownHandler (one of the callers)
@@ -728,15 +728,15 @@ bool Terminal::SendKeyEvent(const WORD vkey,
const auto isSuppressedAltGrAlias = !_altGrAliasing && states.IsAltPressed() && states.IsCtrlPressed() && !states.IsAltGrPressed();
const auto ch = isSuppressedAltGrAlias ? UNICODE_NULL : _CharacterFromKeyEvent(vkey, sc, states);
// Delegate it to the character event handler if this key event can be
// mapped to one (see method description above). For Alt+key combinations
// Delegate it to the character event handler if this is a key down event that
// can be mapped to one (see method description above). For Alt+key combinations
// we'll not receive another character event for some reason though.
// -> Don't delegate the event if this is a Alt+key combination.
//
// As a special case we'll furthermore always handle VK_TAB
// key events here instead of in Terminal::SendCharEvent.
// See the method description for more information.
if (!isAltOnlyPressed && vkey != VK_TAB && ch != UNICODE_NULL)
if (keyDown && !isAltOnlyPressed && vkey != VK_TAB && ch != UNICODE_NULL)
{
return false;
}
@@ -818,15 +818,8 @@ bool Terminal::SendCharEvent(const wchar_t ch, const WORD scanCode, const Contro
MarkOutputStart();
}
// Unfortunately, the UI doesn't give us both a character down and a
// character up event, only a character received event. So fake sending both
// to the terminal input translator. Unless it's in win32-input-mode, it'll
// ignore the keyup.
const KeyEvent keyDown{ true, 1, vkey, scanCode, ch, states.Value() };
const KeyEvent keyUp{ false, 1, vkey, scanCode, ch, states.Value() };
const auto handledDown = _terminalInput->HandleKey(&keyDown);
const auto handledUp = _terminalInput->HandleKey(&keyUp);
return handledDown || handledUp;
return _terminalInput->HandleKey(&keyDown);
}
// Method Description: