Fix two ConPTY HWND focus issues #17828

zadjii-msft · 2024-08-29T18:31:32Z

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 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

this is what Evan said to do

src/interactivity/base/InteractivityFactory.cpp

carlos-zamora

Really just want resolution on Leonard's comment before ✅

carlos-zamora · 2024-08-29T20:21:07Z

src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp

@@ -171,6 +171,7 @@ LRESULT NonClientIslandWindow::_InputSinkMessageHandler(UINT const message,
        // is absolutely critical to making sure Snap Layouts (GH#9443) works!
        return _dragBarNcHitTest({ GET_X_LPARAM(lparam), GET_Y_LPARAM(lparam) });
    }
+


nit: can undo this change

Suggested change

DHowett · 2024-09-04T19:48:57Z

Marking up for 1.21 (consider) and 1.22 (pick)

@ekoschik

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

@ekoschik

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_lADOAF3p4s4AmhmszgSwIkA Service-Version: 1.21

zadjii-msft added 4 commits August 28, 2024 12:44

Handle WM_ENABLE too

04428ca

this is what Evan said to do

Try something similar

222bae3

test

83ea10c

Another thing to try

0df3e0a

microsoft-github-policy-service bot added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-Windowing Window frame, quake mode, tearout Product-Conpty For console issues specifically related to conpty labels Aug 29, 2024

lhecker approved these changes Aug 29, 2024

View reviewed changes

src/interactivity/base/InteractivityFactory.cpp Outdated Show resolved Hide resolved

carlos-zamora reviewed Aug 29, 2024

View reviewed changes

zadjii-msft added 2 commits August 29, 2024 15:41

Should we handle this?

6aa60aa

whitespace

9bece0b

zadjii-msft enabled auto-merge (squash) August 29, 2024 20:45

carlos-zamora approved these changes Aug 29, 2024

View reviewed changes

zadjii-msft merged commit 17a55da into main Aug 29, 2024
18 of 20 checks passed

zadjii-msft deleted the dev/migrie/a-focus-thing branch August 29, 2024 21:19

zadjii-msft mentioned this pull request Aug 29, 2024

Cannot type in new pane or tab until focus is manually activated with mouse click #13388

Open

zadjii-msft mentioned this pull request Sep 5, 2024

Graphic artifacts (small gray squares) on the desktop when the window is minimized #15219

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix two ConPTY HWND focus issues #17828

Fix two ConPTY HWND focus issues #17828

zadjii-msft commented Aug 29, 2024

carlos-zamora left a comment

carlos-zamora Aug 29, 2024

DHowett commented Sep 4, 2024

Fix two ConPTY HWND focus issues #17828

Fix two ConPTY HWND focus issues #17828

Conversation

zadjii-msft commented Aug 29, 2024

Bug the first: the MSAL window ixptools spawns

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

carlos-zamora left a comment

Choose a reason for hiding this comment

carlos-zamora Aug 29, 2024

Choose a reason for hiding this comment

DHowett commented Sep 4, 2024

Bug the first: the MSAL window `ixptools` spawns

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