[PATCH 0/1] MR667: win32u: Ignore SetWindowPos() if it is trying to change topmost state for child window.
I think the behaviour of SetWindowPos() in this case is partially explained by the fact that topmost state is not strictly applicable to child windows and so Windows denies such requests. Child window can still end up with WS_EX_TOPMOST style if it was set at window creation (doesn't work this way in Wine now) or, e. g., like in the test added in the patch.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=20190
Also fixes game load dialog in Indiana Jones and the Infernal Machine.
From: Paul Gofman pgofman@codeweavers.com
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=20190 --- dlls/user32/tests/win.c | 56 +++++++++++++++++++++++++++++++++++++++-- dlls/win32u/window.c | 18 +++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-)
diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c index 4c2af09d0c6..82afff70887 100644 --- a/dlls/user32/tests/win.c +++ b/dlls/user32/tests/win.c @@ -3073,6 +3073,7 @@ static void test_SetWindowPos(HWND hwnd, HWND hwnd2) RECT orig_win_rc, rect; LONG_PTR old_proc; HWND hwnd_grandchild, hwnd_child, hwnd_child2; + DWORD style, ex_style; HWND hwnd_desktop; RECT rc_expected; RECT rc1, rc2; @@ -3170,7 +3171,7 @@ static void test_SetWindowPos(HWND hwnd, HWND hwnd2) ret = SetWindowPos(hwnd_child, HWND_NOTOPMOST, 1, 2, 3, 4, 0); ok(ret, "Got %d.\n", ret); GetWindowRect(hwnd_child, &rc2); - todo_wine ok(EqualRect(&rc1, &rc2), "%s != %s.\n", wine_dbgstr_rect(&rc1), wine_dbgstr_rect(&rc2)); + ok(EqualRect(&rc1, &rc2), "%s != %s.\n", wine_dbgstr_rect(&rc1), wine_dbgstr_rect(&rc2)); check_active_state(hwnd2, hwnd2, hwnd2); SetWindowPos(hwnd_child, HWND_NOTOPMOST, 0, 0, rc1.right - rc1.left, rc1.bottom - rc1.top, 0);
@@ -3178,7 +3179,58 @@ static void test_SetWindowPos(HWND hwnd, HWND hwnd2) ret = SetWindowPos(hwnd_child, HWND_TOPMOST, 1, 2, 3, 4, 0); ok(ret, "Got %d.\n", ret); GetWindowRect(hwnd_child, &rc2); - todo_wine ok(EqualRect(&rc1, &rc2), "%s != %s.\n", wine_dbgstr_rect(&rc1), wine_dbgstr_rect(&rc2)); + ok(EqualRect(&rc1, &rc2), "%s != %s.\n", wine_dbgstr_rect(&rc1), wine_dbgstr_rect(&rc2)); + check_active_state(hwnd2, hwnd2, hwnd2); + SetWindowPos(hwnd_child, HWND_TOPMOST, 0, 0, rc1.right - rc1.left, rc1.bottom - rc1.top, 0); + + /* Child windows with desktop window parent are not treated as child. */ + SetParent(hwnd_child, NULL); + ex_style = GetWindowLongA(hwnd_child, GWL_EXSTYLE); + ok(!(ex_style & WS_EX_TOPMOST), "got ex_style %#lx.\n", ex_style); + GetWindowRect(hwnd_child, &rc1); + rc_expected.left = rc1.left + 1; + rc_expected.top = rc1.top + 2; + rc_expected.right = rc1.left + 4; + rc_expected.bottom = rc1.top + 6; + ret = SetWindowPos(hwnd_child, HWND_TOPMOST, 1, 2, 3, 4, 0); + ok(ret, "Got %d.\n", ret); + GetWindowRect(hwnd_child, &rc2); + ok(EqualRect(&rc_expected, &rc2), "%s != %s.\n", + wine_dbgstr_rect(&rc_expected), wine_dbgstr_rect(&rc2)); + check_active_state(hwnd2, hwnd2, hwnd2); + SetWindowPos(hwnd_child, HWND_TOPMOST, 0, 0, rc1.right - rc1.left, rc1.bottom - rc1.top, 0); + + style = GetWindowLongA(hwnd_child, GWL_STYLE); + ok(style & WS_CHILD, "got style %#lx.\n", style); + ex_style = GetWindowLongA(hwnd_child, GWL_EXSTYLE); + ok(ex_style & WS_EX_TOPMOST, "got ex_style %#lx.\n", ex_style); + + SetParent(hwnd_child, hwnd); + ex_style = GetWindowLongA(hwnd_child, GWL_EXSTYLE); + ok(ex_style & WS_EX_TOPMOST, "got ex_style %#lx.\n", ex_style); + + SetParent(hwnd_child, NULL); + GetWindowRect(hwnd_child, &rc1); + rc_expected.left = rc1.left + 1; + rc_expected.top = rc1.top + 2; + rc_expected.right = rc1.left + 4; + rc_expected.bottom = rc1.top + 6; + ret = SetWindowPos(hwnd_child, HWND_NOTOPMOST, 1, 2, 3, 4, 0); + ok(ret, "Got %d.\n", ret); + GetWindowRect(hwnd_child, &rc2); + ok(EqualRect(&rc_expected, &rc2), "%s != %s.\n", + wine_dbgstr_rect(&rc_expected), wine_dbgstr_rect(&rc2)); + check_active_state(hwnd2, hwnd2, hwnd2); + SetWindowPos(hwnd_child, HWND_NOTOPMOST, 0, 0, rc1.right - rc1.left, rc1.bottom - rc1.top, 0); + SetParent(hwnd_child, hwnd); + ex_style = GetWindowLongA(hwnd_child, GWL_EXSTYLE); + ok(!(ex_style & WS_EX_TOPMOST), "got ex_style %#lx.\n", ex_style); + + GetWindowRect(hwnd_child, &rc1); + ret = SetWindowPos(hwnd_child, HWND_NOTOPMOST, 1, 2, 3, 4, 0); + ok(ret, "Got %d.\n", ret); + GetWindowRect(hwnd_child, &rc2); + ok(EqualRect(&rc1, &rc2), "%s != %s.\n", wine_dbgstr_rect(&rc1), wine_dbgstr_rect(&rc2)); check_active_state(hwnd2, hwnd2, hwnd2); SetWindowPos(hwnd_child, HWND_TOPMOST, 0, 0, rc1.right - rc1.left, rc1.bottom - rc1.top, 0);
diff --git a/dlls/win32u/window.c b/dlls/win32u/window.c index 0aa10d9ec65..e0b01f74be4 100644 --- a/dlls/win32u/window.c +++ b/dlls/win32u/window.c @@ -3387,6 +3387,24 @@ BOOL set_window_pos( WINDOWPOS *winpos, int parent_x, int parent_y ) if (!insertafter_parent) return FALSE; if (insertafter_parent != parent) return TRUE; } + else if (winpos->hwndInsertAfter == HWND_TOPMOST || winpos->hwndInsertAfter == HWND_NOTOPMOST) + { + BOOL is_child_window; + HWND parent; + WND *win; + + if ((win = get_win_ptr( winpos->hwnd )) && win != WND_DESKTOP && win != WND_OTHER_PROCESS) + { + is_child_window = (win->dwStyle & (WS_POPUP | WS_CHILD)) == WS_CHILD; + release_win_ptr( win ); + if (is_child_window && (parent = NtUserGetAncestor( winpos->hwnd, GA_PARENT )) + && parent != get_desktop_window()) + { + TRACE( "Not changing topmost state for a child window.\n" ); + return TRUE; + } + } + } }
/* Make sure that coordinates are valid for WM_WINDOWPOSCHANGING */
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=121467
Your paranoid android.
=== w1064 (32 bit report) ===
user32: win.c:10893: Test failed: pos = 014a015e
=== w1064_2qxl (64 bit report) ===
user32: win.c:4631: Test failed: hwnd 000000000003021A/000000000003021A message 0200 win.c:4635: Test failed: hwnd 000000000003021A/000000000003021A message 0201 win.c:4644: Test failed: hwnd 00000000001002C0/00000000001002C0 message 0202 win.c:4647: Test failed: hwnd 00000000001002C0/00000000001002C0 message 0201
=== w10pro64_en_AE_u8 (64 bit report) ===
user32: win.c:189: Test failed: didn't get start_event win.c:10351: Test failed: WindowFromPoint returned 0000000000020326, expected 0000000000000000 win.c:10359: Test failed: WindowFromPoint returned 0000000000050230, expected 0000000000000000 win.c:10288: Test failed: transparent window didn't get WM_NCHITTEST message
This is a duplicate of https://gitlab.winehq.org/wine/wine/-/merge_requests/419.
On 8/19/22 10:26, Jinoh Kang (@iamahuman) wrote:
This is a duplicate of https://gitlab.winehq.org/wine/wine/-/merge_requests/419.
Is it? I don't see my patch doing quite the same, or having much to do with message windows specifically. Also, the bug linked in my initial message has my initial version of the patch from 2019 (back then it was missing desktop window parent check and was breaking things due to that).
On Fri Aug 19 15:46:01 2022 +0000, **** wrote:
Paul Gofman replied on the mailing list:
On 8/19/22 10:26, Jinoh Kang (@iamahuman) wrote: > This is a duplicate of https://gitlab.winehq.org/wine/wine/-/merge_requests/419. > Is it? I don't see my patch doing quite the same, or having much to do with message windows specifically. Also, the bug linked in my initial message has my initial version of the patch from 2019 (back then it was missing desktop window parent check and was breaking things due to that). _______________________________________________ wine-gitlab mailing list -- wine-gitlab@winehq.org To unsubscribe send an email to wine-gitlab-leave@winehq.org
I stand corrected. Sorry.
As I also attempted to fix this in !5214 and got marked as a dupe, is there still any work being done here? Anything needed to get this to move forward? I am willing to help.
-
Jinoh Kang (＠iamahuman) -
Marvin -
Paul Gofman
-
Paul Gofman -
Paul Gofman (＠gofman)
-
Vaxry (＠vaxerski)