Skip to content

Fix globalSummon with monitor:any moving to wrong monitor when desktop:toCurrent #20485

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.

Sign up for GitHub

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

Open
Criseda (Criseda) wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix globalSummon with monitor:any moving to wrong monitor when desktop:toCurrent #20485

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 37 additions & 2 deletions src/cascadia/WindowsTerminal/AppHost.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -882,6 +882,20 @@ void AppHost::_WindowActivated(bool activated)

safe_void_coroutine AppHost::HandleSummon(const winrt::TerminalApp::SummonWindowBehavior args) const
{
// GH#20305: When monitor:"any" (InPlace) and desktop:"toCurrent" are both
// specified, MoveWindowToDesktop may reposition the window to the monitor
// of the current foreground window. Save the window position beforehand so
// we can restore it after the desktop move completes.
const auto isInPlace = args && args.ToMonitor() == winrt::TerminalApp::MonitorBehavior::InPlace;
const auto shouldRestorePosition = isInPlace && args.MoveToCurrentDesktop();

HWND hwnd = _window->GetHandle();
RECT savedWindowRect{};
if (shouldRestorePosition)
{
::GetWindowRect(hwnd, &savedWindowRect);
}

_window->SummonWindow(args);

if (!args || !args.MoveToCurrentDesktop())
Expand All @@ -895,6 +909,10 @@ safe_void_coroutine AppHost::HandleSummon(const winrt::TerminalApp::SummonWindow
co_return;
}

// Capture the dispatcher on the UI thread before we switch to the
// background thread. We'll need it to restore the window position.
auto dispatcher = _windowLogic.GetRoot().Dispatcher();

// Just like AppHost::GetVirtualDesktopId:
// IVirtualDesktopManager is cross-process COM into explorer.exe,
// and we can't use that on the UI thread.
Expand All @@ -904,7 +922,7 @@ safe_void_coroutine AppHost::HandleSummon(const winrt::TerminalApp::SummonWindow
// we are, then don't call MoveWindowToDesktop. This is to mitigate
// MSFT:33035972
BOOL onCurrentDesktop{ false };
if (SUCCEEDED(desktopManager->IsWindowOnCurrentVirtualDesktop(_window->GetHandle(), &onCurrentDesktop)) && onCurrentDesktop)
if (SUCCEEDED(desktopManager->IsWindowOnCurrentVirtualDesktop(hwnd, &onCurrentDesktop)) && onCurrentDesktop)
{
// If we succeeded, and the window was on the current desktop, then do nothing.
}
Expand All @@ -917,11 +935,28 @@ safe_void_coroutine AppHost::HandleSummon(const winrt::TerminalApp::SummonWindow
GUID currentlyActiveDesktop{ 0 };
if (VirtualDesktopUtils::GetCurrentVirtualDesktopId(&currentlyActiveDesktop))
{
LOG_IF_FAILED(desktopManager->MoveWindowToDesktop(_window->GetHandle(), currentlyActiveDesktop));
LOG_IF_FAILED(desktopManager->MoveWindowToDesktop(hwnd, currentlyActiveDesktop));
}
// If GetCurrentVirtualDesktopId failed, then just leave the window
// where it is. Nothing else to be done :/
}

// GH#20305: MoveWindowToDesktop may have moved our window to the
// foreground window's monitor. Restore the window to the monitor
// and position it was on before the summon.
if (shouldRestorePosition)
{
co_await wil::resume_foreground(dispatcher);

RECT currentRect;
if (::GetWindowRect(hwnd, &currentRect))
{
if (currentRect.left != savedWindowRect.left || currentRect.top != savedWindowRect.top)
{
::SetWindowPos(hwnd, 0, savedWindowRect.left, savedWindowRect.top, 0, 0, SWP_NOZORDER | SWP_NOSIZE | SWP_NOACTIVATE);
}
}
}
}

// Method Description:
Expand Down
Loading