Create a window process for the tray icon #10980

leonMSFT · 2021-08-18T20:47:26Z

Currently, the monarch window will show itself when opening the tray icon context menu. This is because a window must be set as the foreground window when the context menu opens, otherwise the menu won't be able to be dismissed when clicking outside of the context menu.

This PR makes the tray icon create a non visible/interactable window for the sole purpose of being set as the foreground window when the tray icon's context menu is opened. Then none of the terminal windows should be set as the foreground window when opening the context menu.

Closes #10936

… tray icon, I just need to set it as foreground

leonMSFT · 2021-08-18T20:48:40Z

src/cascadia/WindowsTerminal/TrayIcon.cpp

+                                     WS_DISABLED,
+                                     CW_USEDEFAULT,
+                                     CW_USEDEFAULT,
+                                     CW_USEDEFAULT,
+                                     CW_USEDEFAULT,
+                                     HWND_MESSAGE,


Wasn't actually sure what were the necessary options to make this window process as un-interactable as possible, but I believe WS_DISABLED and maybe HWND_MESSAGE should be good?

IIRC, HWND_MESSAGE is enough to make it totally uninteractable. Though, in the past we've also done:

hwnd = CreateWindowExW( 0, PSEUDO_WINDOW_CLASS, nullptr, WS_OVERLAPPEDWINDOW, 0, 0, 0, 0, HWND_DESKTOP, nullptr, nullptr, nullptr);

zadjii-msft

I really only care about the wil::unique_hwnd thing. Rest looks good.

zadjii-msft · 2021-08-18T20:59:10Z

src/cascadia/WindowsTerminal/TrayIcon.h

@@ -32,6 +35,7 @@ class TrayIcon
 private:
    HMENU _CreateTrayContextMenu(winrt::Windows::Foundation::Collections::IMapView<uint64_t, winrt::hstring> peasants);

+    HWND _trayIconWndProc;


naming this WndProc seems weird - makes me thing it's a handle to the actual wndproc function itself. maybe just

Suggested change

HWND _trayIconWndProc;

HWND _trayIconHwnd;

zadjii-msft · 2021-08-18T20:59:44Z

src/cascadia/WindowsTerminal/TrayIcon.cpp

