microsoft / terminal

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

System menu for Terminal shows up on wrong monitor #15421

Open zadjii-msft opened 1 year ago

zadjii-msft commented 1 year ago

From MSFT:41390752, courtesy of @oldnewthing

Open Terminal maximized on right-hand monitor of dual-monitor system. Press Alt+Space to open the system menu.

Expect: Menu appears at the top left corner of the window. Actual: Menu appears at the top right corner of the wrong monitor.

Screen shot shows menu opening on the wrong monitor.

Maybe this has to do with monitor layout? I suppose each of my monitors are set up a little higher than the one on the left: image

Sure enough, this layout repros: image

My monitors are nowhere near that exotically laid out.

image

When maximized on monitor 3 (the primary monitor), the system menu appears in the upper right of monitor 2.

michalnpl commented 1 year ago

I can confirm the monitor display layout is important. The issue does not repro on this layout:

image

but it reproes on both:

image

and

image

michalnpl commented 1 year ago

Getting closer using Inspect. For this configuration:

image

getting :PowersShell" window (HWND?)

image

Answering only to the desktop being out of bounds... almost like there is padding or a margin.

oldnewthing commented 1 year ago

Okay, I see what's going on. You are manually showing the system menu at the upper left corner of the GetWindowRect and didn't take into account the fact that maximized windows have borders which extend beyond the monitor. It never occurred to me that you would try to show the system menu yourself instead of just letting DefWindowProc do it. (Why not just let DefWindowProc do it?)

This also explains why the system menu shows too high. Regular apps show the system menu below the caption, but Terminal shows it over the caption. Put the system menu at the upper left client pixel (not the upper left window pixel) and I think you'll be all good again.

michalnpl commented 1 year ago

I've developed a fix for this issue. I tweaked the position calculations for the system menu, and now it behaves properly. I also added a bunch of error checking and error logging. Thanks for the pointers @oldnewthing!

I've tested manually all the cases that were working just fine before (and they're still good, no worries there) and also with the situations that were causing trouble. Happy to say it's all working correctly now.

I've run the TAEF tests too, and everything came out clean. I also checked the fullscreen mode (F11), and it's all good.

I lack insight into why DefWindowProc doesn't open system menu, but that is recognized in code:

https://github.com/microsoft/terminal/blob/c9e993a38ec04833e15edac230300948a0fab83b/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp#L955

case WM_NCRBUTTONUP:
        // The `DefWindowProc` function doesn't open the system menu for some
        // reason so we have to do it ourselves.
        if (wParam == HTCAPTION)
        {
            OpenSystemMenu(GET_X_LPARAM(lParam), GET_Y_LPARAM(lParam));
        }

Maybe @zadjii-msft can help here; it may be the same reason why some events have to be routed manually.

I was also wondering if this behavior of "borders which extend beyond the monitor" is caused by backward compatibility from DUI or DWM...

Anyway, I'd like to go ahead and create a PR with the fix if that's OK?

michalnpl commented 1 year ago

Ping.

DHowett commented 1 year ago

Oh, sorry @michalnpl! Mike went on paternity leave recently. I'm certain this is a fine fix--it is at least better than what we have--and would be glad to see it in PR. Please do.

Why not just let DefWindowProc do it?

As we always say, "I think there was a reason!" This was probably an early casualty of the weird stuff we had to do to interact with the DirectInput capture site that was making our lives difficult when we moved the tabs into the non-client area[^1].

OpenSystemMenu is the handler for the user-rebindable "open system menu" action, which they can unbind from Alt+Space. I'm not certain whether we can have DefWindowProc handle it for us (though I would very much like to!)

[^1]: which happened long before WinAppSDK/WinUI3 offered support for it, so it's very much homebrew and very much rough-hewn

michalnpl commented 1 year ago

Thanks @DHowett , PR created.