Skip to content

Commit

Permalink
Merge branch 'dev/migrie/b/13066-ws_popup-route' into dev/migrie/b/13…
Browse files Browse the repository at this point in the history
…066-SW_FLASH-2
  • Loading branch information
zadjii-msft committed May 18, 2022
2 parents f1dda12 + ed5222c commit b3e2987
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 8 deletions.
19 changes: 17 additions & 2 deletions src/host/PtySignalInputThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ void PtySignalInputThread::ConnectConsole() noexcept
{
_DoShowHide(_initialShowHide->show);
}

// We should have successfully used the _earlyReparent message in CreatePseudoWindow.
}

// Method Description:
Expand Down Expand Up @@ -234,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));
}
}

Expand Down
8 changes: 7 additions & 1 deletion src/interactivity/base/InteractivityFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,13 @@ 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.
const auto windowStyle = WS_OVERLAPPEDWINDOW;
//
// 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;

// Attempt to create window.
Expand Down
18 changes: 13 additions & 5 deletions src/terminal/adapter/InteractDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -199,11 +199,19 @@ 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.
if (const auto ownerHwnd{ ::GetAncestor(pseudoHwnd, GA_ROOT) })
// BODGY
//
// 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

Expand Down

1 comment on commit b3e2987

@github-actions

This comment was marked as duplicate.

Please sign in to comment.