Skip to content

Commit

Permalink
Fix two ConPTY HWND focus issues (#17828)
Browse files Browse the repository at this point in the history
Worked with @ekoschik on this one.

## Bug the first: the MSAL window `ixptools` spawns

> The auth prompt in pwsh.exe is disabling the terminal window while its
opened and re-enabling it when the window closes. BUT it is enabling
Terminal after dismissing itself, instead of before, which means
terminal is disabled when activated.
>
> Terminal wants focus on the ISLAND window (a grandchild; island is
parented to bridge, which is parented to terminal’s TLW). When it is
activated, it gets a `WM_SETFOCUS` (in response to DefWindowProc
`WM_ACTIVATE`). From `WM_SETFOCUS` it calls `SetFocus` on the bridge
window, and similarly the bridge calls `SetFocus` on the island.
>
> If the TLW is disabled, these `SetFocus` calls fail (see [this
check](#internal-link-redacted) in `SetFocus`). In the case above, this
leaves Terminal’s TLW as focus, and it doesn’t handle keyboard input.
Note that the window IS foreground/active, but because focus is not on
the island it doesn’t see the keyboard input. Another thing to note is
that clicking on the space to the right of the tabs does NOT revive
keyboard input, but clicking on the tabs or main area does.

> **I recommend having the TLW handle WM_ENABLE and call SetFocus on the
island window.**

And guess what, that works!

## Bug the second: When sublime text is the git `EDITOR`, it doesn't
toss focus back to the Terminal

> In this case, Sublime is calling SFW on the pseudo console window. I
don’t have its code, but it is presumably doing something like
SetForegroundWindow(GetConsoleWindow()). This queues an event to the
pseudo window, and when that event is processed the pseudo window
becomes the active and focus window on the queue (which is shared with
Terminal).
>
> The sublime window dismisses itself and does the above SFW call.
Dismissing immediately activates the Terminal TLW, which does the
triple-focus dance (TLW sets focus on itself, then bridge, then island).
This completes but is overwritten immediately when the pseudo window
activates itself. Note that the pseudo window is active at this point
(not the terminal window).

> **I recommend having the Pseudo console window handle WM_ACTIVATE by
calling SetFocus on the island window (and not passing the message to
DefWindowProc).**

And guess what, that works!

----

Closes #15956 (I did test this)
This might be related to #13388, we'll have folks try canary and check

(cherry picked from commit 17a55da)
Service-Card-Id: PVTI_lADOAF3p4s4AmhmQzgSwIkE
Service-Version: 1.22
  • Loading branch information
zadjii-msft authored and DHowett committed Sep 4, 2024
1 parent 649d8b2 commit 696fd11
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 0 deletions.
9 changes: 9 additions & 0 deletions src/cascadia/WindowsTerminal/IslandWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,15 @@ long IslandWindow::_calculateTotalSize(const bool isWidth, const long clientSize
_HandleCreateWindow(wparam, lparam);
return 0;
}
case WM_ENABLE:
{
if (_interopWindowHandle != nullptr)
{
// send focus to the child window
SetFocus(_interopWindowHandle);
}
break;
}
case WM_SETFOCUS:
{
if (_interopWindowHandle != nullptr)
Expand Down
10 changes: 10 additions & 0 deletions src/interactivity/base/InteractivityFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,7 @@ using namespace Microsoft::Console::Interactivity;
{
_WritePseudoWindowCallback((bool)wParam);
}
return 0;
}
case WM_GETOBJECT:
{
Expand All @@ -476,6 +477,15 @@ using namespace Microsoft::Console::Interactivity;
}
return 0;
}
case WM_ACTIVATE:
{
if (const auto ownerHwnd{ ::GetAncestor(hWnd, GA_ROOTOWNER) })
{
SetFocus(ownerHwnd);
return 0;
}
break;
}
}
// If we get this far, call the default window proc
return DefWindowProcW(hWnd, Message, wParam, lParam);
Expand Down

0 comments on commit 696fd11

Please sign in to comment.