[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 <thaleslv(a)yahoo.com>:
>
> <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 <nsivov(a)codeweavers.com> 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 <thaleslv(a)yahoo.com> 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 <nsivov(a)codeweavers.com> wrote:  
 
 Regarding possible fix for this, I expect it's doing recursive 
SetWindowPos() call on WM_SIZE, when cy is "not right".