-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[1.14] ShowWindow(SW_MINIMIZE)
doesn't work anymore
#13066
Comments
I was the murderer That's a 1 line fix, but fixing that causes the window to flicker uncontrollably. Somewhere there's a mismatch between the Terminal HWND visibility and the ConPTY one, some flipped boolean somewhere. |
|
Hmm. It's the combo of 22fb15f with #12526 that causes this. That initial show/hide to sync the terminal with the conpty, that's what's busted somehow. Merely commenting those lines fixes this. (see bbae235) HOWEVER, it breaks doing a
|
The plot thickens. Remove the call to reparent, but leave the initial state sync in. Things seem to work. HOWEVER, if you minimize the Terminal, then restore it, the terminal doesn't get focus. I bet that the pty window is becoming
lmao
Yep you can sure do that. |
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) ```
|
Well this one feels dumb. Make sure to also initially set the visibility of ConPTY windows created for DefTerm connections. * [x] Closes #13066 for real. * [x] tested manually.
Well this one feels dumb. Make sure to also initially set the visibility of ConPTY windows created for DefTerm connections. * [x] Closes #13066 for real. * [x] tested manually.
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) ``` (cherry picked from commit 77215d9) Service-Card-Id: 82170678 Service-Version: 1.14
From bug bash.
.\hide.ps1
expect:
actual:
The text was updated successfully, but these errors were encountered: