egregious notes

This commit is contained in:
Mike Griese
2022-05-18 15:37:41 -05:00
parent 5f6ff346c3
commit 33b8a20e1b
3 changed files with 35 additions and 18 deletions

View File

@@ -74,10 +74,8 @@ void PtySignalInputThread::ConnectConsole() noexcept
{
_DoShowHide(_initialShowHide->show);
}
// if (_earlyReparent)
// {
// _DoSetWindowParent(*_earlyReparent);
// }
// We should have successfully used the _earlyReparent message in CreatePseudoWindow.
}
// Method Description:
@@ -90,13 +88,7 @@ void PtySignalInputThread::ConnectConsole() noexcept
void PtySignalInputThread::CreatePseudoWindow()
{
HWND owner = _earlyReparent.has_value() ? reinterpret_cast<HWND>((*_earlyReparent).handle) : HWND_DESKTOP;
if (const auto pseudoHwnd{ ServiceLocator::LocatePseudoWindow(owner) })
{
// LOG_LAST_ERROR_IF_NULL(::SetParent(pseudoHwnd, owner));
// TODO! This needs to be SetWindowLongPtr
}
ServiceLocator::LocatePseudoWindow(owner);
}
// Method Description:
@@ -244,15 +236,28 @@ void PtySignalInputThread::_DoShowHide(const bool show)
// - <none>
void PtySignalInputThread::_DoSetWindowParent(const SetParentData& data)
{
const auto owner{ reinterpret_cast<HWND>(data.handle) };
// This will initialize s_interactivityFactory for us. It will also
// conveniently return 0 when we're on OneCore.
//
// If the window hasn't been created yet, by some other call to
// LocatePseudoWindow, then this will also initialize the owner of the
// window.
if (const auto pseudoHwnd{ ServiceLocator::LocatePseudoWindow(reinterpret_cast<HWND>(data.handle)) })
if (const auto pseudoHwnd{ ServiceLocator::LocatePseudoWindow(owner) })
{
LOG_LAST_ERROR_IF_NULL(::SetParent(pseudoHwnd, reinterpret_cast<HWND>(data.handle)));
// DO NOT USE SetParent HERE!
//
// 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.
// See #13066
::SetWindowLongPtr(pseudoHwnd, GWLP_HWNDPARENT, reinterpret_cast<LONG_PTR>(owner));
}
}

View File

@@ -320,6 +320,12 @@ using namespace Microsoft::Console::Interactivity;
// as far as the difference between parent/child and owner/owned
// windows). Evan K said we should do it this way, and he
// definitely knows.
//
// GH#13066: Load-bearing: Make sure to set WS_POPUP. If you
// don't, then GetAncestor(GetConsoleWindow(), GA_ROOTOWNER)
// will return the console handle again, not the owning
// terminal's handle. It's not entirely clear why, but WS_POPUP
// is absolutely vital for this to work correctly.
const auto windowStyle = WS_OVERLAPPEDWINDOW | WS_POPUP;
const auto exStyles = WS_EX_TOOLWINDOW | WS_EX_TRANSPARENT | WS_EX_LAYERED | WS_EX_NOACTIVATE;

View File

@@ -204,12 +204,18 @@ bool InteractDispatch::FocusChanged(const bool focused) const
{
// They want focus, we found a pseudo hwnd.
// // Note: ::GetParent(pseudoHwnd) will return 0. GetAncestor works though.
// // GA_PARENT and GA_ROOT seemingly return the same thing for
// // Terminal. We're going with GA_ROOT since it seems
// // semantically more correct here.
// BODGY
//
// TODO! I think this needs to be ROOTOWNER now.
// This needs to be GA_ROOTOWNER here. Not GA_ROOT, GA_PARENT,
// or GetParent. The ConPTY hwnd is an owned, top-level, popup,
// non-parented window. It does not have a parent set. It does
// have an owner set. It is not a WS_CHILD window. This
// combination of things allows us to find the owning window
// with GA_ROOTOWNER. GA_ROOT will get us ourselves, and
// GA_PARENT will return the desktop HWND.
//
// See GH#13066
if (const auto ownerHwnd{ ::GetAncestor(pseudoHwnd, GA_ROOTOWNER) })
{
// We have an owner from a previous call to ReparentWindow