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

List overview All Threads

newer

older

[PATCH v2] oleaut32: Retrieve...

[PATCH 1/5] vcruntime140_1:...

Jiajin Cui

3 Jun 2020 3 Jun '20

3:39 a.m.

window with WS_CHILD does not need to change Z orderwhen SetWindowPos param hwndInsertAfter is HWND_TOPMOST. I wrote test program, and found it wasn't for specific application. Signed-off-by: cuijiajin --- dlls/user32/tests/win.c | 88 +++++++++++++++++++++++++++++++++++++++++ dlls/user32/winpos.c | 6 +++ 2 files changed, 94 insertions(+)

Attachments:

attachment.htm (text/html — 552 bytes)
v3-0001-user32-Fix-z-order-error-when-child-window-want-t.patch (application/octet-stream — 4.4 KB)

Show replies by date

Marvin

3 Jun 3 Jun

4 a.m.

Hi,

While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?

Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=72681

Your paranoid android.

=== w1064v1809 (32 bit report) ===

user32: win.c:3251: Test failed: GetActiveWindow() = 0003004C win.c:3254: Test failed: GetFocus() = 00000000 win.c:3266: Test failed: GetFocus() = 00000000 win.c:3269: Test failed: GetFocus() = 00000000 win.c:3272: Test failed: GetFocus() = 00000000 win.c:3275: Test failed: GetActiveWindow() = 0003004C win.c:3279: Test failed: GetFocus() = 00000000 win.c:3282: Test failed: GetFocus() = 00000000 win.c:3976: Test failed: hwnd 00020174/000E0178 message 0737 win.c:3981: Test failed: hwnd 000E0178/000E0178 message 0202 win.c:3986: Test failed: hwnd 000E0178/000E0178 message 0203 win.c:3990: Test failed: message 0202 available

=== w1064v1809_2scr (32 bit report) ===

user32: win.c:3251: Test failed: GetActiveWindow() = 0004007C win.c:3254: Test failed: GetFocus() = 00000000 win.c:3266: Test failed: GetFocus() = 00000000 win.c:3269: Test failed: GetFocus() = 00000000 win.c:3272: Test failed: GetFocus() = 00000000 win.c:3275: Test failed: GetActiveWindow() = 0004007C win.c:3279: Test failed: GetFocus() = 00000000 win.c:3282: Test failed: GetFocus() = 00000000 win.c:3976: Test failed: hwnd 000200E2/000C0328 message 0737 win.c:3981: Test failed: hwnd 000C0328/000C0328 message 0202 win.c:3986: Test failed: hwnd 000C0328/000C0328 message 0203 win.c:3990: Test failed: message 0202 available win.c:10061: Test failed: pos = 00e700d0 win.c:10065: Test failed: pos = 00e700d0 win.c:10069: Test failed: pos = 00e700d0

=== w1064v1809_ar (32 bit report) ===

user32: win.c:9565: Test failed: Timed out waiting for the child process win.c:10061: Test failed: pos = 00e700d0 win.c:10065: Test failed: pos = 00e700d0 win.c:10069: Test failed: pos = 00e700d0

=== w1064v1809_ja (32 bit report) ===

user32: win.c:3811: Test failed: message 0738 available win.c:3936: Test failed: hwnd 000202FE message 7fff win.c:4015: Test failed: hwnd 000202FE/003002DA message 7fff win.c:4018: Test failed: hwnd 000202FE/003002DA message 7fff

=== w1064v1809_zh_CN (32 bit report) ===

user32: win.c:9550: Test failed: didn't get start_event win.c:9491: Test failed: transparent window didn't get WM_NCHITTEST message win.c:9492: Test failed: button under static window didn't get WM_LBUTTONUP win.c:3936: Test failed: hwnd 00010382 message 0282 win.c:4015: Test failed: hwnd 00010382/000D03CC message 0282 win.c:4018: Test failed: hwnd 00010382/000D03CC message 0282

=== w1064v1809 (64 bit report) ===

