microsoft / terminal

The new Windows Terminal and the original Windows console host, all in the same place!
MIT License
94.85k stars 8.21k forks source link

`OpenConsole.exe` can steal focus from the terminal #17168

Closed tusharsnx closed 11 hours ago

tusharsnx commented 4 months ago

Windows Terminal version

Windows Terminal Dev

Windows build number

10.0.22631.0

Other Software

No response

Steps to reproduce

  1. Open an instance of WT and keep it in the background (without focus).
  2. In Visual Studio, make a scratch project (code doesn't affect repro):
    #include <iostream>
    int main() {
    std::cout << "hello world";
    std::cin.get(); // to keep the program from exiting
    return 0;
    }
  3. Start the program by clicking on the green play button (Start without debugging).
  4. WT starts but without focus.

Expected Behavior

WT starts with focus.

Actual Behavior

WT starts without focus.

github-actions[bot] commented 4 months ago

Hi I'm an AI powered bot that finds similar issues based off the issue title.

Please view the issues below to see if they solve your problem, and if the issue describes your problem please consider closing this one and thumbs upping the other issue to help us prioritize it. Thank you!

Open similar issues:

Note: You can give me feedback by thumbs upping or thumbs downing this comment.

tusharsnx commented 4 months ago

Trace

I repeated step (3) 4 times to get better data, and indeed three instances of openconsole.exe, and all of them stole focus from the terminal just after starting up. The 3rd time it didn't steal the focus, proving that it might not always happen.

image

I was investigating https://github.com/microsoft/wslg/issues/1212 which also deals with the same area of focus lost issue. They shared a trace profile .wprp which other than collecting other data, also gathers window focus traces, and it helped me discover that OpenConsole.exe sometimes steals focus.

foregroundTracing1.zip

zadjii-msft commented 4 months ago

I'm guessing what you've done here is basically investigate #13388. That bug has been my white whale for... really some long time now. I've spent months trying to trace or get consistent enough repros to find something out.

I'm 99% sure the code that creates that window is in https://github.com/microsoft/terminal/blob/32fbb16d43ddfcdd039a61c5af051a16b972faeb/src/interactivity/base/InteractivityFactory.cpp#L297. @ekoschik looked into this for a while to try and figure out why that WS_POPUP, WS_EX_TOOLWINDOW | WS_EX_TRANSPARENT | WS_EX_LAYERED | WS_EX_NOACTIVATE window still gets foreground - because it doesn't make any sense.

Maybe you'll have better luck? If you've got a consistent repro, then maybe there's more traces we could have you collect? I'll reach out to him and see.

zadjii-msft commented 4 months ago

16014 also had some windowing changes that I wanted to experiment with, but I also didn't have a consistent repro, so they were all guesses. If you've got a consistent repro, maybe you might be able to get a better signal out of that PR?

(though, admittedly, that PR was mostly there to address the tiny boxes that sometimes show up in the top-right corner of the monitor, ala #15219)

lhecker commented 4 months ago

re: https://github.com/microsoft/terminal/pull/16014#discussion_r1334836949 As an experiment we could still try to turn it into a HWND_MESSAGE window in e.g. the 1.22 canary. I know you said that it needs WS_OVERLAPPEDWINDOW because otherwise it can't be minimized but in my testing a HWND_MESSAGE could receive SW_MINIMIZE just fine. If it doesn't work after all, we'll at least know for certain that HWND_MESSAGE cannot be used after all.

zadjii-msft commented 4 months ago

in my testing a HWND_MESSAGE could receive SW_MINIMIZE just fine. If it doesn't work after all, we'll at least know for certain that HWND_MESSAGE cannot be used after all

I think part of the problem (and this is me trying to remember what Evan was explaining a year back) was that it can receive a SW_MINIMIZE, but the window isn't actually minimized. Like, message windows have different state that's tracked by user32. So if you query a message window after a SW_MINIMIZE, user32 doesn't actually think that window's minimized. Something like that? (it's been a while now).

I forget why we didn't ever merge that PR in the past, but we should totally just do that as soon as 1.21 forks

lhecker commented 4 months ago

(Oh, it may be worth mentioning for other readers of this issue that I responded over in the mentioned PR, where I felt like it's more appropriate. Here: https://github.com/microsoft/terminal/pull/16014#discussion_r1586293642)

tusharsnx commented 4 months ago

Based on #16014, I tried all window (extended/non-extended) styles in islolation, and nothing seems to stop the child window from stealing parent focus. What am I missing here?

Code ```cpp #include #include "string" static constexpr auto parentWindowClass = L"ParentWindow"; static constexpr auto childWindowClass = L"ChildWindow"; LRESULT static _stdcall WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam) { if (message == WM_DESTROY) { PostQuitMessage(0); } return DefWindowProc(hWnd, message, wParam, lParam); } ATOM MyRegisterClass(const std::wstring& szWindowClass) { WNDCLASSEXW wcex{ 0 }; wcex.cbSize = sizeof(WNDCLASSEX); wcex.lpfnWndProc = WndProc; wcex.cbWndExtra = 0; wcex.lpszClassName = &szWindowClass[0]; return RegisterClassExW(&wcex); } int _stdcall wWinMain(HINSTANCE hInstance, HINSTANCE /*hPrevInstance*/, LPWSTR /*lpCmdLine*/, int nCmdShow) { const auto parentClassName = reinterpret_cast(MyRegisterClass(parentWindowClass)); const auto childClassName = reinterpret_cast(MyRegisterClass(childWindowClass)); // Parent window HWND hWnd = CreateWindowW(parentClassName, nullptr, WS_OVERLAPPEDWINDOW, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, nullptr, nullptr, nullptr, nullptr); if (!hWnd) { return FALSE; } ShowWindow(hWnd, nCmdShow); UpdateWindow(hWnd); // Child window // using WS_POPUP makes the window invisible, but doesn't stop the focus shifting. const auto styles = WS_OVERLAPPED | (WS_MINIMIZEBOX | WS_SYSMENU); // - using WS_EX_LAYERED makes the window invisible, but doesn't stop the focus shifting. // - WS_EX_NOACTIVATE doesn't stop the focus shifting too. const auto exStyles = WS_EX_TRANSPARENT | WS_EX_NOACTIVATE; HWND childHWnd = CreateWindowExW(exStyles, childClassName, nullptr, styles, 0, 0, 0, 0, hWnd, nullptr, nullptr, nullptr); ShowWindow(childHWnd, nCmdShow); UpdateWindow(childHWnd); // SetForegroundWindow(hWnd); // This SETS parent (not ownership) even when WS_CHILD isn't used??? // // SetParent also seems to put the focus to the child hwnd. Another opportunity for focus shifting. // SetParent(childHWnd, hWnd); // Message loop MSG msg; while (GetMessage(&msg, nullptr, 0, 0)) { TranslateMessage(&msg); DispatchMessage(&msg); } return (int) msg.wParam; } ```
lhecker commented 4 months ago

I believe the problem is that you need to pass SW_SHOWNOACTIVATE for the child's ShowWindow call. That's what we do in conhost's ITerminalApi: https://github.com/microsoft/terminal/blob/a0d1329d7aabf72301a0cdaec420bd165a6545d3/src/host/outputStream.cpp#L223

zadjii-msft commented 3 months ago

wait @tusharsnx did the 👍 you gave on https://github.com/microsoft/terminal/issues/17168#issuecomment-2093415691 actually mean "that worked"? I feel like we've been at a loss for so long on this, that I'm just desperate for any progress at all

juliocesarbt commented 2 months ago

Windows Terminal version

Windows Terminal Dev

Windows build number

10.0.22631.0

Other Software

No response

Steps to reproduce

  1. Open an instance of WT and keep it in the background (without focus).
  2. In Visual Studio, make a scratch project (code doesn't affect repro):
#include <iostream>
int main() {
  std::cout << "hello world";
  std::cin.get(); // to keep the program from exiting
  return 0;
}
  1. Start the program by clicking on the green play button (Start without debugging).
  2. WT starts but without focus.

Expected Behavior

WT starts with focus.

Actual Behavior

WT starts without focus.

You can try ANSI escape code

void Clear() { cout << "\x1B[2J\x1B[H"; }

zadjii-msft commented 1 day ago

@tusharsnx just to double check - is this fixed in the latest canary for you/?

tusharsnx commented 12 hours ago

@zadjii-msft Yes, I can't repro this in the latest canary (1.23.2481.0) 😃

zadjii-msft commented 11 hours ago

image