starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-09-25 08:24:54 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

[PR #13118] Fix ShowWindow(GetConsoleWindow()) #29399

New Issue
Open
opened 2026-01-31 09:34:39 +00:00 by claunia · 0 comments
claunia commented 2026-01-31 09:34:39 +00:00
Owner
Copy Link

Original Pull Request: https://github.com/microsoft/terminal/pull/13118

State: closed
Merged: Yes

A bad merge, that actually revealed a horrible bug.

There was a secret conflict between the code in #12526 and #12515. 69b77ca was a bad merge that hid just how bad the issue was. Fixing the one line nullptr->this in InteractivityFactory resulted in a window that would flash uncontrollably, as it minimized and restored itself in a loop. Great.

This can seemingly be fixed by making sure that the conpty window is initially created with the owner already set, rather than relying on a SetParent call in post. This does pose some complications for the #1256 future we're approaching. However, this is a blocking bug now, and we can figure out the tearout/SetParent thing in post.

  • fixes #13066.
  • Tested with the script in that issue.
  • Window doesn't flash uncontrollably.
  • gci | ogv still works right
  • I work here.
  • Opening a new tab doesn't spontaneously cause the window to minimize
  • Restoring from minimized doesn't yeet focus to an invisible window
  • Opening a new tab doesn't yeet focus to an invisible window
  • There is a viable way to call GetAncestor s.t. it returns the Terminal's hwnd in Terminal, and the console's in Conhost

The SW_SHOWNOACTIVATE change is also quite load bearing. With just SW_NORMAL, the pseudo window (which is invisible!) gets activated whenever the terminal window is restored from minimized. That's BAD.

There's actually more to this as well.

Calling SetParent on a window that is WS_VISIBLE will cause the OS to hide the window, make it a child window, then call SW_SHOW on the window to re-show it. SW_SHOW, however, will cause the OS to also set that window as the foreground window, which would result in the pty's hwnd stealing the foreground away from the owning terminal window. That's bad.

SetWindowLongPtr seems to do the job of changing who the window owner is, without all the other side effects of reparenting the window.

Without SetParent, however, the pty HWND is no longer a descendant of the Terminal HWND, so that means GA_ROOT can no longer be used to find the owner's hwnd. For even more insanity, without WS_POPUP, none of the values of GetAncestor will actually get the terminal HWND. So, now we also need WS_POPUP on the pty hwnd. To get at the Terminal hwnd, you'll need

GetAncestor(GetConsoleWindow(), GA_ROOTOWNER)
Reparenting

For #1256.

Although not tested in the scope of the Terminal, I'm fairly confident that SetWindowLongPtr will work for this.
image

In that screenshot, I reparented the black terminal to be owned by the blue one. API calls all seemed to confirm that worked, and the hidden pty window wasn't activated as a middle step.

Helper Scripts

Guys TIL how handy the Win32 interop of powershell is. Look at this. This is so much easier.

$NativeFunctions=@"
using System;
using System.Runtime.InteropServices;
public class Native {

    [DllImport("user32.dll")]
    public static extern bool ShowWindow(IntPtr hWnd, int nCmdShow);

    [DllImport("user32.dll")]
    public static extern IntPtr GetAncestor(IntPtr hWnd, uint gaFlags);

    [DllImport("user32.dll")]
    public static extern IntPtr GetParent(IntPtr hWnd);

    [DllImport("user32.dll")]
    public static extern IntPtr GetWindowLongPtr(IntPtr hWnd, int nIndex);

    [DllImport("user32.dll")]
    public static extern IntPtr SetWindowLongPtr(IntPtr hWnd, int nIndex, IntPtr dwNewLong);

    [DllImport("kernel32.dll", SetLastError = true)]
    public static extern IntPtr GetConsoleWindow();
}
"@
Add-Type -TypeDefinition $NativeFunctions


# [Native]::ShowWindow([Native]::GetConsoleWindow(), 0)

function hwnds {
  $console = [Native]::GetConsoleWindow()

  $GA_PARENT = [Native]::GetAncestor($console, 1) # GA_PARENT
  $GA_ROOT = [Native]::GetAncestor($console, 2) # GA_ROOT
  $GA_ROOTOWNER = [Native]::GetAncestor($console, 3) # GA_ROOTOWNER
  $Parent = [Native]::GetParent($console)
  $GWLP_HWNDPARENT = [Native]::GetWindowLongPtr($console, -8)

  Write-Host ("Console`t`t{0:x}`nGA_PARENT`t{1:x}`nGA_ROOT`t`t{2:x}`nGA_ROOTOWNER`t{3:x}`nParent`t`t{4:x}`nGWLP_HWNDPARENT`t{5:x}" -f $console,$GA_PARENT,$GA_ROOT,$GA_ROOTOWNER,$Parent,$GWLP_HWNDPARENT)

  if ($console -eq $GA_ROOTOWNER) { Write-Host "`e[31mThis is bad`e[m" }
  else {
    if ($console -eq $GA_ROOT) { Write-Host "`e[33mThis is okay`e[m - we told folks that they should use ROOT. They probably will need ROOTOWNER`e[m" }
    else { Write-Host "`e[32mThis is good`e[m" }
  }
}
**Original Pull Request:** https://github.com/microsoft/terminal/pull/13118 **State:** closed **Merged:** Yes --- A bad merge, that actually revealed a horrible bug. There was a secret conflict between the code in #12526 and #12515. 69b77ca was a bad merge that hid just how bad the issue was. Fixing the one line `nullptr`->`this` in `InteractivityFactory` resulted in a window that would flash uncontrollably, as it minimized and restored itself in a loop. Great. This can seemingly be fixed by making sure that the conpty window is initially created with the owner already set, rather than relying on a `SetParent` call in post. This does pose some complications for the #1256 future we're approaching. However, this is a blocking bug _now_, and we can figure out the tearout/`SetParent` thing in post. * fixes #13066. * Tested with the script in that issue. * Window doesn't flash uncontrollably. * `gci | ogv` still works right * I work here. * Opening a new tab doesn't spontaneously cause the window to minimize * Restoring from minimized doesn't yeet focus to an invisible window * Opening a new tab doesn't yeet focus to an invisible window * There _is_ a viable way to call `GetAncestor` s.t. it returns the Terminal's hwnd in Terminal, and the console's in Conhost The `SW_SHOWNOACTIVATE` change is also quite load bearing. With just `SW_NORMAL`, the pseudo window (which is invisible!) gets activated whenever the terminal window is restored from minimized. That's BAD. There's actually more to this as well. Calling `SetParent` on a window that is `WS_VISIBLE` will cause the OS to hide the window, make it a _child_ window, then call `SW_SHOW` on the window to re-show it. `SW_SHOW`, however, will cause the OS to also set that window as the _foreground_ window, which would result in the pty's hwnd stealing the foreground away from the owning terminal window. That's bad. `SetWindowLongPtr` seems to do the job of changing who the window owner is, without all the other side effects of reparenting the window. Without `SetParent`, however, the pty HWND is no longer a descendant of the Terminal HWND, so that means `GA_ROOT` can no longer be used to find the owner's hwnd. For even more insanity, without `WS_POPUP`, none of the values of `GetAncestor` will actually get the terminal HWND. So, now we also need `WS_POPUP` on the pty hwnd. To get at the Terminal hwnd, you'll need ```c++ GetAncestor(GetConsoleWindow(), GA_ROOTOWNER) ``` <details> <summary>Reparenting</summary> For #1256. Although not tested in the scope of the Terminal, I'm fairly confident that `SetWindowLongPtr` will work for this. ![image](https://user-images.githubusercontent.com/18356694/169155530-034bfb1f-2d34-4371-b083-8fc41cf9ecdc.png) In that screenshot, I reparented the black terminal to be owned by the blue one. API calls all seemed to confirm that worked, and the hidden pty window wasn't activated as a middle step. </details> <details> <summary>Helper Scripts</summary> Guys TIL how handy the Win32 interop of powershell is. Look at this. This is so much easier. ```pwsh $NativeFunctions=@" using System; using System.Runtime.InteropServices; public class Native { [DllImport("user32.dll")] public static extern bool ShowWindow(IntPtr hWnd, int nCmdShow); [DllImport("user32.dll")] public static extern IntPtr GetAncestor(IntPtr hWnd, uint gaFlags); [DllImport("user32.dll")] public static extern IntPtr GetParent(IntPtr hWnd); [DllImport("user32.dll")] public static extern IntPtr GetWindowLongPtr(IntPtr hWnd, int nIndex); [DllImport("user32.dll")] public static extern IntPtr SetWindowLongPtr(IntPtr hWnd, int nIndex, IntPtr dwNewLong); [DllImport("kernel32.dll", SetLastError = true)] public static extern IntPtr GetConsoleWindow(); } "@ Add-Type -TypeDefinition $NativeFunctions # [Native]::ShowWindow([Native]::GetConsoleWindow(), 0) function hwnds { $console = [Native]::GetConsoleWindow() $GA_PARENT = [Native]::GetAncestor($console, 1) # GA_PARENT $GA_ROOT = [Native]::GetAncestor($console, 2) # GA_ROOT $GA_ROOTOWNER = [Native]::GetAncestor($console, 3) # GA_ROOTOWNER $Parent = [Native]::GetParent($console) $GWLP_HWNDPARENT = [Native]::GetWindowLongPtr($console, -8) Write-Host ("Console`t`t{0:x}`nGA_PARENT`t{1:x}`nGA_ROOT`t`t{2:x}`nGA_ROOTOWNER`t{3:x}`nParent`t`t{4:x}`nGWLP_HWNDPARENT`t{5:x}" -f $console,$GA_PARENT,$GA_ROOT,$GA_ROOTOWNER,$Parent,$GWLP_HWNDPARENT) if ($console -eq $GA_ROOTOWNER) { Write-Host "`e[31mThis is bad`e[m" } else { if ($console -eq $GA_ROOT) { Write-Host "`e[33mThis is okay`e[m - we told folks that they should use ROOT. They probably will need ROOTOWNER`e[m" } else { Write-Host "`e[32mThis is good`e[m" } } } ``` </details>
claunia added the pull-request label 2026-01-31 09:34:39 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
⛺ Reserved
A11yCO
A11yMAS
A11ySev1
A11ySev2
A11ySev3
A11yTTValidated
A11yUsable
A11yVoiceAccess
A11yWCAG
Area-Accessibility
Area-AtlasEngine
Area-AzureShell
Area-Build
Area-Build
Area-Chat
Area-CmdPal
Area-CodeHealth
Area-Commandline
Area-CookedRead
Area-DefApp
Area-Extensibility
Area-Fonts
Area-GroupPolicy
Area-i18n
Area-Input
Area-Interaction
Area-Interop
Area-Localization
Area-Output
Area-Performance
Area-Portable
Area-Quality
Area-Remoting
Area-Rendering
Area-Schema
Area-Server
Area-Settings
Area-SettingsUI
Area-ShellExtension
Area-ShellExtension
Area-ShellExtension
Area-Suggestions
Area-Suggestions
Area-TerminalConnection
Area-TerminalControl
Area-Theming
Area-UserInterface
Area-VT
Area-Windowing
Area-WPFControl
AutoMerge
Blocking-Ingestion
Culprit-Centennial
Culprit-WinUI
Disability-All
Disability-Blind
Disability-LowVision
Disability-Mobility
External-Blocked-WinUI3
Fixed
Gathering-Data
good first issue
HCL-E+D
HCL-WindowsTerminal
Help Wanted
Impact-Compatibility
Impact-Compliance
Impact-Correctness
Impact-Visual
In-PR
InclusionBacklog
InclusionBacklog-Windows TerminalWin32
InclusionCommitted-202206
Issue-Bug
Issue-Docs
Issue-Feature
Issue-Feature
Issue-Question
Issue-Samples
Issue-Scenario
Issue-Task
Needs-Attention
Needs-Author-Feedback
Needs-Bisect
Needs-Discussion
Needs-Repro
Needs-Tag-Fix
Needs-Tag-Fix
Needs-Triage
No-Recent-Activity
Priority-0
Priority-1
Priority-2
Priority-3
Product-Cmd.exe
Product-Colortool
Product-Colortool
Product-Colortool
Product-Conhost
Product-Conpty
Product-Meta
Product-Powershell
Product-Terminal
Product-WSL
pull-request
Mirrored from GitHub Pull Request
 Resolution-Answered
Resolution-By-Design
Resolution-Duplicate
Resolution-External
Resolution-Fix-Available
Resolution-Fix-Committed
Resolution-No-Repro
Resolution-Won't-Fix
Severity-Blocking
Severity-Crash
Severity-DataLoss
spam
this-will-be-a-breaking-change
Tracking-External
WindowsTerminal_Win32
Work-Item
zAskModeBug
zInbox-Bug
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#29399
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?