[PATCH] comctl32/tests: Fix combobox height after SetItemHeight, SetWindowPos.

[Marvin]

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

Your paranoid android.

=== debian10 (32 bit report) ===

comctl32:
combo.c:1379: Test failed: SetWindowPos() failed. Height (6)

=== debian10 (32 bit French report) ===

comctl32:
combo.c:1379: Test failed: SetWindowPos() failed. Height (6)

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

comctl32:
combo.c:1379: Test failed: SetWindowPos() failed. Height (6)

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

comctl32:
combo.c:1379: Test failed: SetWindowPos() failed. Height (6)

=== debian10 (32 bit WoW report) ===

comctl32:
combo.c:1379: Test failed: SetWindowPos() failed. Height (6)

=== debian10 (64 bit WoW report) ===

comctl32:
combo.c:1379: Test failed: SetWindowPos() failed. Height (6)

[Stefan Dösinger]

Hi,

Thank you for the test! As the testbot (Marvin) wrote, the test is failing because Wine does not yet behave correctly. You can handle it in the test with todo_wine, like so:

todo_wine ok(height > 6, "SetWindowPos() failed. Height (%d)\n", height);

Cheers,
Stefan
> Am 23.11.2019 um 23:19 schrieb Thales :
>
> <0001-comctl32-tests-Fix-combobox-height-after-SetItemHeig.patch>

[Nikolay Sivov]

Hi, thanks for the patch. Some comments below.
> - HWND hCombo;
> - HFONT hFont;
> - int height;
> -
> - /* This test requires the WC_COMBOBOXA style.  It won't work for WC_COMBOBOXEXA */
> - hCombo = CreateWindowExA(0, WC_COMBOBOXA, NULL, WS_BORDER | WS_VISIBLE | WS_CHILD | CBS_DROPDOWN, 0, 0, 300, 300,
> -
>                            hComboExParentWnd, NULL, hMainHinst, NULL);
> -
> - /* Add items to the combobox */
> - SendMessageA(hCombo, (UINT)CB_ADDSTRING, (WPARAM)0, (LPARAM)"Item 0");
> - SendMessageA(hCombo, (UINT)CB_ADDSTRING, (WPARAM)0, (LPARAM)"Item 1");
> - SendMessageA(hCombo, (UINT)CB_ADDSTRING, (WPARAM)0, (LPARAM)"Item 2");
> - SendMessageA(hCombo, (UINT)CB_ADDSTRING, (WPARAM)0, (LPARAM)"Item 3");
> - SendMessageA(hCombo, CB_SETCURSEL, 1, 0);

I don't think this is necessary. You don't need any string to trigger 
this feature.
> -
> - /* Create large enough font for test to work. */
> - hFont = CreateFontA(17, 0, 0, 0, FW_DONTCARE, FALSE, FALSE, FALSE, ANSI_CHARSET, OUT_DEFAULT_PRECIS,
> -
>                    CLIP_DEFAULT_PRECIS, DEFAULT_QUALITY, DEFAULT_PITCH|FF_DONTCARE, "Marlett");
> - SendMessageA(hCombo, WM_SETFONT, (WPARAM)hFont, FALSE);
> -
> - /* Set Item Height followed by SetWindowPos() is the test */
> - SendMessageA(hCombo, CB_SETITEMHEIGHT, -1, 4);
> -
> - /* SetWindowPos() should set the height according to font size*/
> - SetWindowPos(hCombo,NULL, 10,10,150,20, SWP_SHOWWINDOW);
> - height = SendMessageA(hCombo, CB_GETITEMHEIGHT, -1, 0);
> - ok(height > 6, "SetWindowPos() failed. Height (%d)\n", height);

You don't need special font either I think. The point is to set item 
height smaller than its natural default, and then trigger window size 
update. So you could set height to a half of current item height to the 
same effect I believe.
> -
> - /* Cleanup */
> - DestroyWindow(hCombo);
> - DeleteObject(hFont);

P.S. subject line is inaccurate because patch does not fix anything. 
Same test would apply to user32 implementation too.

[Nikolay Sivov]

Regarding possible fix for this, I expect it's doing recursive 
SetWindowPos() call on WM_SIZE, when cy is "not right".

[Thales]

Thanks for the feedback everyone!  I made the changes requested, except for this:
@Nikolay Sivov  -- About the subject.  I used the word "fix" with the idea that is in the "imperative" voice, as in "fix this problem."    How should I word the subject?
Thanks!...John Alway    On Sunday, November 24, 2019, 02:57:56 PM CST, Nikolay Sivov  wrote:

Regarding possible fix for this, I expect it's doing recursive 
SetWindowPos() call on WM_SIZE, when cy is "not right".

[Thales]

I should point out that I renamed the patch the following:
"[PATCH] comctl32/tests: Check combobox height after SetItemHeight SetWindowPos."

I posted the updated patch on Nov 25.
Regards,...John Alway

On Sunday, November 24, 2019, 07:49:59 PM CST, Thales  wrote:

Thanks for the feedback everyone!  I made the changes requested, except for this:
@Nikolay Sivov  -- About the subject.  I used the word "fix" with the idea that is in the "imperative" voice, as in "fix this problem."    How should I word the subject?
Thanks!...John Alway    On Sunday, November 24, 2019, 02:57:56 PM CST, Nikolay Sivov  wrote:

Regarding possible fix for this, I expect it's doing recursive 
SetWindowPos() call on WM_SIZE, when cy is "not right".