From: Zebediah Figura <z.figura12@gmail.com>
Subject: Re: [PATCH v5 2/2] user32:Fix z order error when WS_CHILD window want to be topmost windows.【请注意，邮件由wine-devel-bounces@winehq.org代发】
Message-Id: <0d146502-5901-3ac1-1b93-f461a93b610e@gmail.com>
Date: Sun, 28 Jun 2020 10:09:47 -0500
In-Reply-To: <2020062218031463491716@uniontech.com>
References: <2020062218031463491716@uniontech.com>

Hello Jiajin,

On 6/22/20 5:03 AM, Jiajin Cui wrote:
> diff --git a/dlls/user32/winpos.c b/dlls/user32/winpos.c
> index b92a20df18..da07b18282 100644
> --- a/dlls/user32/winpos.c
> +++ b/dlls/user32/winpos.c
> @@ -1998,12 +1998,13 @@ static BOOL fixup_flags( WINDOWPOS *winpos, const RECT *old_window_rect, int par
>      }
>      else if (winpos->hwndInsertAfter == HWND_TOPMOST)
>      {
> -        if ((wndPtr->dwExStyle & WS_EX_TOPMOST) && GetWindow(winpos->hwnd, GW_HWNDFIRST) == winpos->hwnd)
> +        if (((wndPtr->dwExStyle & WS_EX_TOPMOST) && GetWindow(winpos->hwnd, GW_HWNDFIRST) == winpos->hwnd) ||
> +            ((wndPtr->dwStyle & (WS_CHILD | WS_POPUP)) == WS_CHILD))
>              winpos->flags |= SWP_NOZORDER;
>      }
>      else if (winpos->hwndInsertAfter == HWND_NOTOPMOST)
>      {
> -        if (!(wndPtr->dwExStyle & WS_EX_TOPMOST))
> +        if (!(wndPtr->dwExStyle & WS_EX_TOPMOST) || ((wndPtr->dwStyle & (WS_CHILD | WS_POPUP)) == WS_CHILD))
>              winpos->flags |= SWP_NOZORDER;
>      }
>      else

While this does address my concern regarding HWND_NOTOPMOST, it still
doesn't address the other concerns I brought up in [1]. In particular:

* Does SetWindowPos() even succeed? I.e. I think it would be a good idea
to test its return value.

* The fixed up flags are passed to WM_WINDOWPOSCHANGED. We have a good
framework in msg.c that will make it easy to test which flags are passed
to WM_WINDOWPOSCHANGED. Assuming that SetWindowPos() does succeed, I
would recommend adding a test in msg.c to ensure that removing
SWP_NOZORDER is correct.

[1] https://www.winehq.org/pipermail/wine-devel/2020-June/167321.html

On 6/22/20 6:22 AM, Marvin wrote:
> === debiant (32 bit report) ===
> 
> user32:
> combo.c:698: Test failed: 00000002: got 00000080
> combo.c:704: Test failed: 00000002: got 00000080
> combo.c:698: Test failed: 00000003: got 00000080
> combo.c:704: Test failed: 00000003: got 00000080

It seems this error is caused by this patch (the others are probably
spurious, but this one is not; I verified locally). You'll need to fix
it (possibly with another patch) before this patch can be accepted.