[PATCH v8 0/2] MR11135: win32u: fix the z-order issue when swp_owner_popups returns its own window

[Jiajin Cui (＠jin-king1)]

By adding parameters to test WM_WINDOWPOSCHANGED, when hwndInsertAfter in swp_owner_popups returns the window itself, it should be modified to the previously prepared hwndInsertAfter, and the final window stacking order should also be after the previously set hwndInsertAfter, rather than before its owner.

--
  v8: 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

[PATCH v8 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 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

[PATCH v8 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/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 1ecdca6c7d8..7ea9bce2706 100644
--- a/dlls/win32u/window.c
+++ b/dlls/win32u/window.c
@@ -3829,7 +3829,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 );
@@ -3899,6 +3899,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