user32: win.c:3251: Test failed: GetActiveWindow() = 0000000000030296 win.c:3254: Test failed: GetFocus() = 0000000000000000 win.c:3266: Test failed: GetFocus() = 0000000000000000 win.c:3269: Test failed: GetFocus() = 0000000000000000 win.c:3272: Test failed: GetFocus() = 0000000000000000 win.c:3275: Test failed: GetActiveWindow() = 0000000000030296 win.c:3279: Test failed: GetFocus() = 0000000000000000 win.c:3282: Test failed: GetFocus() = 0000000000000000 win.c:3976: Test failed: hwnd 00000000000200E4/000000000012007E message 0737 win.c:3981: Test failed: hwnd 000000000012007E/000000000012007E message 0202 win.c:3986: Test failed: hwnd 000000000012007E/000000000012007E message 0203 win.c:3990: Test failed: message 0202 available

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

=== debiant (32 bit French report) ===

user32: win.c:3042: Test succeeded inside todo block: 000D00A2: expected next 00000000, got 00000000 win.c:3042: Test succeeded inside todo block: 000D00A2: expected prev 000A00A4, got 000A00A4 win.c:3042: Test succeeded inside todo block: 000D00A2: expected owner 00000000, got 00000000 win.c:3048: Test succeeded inside todo block: 000D00A2: expected next 000C00AC, got 000C00AC win.c:3048: Test succeeded inside todo block: 000D00A2: expected prev 00000000, got 00000000 win.c:3048: Test succeeded inside todo block: 000D00A2: expected owner 00000000, got 00000000 win.c:3054: Test succeeded inside todo block: 000D00A2: expected next 000C00AC, got 000C00AC win.c:3054: Test succeeded inside todo block: 000D00A2: expected prev 000A00A4, got 000A00A4 win.c:3054: Test succeeded inside todo block: 000D00A2: expected owner 00000000, got 00000000 win.c:3060: Test succeeded inside todo block: 000D00A2: expected next 00000000, got 00000000 win.c:3060: Test succeeded inside todo block: 000D00A2: expected prev 000A00A4, got 000A00A4 win.c:3060: Test succeeded inside todo block: 000D00A2: expected owner 00000000, got 00000000 win.c:3066: Test succeeded inside todo block: 000D00A2: expected next 00000000, got 00000000 win.c:3066: Test succeeded inside todo block: 000D00A2: expected prev 000A00A4, got 000A00A4 win.c:3066: Test succeeded inside todo block: 000D00A2: expected owner 00000000, got 00000000 win.c:3072: Test succeeded inside todo block: 000D00A2: expected next 00000000, got 00000000 win.c:3072: Test succeeded inside todo block: 000D00A2: expected prev 000A00A4, got 000A00A4 win.c:3072: Test succeeded inside todo block: 000D00A2: expected owner 00000000, got 00000000 win.c:3078: Test succeeded inside todo block: 000D00A2: expected next 00000000, got 00000000 win.c:3078: Test succeeded inside todo block: 000D00A2: expected prev 000A00A4, got 000A00A4 win.c:3078: Test succeeded inside todo block: 000D00A2: expected owner 00000000, got 00000000 win.c:3084: Test succeeded inside todo block: 000D00A2: expected next 000C00AC, got 000C00AC win.c:3084: Test succeeded inside todo block: 000D00A2: expected prev 00000000, got 00000000 win.c:3084: Test succeeded inside todo block: 000D00A2: expected owner 00000000, got 00000000 win.c:3090: Test succeeded inside todo block: 000D00A2: expected next 000C00AC, got 000C00AC win.c:3090: Test succeeded inside todo block: 000D00A2: expected prev 000A00A4, got 000A00A4 win.c:3090: Test succeeded inside todo block: 000D00A2: expected owner 00000000, got 00000000 win.c:3096: Test succeeded inside todo block: 000D00A2: expected next 00000000, got 00000000 win.c:3096: Test succeeded inside todo block: 000D00A2: expected prev 000A00A4, got 000A00A4 win.c:3096: Test succeeded inside todo block: 000D00A2: expected owner 00000000, got 00000000

=== debiant (32 bit Japanese:Japan report) ===

=== debiant (32 bit Chinese:China report) ===

=== debiant (32 bit WoW report) ===

=== debiant (64 bit WoW report) ===

Zebediah Figura

5:13 a.m.

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

1680

Age (days ago)

1680

Last active (days ago)

wine-devel@winehq.org

2 comments

3 participants

tags (0)

participants (3)

Jiajin Cui
Marvin
Zebediah Figura