[PATCH v4 0/2] MR11135: win32u/tests: add test for swp_owner_popups self-reference fix

[Jiajin Cui (＠jin-king1)]

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 <[e-mail]>

--
  v4: win32u: restore initial after value in swp_owner_popups
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/11135

[PATCH v4 1/2] win32u/tests: add test for swp_owner_popups self-reference fix

[Jiajin Cui]

From: Jiajin Cui <[e-mail]>

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 <[e-mail]>
---
 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

[PATCH v4 2/2] win32u: restore initial after value in swp_owner_popups

[Jiajin Cui]

From: Jiajin Cui <[e-mail]>

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 <[e-mail]>
---
 dlls/win32u/tests/win32u.c | 12 ++++++------
 dlls/win32u/window.c       |  4 +++-
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/dlls/win32u/tests/win32u.c b/dlls/win32u/tests/win32u.c
index 8eff33c381f..ebfcf82a48e 100644
--- a/dlls/win32u/tests/win32u.c
+++ b/dlls/win32u/tests/win32u.c
@@ -3086,19 +3086,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);
 
@@ -3112,7 +3112,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);
 
@@ -3131,13 +3131,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 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