Skip to content

Possible bug: Inconsistency between WM_KEYDOWN and WM_KEYUP #20002

Open

Description

While investigating something related to control key events, I was looking at the source code to TerminalContainer_MessageHook in TerminalContainer.cs and saw this:

case NativeMethods.WindowMessage.WM_SYSKEYDOWN: // fallthrough
case NativeMethods.WindowMessage.WM_KEYDOWN:
{
UnpackKeyMessage(wParam, lParam, out ushort vkey, out ushort scanCode, out ushort flags);
NativeMethods.TerminalSendKeyEvent(this.terminal, vkey, scanCode, flags, true);
break;
}
case NativeMethods.WindowMessage.WM_SYSKEYUP: // fallthrough
case NativeMethods.WindowMessage.WM_KEYUP:
{
// WM_KEYUP lParam layout documentation: https://docs.microsoft.com/en-us/windows/win32/inputdev/wm-keyup
UnpackKeyMessage(wParam, lParam, out ushort vkey, out ushort scanCode, out ushort flags);
NativeMethods.TerminalSendKeyEvent(this.terminal, (ushort)wParam, scanCode, flags, false);
break;
}

This has a "smell" to it: Both handler cases call UnpackKeyMessage populating a local variable vkey, but only one of them actually uses it. The WM_KEYDOWN handler passes vkey on to TerminalSendKeyEvent, but the WM_KEYUP handler passes wParam unaltered for the same parameter.

Is this a bug?

If not, I would recommend calling out the difference between the two TerminalSendKeyEvent calls explicitly, with a comment or a strategically-named local variable or what have you.

Activity

  1. added
    Issue-BugIt either shouldn't be doing this or needs an investigation.
    Needs-TriageIt's a new issue that the core contributor team needs to triage at the next triage meeting
    on Mar 23, 2026
  2. logiclrd commented on Mar 23, 2026

    @logiclrd
    Author

    I just looked one layer deeper, and I see that UnpackKeyMessage fills vkey with the value of wParam. So this is definitely not a functional bug. But, it is still a bug for the calls to not match; if UnpackKeyMessage were ever updated in the future to perform some more complicated logic to produce vkey, it would graduate to being an actual functional bug at that time.

  3. DHowett commented on Apr 8, 2026

    @DHowett
    Member

    Thanks for the investigation! I'm going to tag it up to visit during #6999, which will replace a lot of this code.

  4. added
    Area-CodeHealthIssues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc.
    Area-WPFControlThings related to the WPF version of the TermControl
    and removed
    Needs-TriageIt's a new issue that the core contributor team needs to triage at the next triage meeting
    on Apr 8, 2026
  5. DHowett commented on Oct 21, 2026

    @DHowett
    Member

    Alright, so, I touched almost all the code in the WPF<->native code bridge and still did nothing about this. I just checked it in for the second round of fixes. Thanks!

    (see #20812 for more)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Area-CodeHealthIssues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc.Area-WPFControlThings related to the WPF version of the TermControlIssue-BugIt either shouldn't be doing this or needs an investigation.Product-TerminalThe new Windows Terminal.

    Type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions