[PATCH v3 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> -- v3: 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 4481ffa1b32..8eff33c381f 100644 --- a/dlls/win32u/tests/win32u.c +++ b/dlls/win32u/tests/win32u.c @@ -3004,6 +3004,152 @@ static void test_NtUserRegisterWindowMessage(void) ok( !wcscmp( buf, L"#0xabc" ), "buf = %s\n", debugstr_w(buf) ); } +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) { char **argv; @@ -3039,6 +3185,7 @@ START_TEST(win32u) test_NtUserAlterWindowStyle(); test_NtUserCreateInputContext(); test_NtUserBuildHimcList(); + test_swp_owner_popups(); test_NtUserBuildHwndList(); test_NtUserBuildNameList(); test_cursoricon(); -- 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/win32u/tests/win32u.c | 2 +- dlls/win32u/window.c | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/dlls/win32u/tests/win32u.c b/dlls/win32u/tests/win32u.c index 8eff33c381f..e8c3ef510d6 100644 --- a/dlls/win32u/tests/win32u.c +++ b/dlls/win32u/tests/win32u.c @@ -3086,7 +3086,7 @@ 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); diff --git a/dlls/win32u/window.c b/dlls/win32u/window.c index c3a32dde6cf..900f6cf61f4 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
participants (2)
-
Jiajin Cui -
Jiajin Cui (＠jin-king1)