[PATCH v3] user32:Fix z order error when child window want to be topmost windows

[Zebediah Figura]

Hello, thanks for the patch. I have some comments inlined:

> === 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

Unfortunately I expect you'll need to debug and fix this. Fortunately,
the other errors are probably spurious...

> win.c:3042: Test succeeded inside todo block: 001000D4: expected next 00000000, got 00000000
> win.c:3042: Test succeeded inside todo block: 001000D4: expected prev 000D00D6, got 000D00D6
> win.c:3042: Test succeeded inside todo block: 001000D4: expected owner 00000000, got 00000000
> win.c:3048: Test succeeded inside todo block: 001000D4: expected next 000E00DE, got 000E00DE
> win.c:3048: Test succeeded inside todo block: 001000D4: expected prev 00000000, got 00000000
> win.c:3048: Test succeeded inside todo block: 001000D4: expected owner 00000000, got 00000000
> win.c:3054: Test succeeded inside todo block: 001000D4: expected next 000E00DE, got 000E00DE
> win.c:3054: Test succeeded inside todo block: 001000D4: expected prev 000D00D6, got 000D00D6
> win.c:3054: Test succeeded inside todo block: 001000D4: expected owner 00000000, got 00000000
> win.c:3060: Test succeeded inside todo block: 001000D4: expected next 00000000, got 00000000
> win.c:3060: Test succeeded inside todo block: 001000D4: expected prev 000D00D6, got 000D00D6
> win.c:3060: Test succeeded inside todo block: 001000D4: expected owner 00000000, got 00000000
> win.c:3066: Test succeeded inside todo block: 001000D4: expected next 00000000, got 00000000
> win.c:3066: Test succeeded inside todo block: 001000D4: expected prev 000D00D6, got 000D00D6
> win.c:3066: Test succeeded inside todo block: 001000D4: expected owner 00000000, got 00000000
> win.c:3072: Test succeeded inside todo block: 001000D4: expected next 00000000, got 00000000
> win.c:3072: Test succeeded inside todo block: 001000D4: expected prev 000D00D6, got 000D00D6
> win.c:3072: Test succeeded inside todo block: 001000D4: expected owner 00000000, got 00000000
> win.c:3078: Test succeeded inside todo block: 001000D4: expected next 00000000, got 00000000
> win.c:3078: Test succeeded inside todo block: 001000D4: expected prev 000D00D6, got 000D00D6
> win.c:3078: Test succeeded inside todo block: 001000D4: expected owner 00000000, got 00000000
> win.c:3084: Test succeeded inside todo block: 001000D4: expected next 000E00DE, got 000E00DE
> win.c:3084: Test succeeded inside todo block: 001000D4: expected prev 00000000, got 00000000
> win.c:3084: Test succeeded inside todo block: 001000D4: expected owner 00000000, got 00000000
> win.c:3090: Test succeeded inside todo block: 001000D4: expected next 000E00DE, got 000E00DE
> win.c:3090: Test succeeded inside todo block: 001000D4: expected prev 000D00D6, got 000D00D6
> win.c:3090: Test succeeded inside todo block: 001000D4: expected owner 00000000, got 00000000
> win.c:3096: Test succeeded inside todo block: 001000D4: expected next 00000000, got 00000000
> win.c:3096: Test succeeded inside todo block: 001000D4: expected prev 000D00D6, got 000D00D6
> win.c:3096: Test succeeded inside todo block: 001000D4: expected owner 00000000, got 00000000

If you're introducing the fix at the same time as the tests, you don't
want to use todo_wine.

That said, it's often better to split patches up, sending the tests
first and the fix second, so that you can show exactly what gets fixed.

> diff --git a/dlls/user32/winpos.c b/dlls/user32/winpos.c
> index b92a20df18..9c8576e07b 100644
> --- a/dlls/user32/winpos.c
> +++ b/dlls/user32/winpos.c
> @@ -1983,6 +1983,12 @@ static BOOL fixup_flags( WINDOWPOS *winpos, const RECT *old_window_rect, int par
>          }
>      }
>  
> +    if ((wndPtr->dwStyle & (WS_CHILD | WS_POPUP)) == WS_CHILD
> +        && winpos->hwndInsertAfter == HWND_TOPMOST )
> +    {
> +        winpos->flags |= SWP_NOZORDER;
> +    }
> +
>      /* Check hwndInsertAfter */
>      if (winpos->flags & SWP_NOZORDER) goto done;
> 

This solution seems suspicious, and is possibly even demonstrably
(in)correct:

* If SetWindowPos() seems to do nothing, does it even succeed in this
case? (I.e. what is the return value, and last error if applicable?)

* The fixed up flags are passed to WM_WINDOWPOSCHANGED. Is SWP_NOZORDER
really set?

Finally, should we add similar treatment for HWND_NOTOPMOST?

> 
> -- 
> 2.20.1
>