@@ -20,9 +22,41 @@ TrayIcon::TrayIcon(const HWND owningHwnd) :

 TrayIcon::~TrayIcon()
 {
+    DestroyWindow(_trayIconWndProc);


If you make this member a wil::unique_hwnd, it'll clean itself up for you 😉

zadjii-msft · 2021-08-18T21:01:30Z

src/cascadia/WindowsTerminal/TrayIcon.cpp

+                                     WS_DISABLED,
+                                     CW_USEDEFAULT,
+                                     CW_USEDEFAULT,
+                                     CW_USEDEFAULT,
+                                     CW_USEDEFAULT,
+                                     HWND_MESSAGE,


IIRC, HWND_MESSAGE is enough to make it totally uninteractable. Though, in the past we've also done:

hwnd = CreateWindowExW( 0, PSEUDO_WINDOW_CLASS, nullptr, WS_OVERLAPPEDWINDOW, 0, 0, 0, 0, HWND_DESKTOP, nullptr, nullptr, nullptr);

DHowett · 2021-08-18T21:34:01Z

src/cascadia/WindowsTerminal/TrayIcon.cpp

+{
+    WNDCLASSW wc{};
+    wc.hCursor = LoadCursor(nullptr, IDC_ARROW);
+    wc.hInstance = reinterpret_cast<HINSTANCE>(&__ImageBase);


Huh, i suspect this got copied from some other Cascadia stuff. Surprised it wasn't using a wil helper!

DHowett · 2021-08-19T23:11:27Z

src/cascadia/WindowsTerminal/TrayIcon.cpp

@@ -23,6 +23,37 @@ TrayIcon::~TrayIcon()
    RemoveIconFromTray();
 }

+void TrayIcon::CreateWindowProcess()


Suggested change

void TrayIcon::CreateWindowProcess()

void TrayIcon::CreateWindow()

This is sufficient -- it's not a process, after all! 😄

DHowett · 2021-08-19T23:14:49Z

src/cascadia/WindowsTerminal/TrayIcon.cpp

+
+LRESULT CALLBACK TrayIcon::_WindowProc(HWND window, UINT message, WPARAM wparam, LPARAM lparam) noexcept
+{
+    return DefWindowProc(window, message, wparam, lparam);


I think you can actually just literally set, lpfnWndProc = DefWindowProcW above! No need for all this ^^

vefatica · 2021-08-20T01:55:33Z

I don't know if it has been suggested or considered, but if the tray icon gets a window process, it might process WM_MOUSEMOVE by updating NOTIFYICONDATAW::szTip, perhaps with the caption of the current tab.

leonMSFT · 2021-08-20T18:07:04Z

I don't know if it has been suggested or considered, but if the tray icon gets a window process, it might process WM_MOUSEMOVE by updating NOTIFYICONDATAW::szTip, perhaps with the caption of the current tab.

Oh whoa that's a really weird interaction, I'm curious where you've encountered this!
Though, I've given the tray icon window WS_DISABLED to prevent it from receiving user input, and HWND_MESSAGE to make it invisible and ignore broadcast messages, so I believe it should be sufficient?

vefatica · 2021-08-20T18:47:42Z

Oh whoa that's a really weird interaction, I'm curious where you've encountered this!
Though, I've given the tray icon window WS_DISABLED to prevent it from receiving user input, and HWND_MESSAGE to make it invisible and ignore broadcast messages, so I believe it should be sufficient?

I've done it several times. In a plugin for a console app (TCC interpreter from JPSoft) I do these things (in various places)

WNDCLASS wndclass = {0}; // global

wndclass.lpfnWndProc = TrayMeWndProc;
wndclass.hInstance = hThisDll;
wndclass.lpszClassName = TRAYME_WND_CLASS;

// Icon and Tip added later
NOTIFYICONDATA nid = {sizeof(NOTIFYICONDATA), NULL, 0, NIF_TIP | NIF_MESSAGE | NIF_ICON, WM_TRAYMESSAGE, NULL, L"", 0};

// tried adding WS_DISABLED; it doesn't hurt
nid.hWnd = hWndTrayMe = CreateWindowEx (0, TRAYME_WND_CLASS, szTrayMeWindowName, 0, 0, 0, 0, 0, HWND_MESSAGE, NULL, NULL, NULL); 

// in TrayMeWndProc
case WM_MOUSEMOVE 
        //if ( (dwUtil = GetTickCount()) - dwLastUpdate > 500 ) // see note below
	{
		if ( GetConsoleTitle(szConTitle, MAX_PATH) > 127 ) // shorten with ellipsis in the middle
		{
		        nExtra = lstrlenW(szConTitle) - 127; // nExtra > 0
			szConTitle[61] = L' ';
			szConTitle[62] = szConTitle[63] = szConTitle[64] = L'.';
			szConTitle[65] = L' ';
			lstrcpy(szConTitle + 66, szConTitle + 66 + nExtra); // overlapping copy seems OK
		}
		lstrcpy(nid.szTip, szConTitle);
		Shell_NotifyIcon(NIM_MODIFY, &nid);
	        //dwLastUpdate = dwUtil; 
	}
	return 0;

I first did this in 2004 and on Windows XP and on a much slower machine. I don't recall if the timing mechanism (commented out) was necessary. It doesn't seem necessary today.

Since nid.szTip is only 128 WCHARs, I shorten the Caption if necessary. It winds up "first_part ... last_part".

I also process WM_LBUTTONDOWN (restore, tray icon only shows when minimized/hidden) and WM_RBUTTONDOWN (show a menu of things to do).

I hope that answers your question.

vefatica · 2021-08-20T18:51:17Z

I failed to mention that "case WM_MOUSEMOVE" (WM_LBUTTONDOWN and WM_RIGHTBUTTONDOWN too) is inside

switch (iMsg)
	{
		case WM_TRAYMESSAGE : 
			switch (lParam)

leonMSFT · 2021-08-20T19:19:09Z

Ahh, sorry I misunderstood, I was under the impression that you were trying to let me know that there may be a bug to watch out for where WM_MOUSEMOVE would update szTip, but now I understand that you were suggesting it could be a feature! 😄

vefatica · 2021-08-20T19:22:58Z

Right ... just suggesting a feature.

ghost · 2021-08-20T23:24:03Z

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

leonMSFT added 5 commits August 17, 2021 13:43

add window proc functions

1a4e782

maybe use parent hwnd?

7e1e60d

weirdly it turns out I don't even need to give the window proc to the…

d621e11

… tray icon, I just need to set it as foreground

ok turns out i do need to give it a windowproc

4cd113a

maybe I should do this as well?

0ffdcf4

ghost added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. labels Aug 18, 2021

leonMSFT commented Aug 18, 2021

View reviewed changes

zadjii-msft requested changes Aug 18, 2021

View reviewed changes

ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 18, 2021

DHowett reviewed Aug 18, 2021

View reviewed changes

unique_hwnd, wil helpers

59d15d6

ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 18, 2021

DHowett reviewed Aug 19, 2021

View reviewed changes

enhance!

0e162d8

DHowett approved these changes Aug 20, 2021

View reviewed changes

zadjii-msft approved these changes Aug 20, 2021

View reviewed changes

zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Aug 20, 2021

ghost merged commit 0c901ed into main Aug 20, 2021

ghost deleted the dev/lelian/notifyicon/withwindowproc branch August 20, 2021 23:24

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create a window process for the tray icon #10980

Create a window process for the tray icon #10980

leonMSFT commented Aug 18, 2021

leonMSFT Aug 18, 2021

zadjii-msft Aug 18, 2021

zadjii-msft left a comment

zadjii-msft Aug 18, 2021

zadjii-msft Aug 18, 2021

zadjii-msft Aug 18, 2021

DHowett Aug 18, 2021

DHowett Aug 19, 2021

DHowett Aug 19, 2021

vefatica commented Aug 20, 2021 •

edited

Loading

leonMSFT commented Aug 20, 2021

vefatica commented Aug 20, 2021

vefatica commented Aug 20, 2021 •

edited

Loading

leonMSFT commented Aug 20, 2021

vefatica commented Aug 20, 2021

ghost commented Aug 20, 2021

	void TrayIcon::CreateWindowProcess()
	void TrayIcon::CreateWindow()

Create a window process for the tray icon #10980

Create a window process for the tray icon #10980

Conversation

leonMSFT commented Aug 18, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zadjii-msft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vefatica commented Aug 20, 2021 • edited Loading

leonMSFT commented Aug 20, 2021

vefatica commented Aug 20, 2021

vefatica commented Aug 20, 2021 • edited Loading

leonMSFT commented Aug 20, 2021

vefatica commented Aug 20, 2021

ghost commented Aug 20, 2021

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

vefatica commented Aug 20, 2021 •

edited

Loading

vefatica commented Aug 20, 2021 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.