From 696fd1196bf851a5e411cd9b7958083e2bff28fa Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 29 Aug 2024 16:19:15 -0500 Subject: [PATCH] Fix two ConPTY HWND focus issues (#17828) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 17a55da0f9889aafd84df024299982e7b94f5482) Service-Card-Id: PVTI_lADOAF3p4s4AmhmQzgSwIkE Service-Version: 1.22 --- src/cascadia/WindowsTerminal/IslandWindow.cpp | 9 +++++++++ src/interactivity/base/InteractivityFactory.cpp | 10 ++++++++++ 2 files changed, 19 insertions(+) diff --git a/src/cascadia/WindowsTerminal/IslandWindow.cpp b/src/cascadia/WindowsTerminal/IslandWindow.cpp index 2bfe9d1dd96..e0b13ded48f 100644 --- a/src/cascadia/WindowsTerminal/IslandWindow.cpp +++ b/src/cascadia/WindowsTerminal/IslandWindow.cpp @@ -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) diff --git a/src/interactivity/base/InteractivityFactory.cpp b/src/interactivity/base/InteractivityFactory.cpp index 0af2cb76d11..862b4db514d 100644 --- a/src/interactivity/base/InteractivityFactory.cpp +++ b/src/interactivity/base/InteractivityFactory.cpp @@ -463,6 +463,7 @@ using namespace Microsoft::Console::Interactivity; { _WritePseudoWindowCallback((bool)wParam); } + return 0; } case WM_GETOBJECT: { @@ -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);