[PATCH v2 0/2] MR5034: explorer: Systray icons regression fixes.
-- v2: explorer: Set layered style on systray icons only when it's actually layered. explorer: Don't activate the systray icon when showing it. https://gitlab.winehq.org/wine/wine/-/merge_requests/5034
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Fixes a regression introduced by 62c6646d8f44cee55ffdef6d2bede19f681dc9b8, because SetParent will unconditionally activate the window, causing newly added icons to deactivate the foreground window. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- programs/explorer/systray.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/programs/explorer/systray.c b/programs/explorer/systray.c index 3b4e5bdc8ac..7d2d3795575 100644 --- a/programs/explorer/systray.c +++ b/programs/explorer/systray.c @@ -540,6 +540,15 @@ static LRESULT WINAPI tray_icon_wndproc( HWND hwnd, UINT msg, WPARAM wparam, LPA break; } + case WM_WINDOWPOSCHANGING: + if (icon->display == ICON_DISPLAY_HIDDEN) + { + /* Changing the icon's parent via SetParent would activate it, stealing the focus. */ + WINDOWPOS *wp = (WINDOWPOS*)lparam; + wp->flags |= SWP_NOACTIVATE; + } + break; + case WM_WINDOWPOSCHANGED: update_systray_balloon_position(); break; @@ -563,9 +572,9 @@ static void systray_add_icon( struct icon *icon ) if (icon->display != ICON_DISPLAY_HIDDEN) return; /* already added */ - icon->display = nb_displayed++; SetWindowLongW( icon->window, GWL_STYLE, GetWindowLongW( icon->window, GWL_STYLE ) | WS_CHILD ); SetParent( icon->window, tray_window ); + icon->display = nb_displayed++; pos = get_icon_pos( icon ); SetWindowPos( icon->window, 0, pos.x, pos.y, 0, 0, SWP_NOSIZE | SWP_NOACTIVATE | SWP_NOZORDER | SWP_SHOWWINDOW ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5034
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Fixes a regression introduced by 229b4561d9a8f10cbb49342dff0b6a3472d81c68, which caused the icons to not be visible initially in the virtual desktop systray. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- programs/explorer/systray.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/programs/explorer/systray.c b/programs/explorer/systray.c index 7d2d3795575..1d8a83536ca 100644 --- a/programs/explorer/systray.c +++ b/programs/explorer/systray.c @@ -621,6 +621,7 @@ static BOOL show_icon(struct icon *icon) { icon->display = ICON_DISPLAY_DOCKED; icon->layered = TRUE; + SetWindowLongW( icon->window, GWL_EXSTYLE, GetWindowLongW( icon->window, GWL_EXSTYLE ) | WS_EX_LAYERED ); SendMessageW( icon->window, WM_SIZE, SIZE_RESTORED, MAKELONG( icon_cx, icon_cy ) ); } systray_add_icon( icon ); @@ -642,6 +643,7 @@ static BOOL hide_icon(struct icon *icon) { icon->display = ICON_DISPLAY_HIDDEN; icon->layered = FALSE; + SetWindowLongW( icon->window, GWL_EXSTYLE, GetWindowLongW( icon->window, GWL_EXSTYLE ) & ~WS_EX_LAYERED ); } ShowWindow( icon->window, SW_HIDE ); systray_remove_icon( icon ); @@ -729,7 +731,7 @@ static BOOL add_icon(NOTIFYICONDATAW *nid) icon->owner = nid->hWnd; icon->display = ICON_DISPLAY_HIDDEN; - CreateWindowExW( WS_EX_LAYERED, tray_icon_class.lpszClassName, NULL, WS_CLIPSIBLINGS | WS_POPUP, + CreateWindowExW( 0, tray_icon_class.lpszClassName, NULL, WS_CLIPSIBLINGS | WS_POPUP, 0, 0, icon_cx, icon_cy, 0, NULL, NULL, icon ); if (!icon->window) ERR( "Failed to create systray icon window\n" ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5034
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=142837 Your paranoid android. === w10pro64 (32 bit report) === user32: win.c:3818: Test failed: GetForegroundWindow returned 000202D0 win.c:3749: Test failed: SetForegroundWindow failed, error 0 win.c:3752: Test failed: GetForegroundWindow returned 000202D0 win.c:3789: Test failed: GetForegroundWindow returned 000202D0 win.c:3877: Test failed: GetActiveWindow() = 0002021C win.c:3881: Test failed: GetFocus() = 00000000 win.c:3884: Test failed: GetFocus() = 00000000 === w10pro64 (64 bit report) === user32: win.c:3818: Test failed: GetForegroundWindow returned 00000000000202A0 win.c:3749: Test failed: SetForegroundWindow failed, error 0 win.c:3752: Test failed: GetForegroundWindow returned 00000000000202A0 win.c:3789: Test failed: GetForegroundWindow returned 00000000000202A0 win.c:3877: Test failed: GetActiveWindow() = 00000000000201A8 win.c:3881: Test failed: GetFocus() = 0000000000000000 win.c:3884: Test failed: GetFocus() = 0000000000000000
I find a bit surprising that changing a window parent would activate it (actually, activate its parent?). There was a discussion elsewhere, about whether the standalone systray window should be created with WS_EX_NOACTIVATE, to avoid it stealing focus (although I believe winex11 has trouble handling NOACTIVATE anyway), would that be enough? Maybe it should be the same for the icons? Do you have a specific scenario where this happens? Is this with the taskbar or with the standalone systray? Regarding the layered windows, I think it should be able to draw layered windows regardless of whether they are in the host dock or not? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5034#note_60722
On Fri Feb 9 16:01:21 2024 +0000, Rémi Bernon wrote:
I find a bit surprising that changing a window parent would activate it (actually, activate its parent?). There was a discussion elsewhere, about whether the standalone systray window should be created with WS_EX_NOACTIVATE, to avoid it stealing focus (although I believe winex11 has trouble handling NOACTIVATE anyway), would that be enough? Maybe it should be the same for the icons? Do you have a specific scenario where this happens? Is this with the taskbar or with the standalone systray? Regarding the layered windows, I think it should be able to draw layered windows regardless of whether they are in the host dock or not? Yes, I tried WS_EX_NOACTIVATE but it doesn't work. It only prevents focus stealing from stuff like user input, not from APIs.
I was surprised it sets focus too, but it seems it's by design. The `set_window_pos` in `NtUserSetParent` (line 475) doesn't have SWP_NOACTIVATE. Can you not reproduce it? I mean all you have to do is open a window in a virtual desktop, then launch an app in the same desktop that has a systray like a basic AutoHotkey script. It shouldn't steal focus, but it does. Here's an example of an AutoHotkey script to test this easily: ``` #NoEnv *$F12:: Sleep 1000 Reload return ``` Launch notepad in a virtual desktop, then launch the script with AutoHotkey. Focus notepad again. Then press F12. After 1 second Notepad will lose focus (because the ahk systray icon gets removed and re-added). BTW I've no idea what you mean with layered windows. I admit I don't understand this code, but what's the purpose of the `layered` flag? Because by default, the window is created with WS_EX_LAYERED but the layered flag is FALSE… To reproduce the second issue, just launch the virtual desktop with the AutoHotkey script above. You should not see the systray icon, although it is there (you can right click its location for example), the icon is just not visible. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5034#note_60723
On Fri Feb 9 16:01:20 2024 +0000, Gabriel Ivăncescu wrote:
Yes, I tried WS_EX_NOACTIVATE but it doesn't work. It only prevents focus stealing from stuff like user input, not from APIs. I was surprised it sets focus too, but it seems it's by design. The `set_window_pos` in `NtUserSetParent` (line 475) doesn't have SWP_NOACTIVATE. Can you not reproduce it? I mean all you have to do is open a window in a virtual desktop, then launch an app in the same desktop that has a systray like a basic AutoHotkey script. It shouldn't steal focus, but it does. Here's an example of an AutoHotkey script to test this easily: ``` #NoEnv *$F12:: Sleep 1000 Reload return ``` Launch notepad in a virtual desktop, then launch the script with AutoHotkey. Focus notepad again. Then press F12. After 1 second Notepad will lose focus (because the ahk systray icon gets removed and re-added). BTW I've no idea what you mean with layered windows. I admit I don't understand this code, but what's the purpose of the `layered` flag? Because by default, the window is created with WS_EX_LAYERED but the layered flag is FALSE… To reproduce the second issue, just launch the virtual desktop with the AutoHotkey script above. You should not see the systray icon, although it is there (you can right click its location for example), the icon is just not visible. Yeah sorry, I got confused by the Linux WM state which is still showing the window as active somehow (but indeed Wine then doesn't dispatch the input).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5034#note_60731
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5034
participants (3)
-
Gabriel Ivăncescu -
Marvin -
Rémi Bernon