[PATCH v7 0/2] MR11135: win32u/tests: add test for swp_owner_popups self-reference fix
Add test_swp_owner_popups to verify that swp_owner_popups restores initial_after when it becomes the window itself. When an owned popup is immediately above its owner in z-order and is moved after the owner, the function incorrectly sets after = hwnd (self-reference). The fix restores initial_after, and the test verifies this via WM_WINDOWPOSCHANGED hwndInsertAfter capture. Change-Id: I1f63fc4046d65ea767beafbcae4f6f26b25a26ba Signed-off-by: Jiajin Cui <cuijiajin@uniontech.com> -- v7: win32u: restore initial after value in swp_owner_popups win32u/tests: add test for swp_owner_popups self-reference fix https://gitlab.winehq.org/wine/wine/-/merge_requests/11135
From: Jiajin Cui <cuijiajin@uniontech.com> Add test_swp_owner_popups to verify that swp_owner_popups restores initial_after when it becomes the window itself. When an owned popup is immediately above its owner in z-order and is moved after the owner, the function incorrectly sets after = hwnd (self-reference). The fix restores initial_after, and the test verifies this via WM_WINDOWPOSCHANGED hwndInsertAfter capture. Signed-off-by: Jiajin Cui <cuijiajin@uniontech.com> --- dlls/win32u/tests/win32u.c | 147 +++++++++++++++++++++++++++++++++++++ 1 file changed, 147 insertions(+) diff --git a/dlls/win32u/tests/win32u.c b/dlls/win32u/tests/win32u.c index a67b918626d..6d25500339b 100644 --- a/dlls/win32u/tests/win32u.c +++ b/dlls/win32u/tests/win32u.c @@ -3053,6 +3053,152 @@ void test_NtUserGetPointerDeviceRects( const char *arg ) ok( EqualRect( &display, &screen ), "display %s, expected %s\n", wine_dbgstr_rect( &display ), wine_dbgstr_rect( &screen ) ); } + +static WNDPROC test_swp_owner_popup_prev_proc; +static HWND test_swp_owner_captured_insert_after; +static HWND test_swp_owner_target_hwnd; + +static LRESULT CALLBACK test_swp_owner_popup_wndproc(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) +{ + if (msg == WM_WINDOWPOSCHANGED && hwnd == test_swp_owner_target_hwnd) + { + WINDOWPOS *wp2 = (WINDOWPOS *)lp; + test_swp_owner_captured_insert_after = wp2->hwndInsertAfter; + } + return CallWindowProcA(test_swp_owner_popup_prev_proc, hwnd, msg, wp, lp); +} + +static void test_swp_owner_popups(void) +{ + HWND owner, popup1, popup2, other1, other2; + HWND prev; + WNDCLASSA cls = {0}; + BOOL ret; + + cls.lpfnWndProc = DefWindowProcA; + cls.hInstance = GetModuleHandleA(NULL); + cls.lpszClassName = "TestOwnerPopups"; + + ok( RegisterClassA(&cls), "RegisterClassA failed\n" ); + + owner = CreateWindowA("TestOwnerPopups", "owner", WS_OVERLAPPEDWINDOW, + 0, 0, 100, 100, NULL, NULL, GetModuleHandleA(NULL), NULL); + ok( owner != NULL, "Failed to create owner window\n" ); + + popup1 = CreateWindowExA(0, "TestOwnerPopups", "popup1", + WS_POPUP | WS_VISIBLE, 0, 0, 50, 50, + owner, NULL, GetModuleHandleA(NULL), NULL); + ok( popup1 != NULL, "Failed to create popup1\n" ); + + popup2 = CreateWindowExA(0, "TestOwnerPopups", "popup2", + WS_POPUP | WS_VISIBLE, 0, 0, 50, 50, + owner, NULL, GetModuleHandleA(NULL), NULL); + ok( popup2 != NULL, "Failed to create popup2\n" ); + + /* Unrelated top-level window, used as a non-owner hwndInsertAfter target */ + other1 = CreateWindowA("TestOwnerPopups", "other1", WS_OVERLAPPEDWINDOW, + 200, 0, 100, 100, NULL, NULL, GetModuleHandleA(NULL), NULL); + ok( other1 != NULL, "Failed to create other1 window\n" ); + + /* Unrelated top-level window, used as a non-owner hwndInsertAfter target */ + other2 = CreateWindowA("TestOwnerPopups", "other2", WS_OVERLAPPEDWINDOW, + 200, 0, 100, 100, NULL, NULL, GetModuleHandleA(NULL), NULL); + ok( other2 != NULL, "Failed to create other2 window\n" ); + + /* Bring owner to top; owned popups should be reordered above owner */ + ret = SetWindowPos(owner, HWND_TOP, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE); + ok( ret, "SetWindowPos(owner, HWND_TOP) failed\n" ); + + /* z-order should be: popup2 > popup1 > owner */ + prev = GetWindow(owner, GW_HWNDPREV); + ok( prev == popup1, "expected popup1 (%p) below owner (%p), got %p\n", + popup1, owner, prev); + prev = GetWindow(popup1, GW_HWNDPREV); + ok( prev == popup2, "expected popup2 (%p) below popup1 (%p), got %p\n", + popup2, popup1, prev); + + /* + * Test the bug fix: swp_owner_popups restoring initial_after when + * after becomes hwnd itself. + * + * z-order: popup2 > popup1 > owner. Move popup1 after owner. Since popup1 + * is immediately above owner, owner's predecessor is popup1 itself, causing + * after = hwnd (self-reference). With the fix, after is restored to + * initial_after (owner). + */ + test_swp_owner_target_hwnd = popup1; + test_swp_owner_popup_prev_proc = (WNDPROC)SetWindowLongPtrA(popup1, GWLP_WNDPROC, + (LONG_PTR)test_swp_owner_popup_wndproc); + ok( test_swp_owner_popup_prev_proc != NULL, "Failed to subclass popup1\n" ); + + test_swp_owner_captured_insert_after = NULL; + ret = SetWindowPos(popup1, owner, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE | SWP_NOACTIVATE); + ok( ret, "[case1] SetWindowPos(popup1, owner) failed\n" ); + + /* Verify that hwndInsertAfter is not the window itself, should be owner */ + todo_wine ok( test_swp_owner_captured_insert_after == owner, + "hwndInsertAfter should be owner (%p), got %p\n", + owner, test_swp_owner_captured_insert_after); + + /* Verify z-order: popup2 should still be the topmost owned popup above owner */ + prev = GetWindow(owner, GW_HWNDPREV); + todo_wine ok( prev == popup2, + "[case1] window above owner should be popup2 (%p), got %p\n", + popup2, prev); + + /* popup1 was inserted right after owner, so owner should be directly above popup1 */ + prev = GetWindow(popup1, GW_HWNDPREV); + todo_wine ok( prev == owner, + "[case1] window above popup1 should be owner (%p), got %p\n", + owner, prev); + + /* + * Test inserting popup1 after a non-owner window ('other1'). + * swp_owner_popups should detect that popup1 is an owned popup and + * adjust hwndInsertAfter so popup1 stays above its owner. + */ + test_swp_owner_captured_insert_after = NULL; + ret = SetWindowPos(popup1, other1, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE | SWP_NOACTIVATE); + ok( ret, "[case2] SetWindowPos(popup1, other) failed\n" ); + + /* hwndInsertAfter should be adjusted to popup2 to keep popup1 above owner */ + todo_wine ok( test_swp_owner_captured_insert_after == popup2, + "[case2] captured hwndInsertAfter should be popup2 (%p), got %p\n", + popup2, test_swp_owner_captured_insert_after); + + /* Verify z-order: popup1 should remain above owner */ + prev = GetWindow(owner, GW_HWNDPREV); + ok( prev == popup1, + "[case2] window above owner should be popup1 (%p), got %p\n", + popup1, prev); + + /* + * Repeat the same SetWindowPos call to verify the behavior is stable + * and idempotent when popup1 is already above its owner. + */ + test_swp_owner_captured_insert_after = NULL; + ret = SetWindowPos(popup1, other2, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE | SWP_NOACTIVATE); + ok( ret, "[case3] SetWindowPos(popup1, other) failed\n" ); + + /* hwndInsertAfter should again be adjusted to other */ + todo_wine ok( test_swp_owner_captured_insert_after == other2, + "[case3] captured hwndInsertAfter should still be other2 (%p), got %p\n", + other2, test_swp_owner_captured_insert_after); + + /* Verify z-order: popup2 should still be above owner */ + prev = GetWindow(owner, GW_HWNDPREV); + todo_wine ok( prev == popup2, + "[case3] window above owner should still be popup2 (%p), got %p\n", + popup2, prev); + + /* Cleanup: restore original wndproc */ + SetWindowLongPtrA(popup1, GWLP_WNDPROC, (LONG_PTR)test_swp_owner_popup_prev_proc); + DestroyWindow(other1); + DestroyWindow(other2); + DestroyWindow(popup2); + DestroyWindow(popup1); + DestroyWindow(owner); +} START_TEST(win32u) { @@ -3107,6 +3253,7 @@ START_TEST(win32u) test_timer(); test_inter_process_messages( argv[0] ); test_wndproc_hook(); + test_swp_owner_popups(); test_NtUserCloseWindowStation(); test_NtUserDisplayConfigGetDeviceInfo(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11135
From: Jiajin Cui <cuijiajin@uniontech.com> If the after parameter was changed to the window itself during z-order processing, restore it to the initial value before returning. This prevents self-referencing z-order values that could cause issues in subsequent window positioning operations. Signed-off-by: Jiajin Cui <cuijiajin@uniontech.com> --- dlls/user32/tests/win.c | 2 -- dlls/win32u/tests/win32u.c | 12 ++++++------ dlls/win32u/window.c | 4 +++- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c index 6ef13eaeaa3..b6b90116017 100644 --- a/dlls/user32/tests/win.c +++ b/dlls/user32/tests/win.c @@ -12060,7 +12060,6 @@ static void reset_window_state(HWND *state, int n) if (state[i]) { swp_after(state[i], HWND_NOTOPMOST); - todo_wine_if(i == 5) /* FIXME: remove once Wine is fixed */ ok(!is_topmost(state[i]), "%d: hwnd %p is still topmost\n", i, state[i]); swp_after(state[i], HWND_TOP); } @@ -12287,7 +12286,6 @@ static void test_topmost(void) ok(!is_topmost(hwnd), "hwnd should NOT be topmost\n"); ok(!is_topmost(hwnd_child), "child should NOT be topmost\n"); ok(!is_topmost(hwnd_child2), "child2 should NOT be topmost\n"); - todo_wine ok(!is_topmost(hwnd_grandchild), "grandchild should NOT be topmost\n"); check_z_order(hwnd, hwnd2, 0, owner, FALSE); check_z_order(hwnd_child, hwnd_child2, 0, hwnd, FALSE); diff --git a/dlls/win32u/tests/win32u.c b/dlls/win32u/tests/win32u.c index 6d25500339b..2409ff449fa 100644 --- a/dlls/win32u/tests/win32u.c +++ b/dlls/win32u/tests/win32u.c @@ -3136,19 +3136,19 @@ static void test_swp_owner_popups(void) ok( ret, "[case1] SetWindowPos(popup1, owner) failed\n" ); /* Verify that hwndInsertAfter is not the window itself, should be owner */ - todo_wine ok( test_swp_owner_captured_insert_after == owner, + ok( test_swp_owner_captured_insert_after == owner, "hwndInsertAfter should be owner (%p), got %p\n", owner, test_swp_owner_captured_insert_after); /* Verify z-order: popup2 should still be the topmost owned popup above owner */ prev = GetWindow(owner, GW_HWNDPREV); - todo_wine ok( prev == popup2, + ok( prev == popup2, "[case1] window above owner should be popup2 (%p), got %p\n", popup2, prev); /* popup1 was inserted right after owner, so owner should be directly above popup1 */ prev = GetWindow(popup1, GW_HWNDPREV); - todo_wine ok( prev == owner, + ok( prev == owner, "[case1] window above popup1 should be owner (%p), got %p\n", owner, prev); @@ -3162,7 +3162,7 @@ static void test_swp_owner_popups(void) ok( ret, "[case2] SetWindowPos(popup1, other) failed\n" ); /* hwndInsertAfter should be adjusted to popup2 to keep popup1 above owner */ - todo_wine ok( test_swp_owner_captured_insert_after == popup2, + ok( test_swp_owner_captured_insert_after == popup2, "[case2] captured hwndInsertAfter should be popup2 (%p), got %p\n", popup2, test_swp_owner_captured_insert_after); @@ -3181,13 +3181,13 @@ static void test_swp_owner_popups(void) ok( ret, "[case3] SetWindowPos(popup1, other) failed\n" ); /* hwndInsertAfter should again be adjusted to other */ - todo_wine ok( test_swp_owner_captured_insert_after == other2, + ok( test_swp_owner_captured_insert_after == other2, "[case3] captured hwndInsertAfter should still be other2 (%p), got %p\n", other2, test_swp_owner_captured_insert_after); /* Verify z-order: popup2 should still be above owner */ prev = GetWindow(owner, GW_HWNDPREV); - todo_wine ok( prev == popup2, + ok( prev == popup2, "[case3] window above owner should still be popup2 (%p), got %p\n", popup2, prev); diff --git a/dlls/win32u/window.c b/dlls/win32u/window.c index 7c17b32787e..f1ed107d826 100644 --- a/dlls/win32u/window.c +++ b/dlls/win32u/window.c @@ -3787,7 +3787,7 @@ static BOOL fixup_swp_flags( WINDOWPOS *winpos, const RECT *old_window_rect, int */ static HWND swp_owner_popups( HWND hwnd, HWND after ) { - HWND owner, *list = NULL; + HWND owner, *list = NULL, initial_after = after; unsigned int i; TRACE( "(%p) after = %p\n", hwnd, after ); @@ -3857,6 +3857,8 @@ static HWND swp_owner_popups( HWND hwnd, HWND after ) done: free( list ); + + if(after == hwnd) after = initial_after; /* restore initial after value if it was changed to the window itself */ return after; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11135
{width=900 height=77} {width=900 height=336} If swp_owner_popups returns its own window, it will cause continuous recursion later. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11135#note_144506
participants (2)
-
Jiajin Cui -
Jiajin Cui (＠jin-king1)