Re: comctl32: CreateToolbarEx should send TB_SETBITMAPSIZE twice (fixes bug #6392)

[James Hawkins]

On 3/19/07, Mikołaj Zalewski <mikolaj(a)zalewski.pl> wrote:
> As can be seen in Spy++ and by the attached tests, CreateToolbarEx sends
> TB_SETBITMAPSIZE twice instead of TB_SETBITMAPSIZE and TB_SETBUTTONSIZE.
> ShellExViewer depends on this bug. The patch contains also some tests
> and fixes for d[xy]Button <= 0.
>
> From c8a84900ae5d9ca7d77d443990e935b8c268bf4c Mon Sep 17 00:00:00 2001
> From: =?utf-8?q?Miko=C5=82aj_Zalewski?= <mikolaj(a)zalewski.pl>
> Date: Mon, 19 Mar 2007 10:40:53 +0100
> Subject: [PATCH] comctl32: CreateToolbarEx should send TB_SETBITMAPSIZE twice (fixes bug #6392)
>
> ---
>  dlls/comctl32/commctrl.c      |   13 +++++++------
>  dlls/comctl32/tests/toolbar.c |   40 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 47 insertions(+), 6 deletions(-)
>
> diff --git a/dlls/comctl32/commctrl.c b/dlls/comctl32/commctrl.c
> index c21cae3..7e44e03 100644
> --- a/dlls/comctl32/commctrl.c
> +++ b/dlls/comctl32/commctrl.c
> @@ -693,12 +693,13 @@ CreateToolbarEx (HWND hwnd, DWORD style, UINT wID, INT nBitmaps,
>         SendMessageW (hwndTB, TB_SETBITMAPSIZE, 0,
>                       MAKELPARAM((WORD)dxBitmap, (WORD)dyBitmap));
>
> -       if (dxButton <= 0)
> -           dxButton = 24;
> -       if (dyButton <= 0)
> -           dyButton = 22;
> -       SendMessageW (hwndTB, TB_SETBUTTONSIZE, 0,
> -                     MAKELPARAM((WORD)dxButton, (WORD)dyButton));
> +       if (dxButton < 0)
> +           dxButton = dxBitmap;
> +       if (dyButton < 0)
> +           dyButton = dyBitmap;
> +       /* TB_SETBUTTONSIZE -> TB_SETBITMAPSIZE bug introduced for Windows compatibility */
> +       if (dxButton != 0 && dyButton != 0)
> +            SendMessageW(hwndTB, TB_SETBITMAPSIZE, 0, MAKELPARAM(dxButton, dyButton));
>
> /* add bitmaps */
> diff --git a/dlls/comctl32/tests/toolbar.c b/dlls/comctl32/tests/toolbar.c
> index 164644e..b583295 100644
> --- a/dlls/comctl32/tests/toolbar.c
> +++ b/dlls/comctl32/tests/toolbar.c
> @@ -869,6 +869,45 @@ static void test_sizes(void)
>      DestroyWindow(hToolbar);
>  }
>
> +
> +static void test_createtoolbarex()
> +{
> +    HWND hToolbar;
> +    TBBUTTON btns[3];
> +    ZeroMemory(&btns, sizeof(btns));
> +
> +    hToolbar = CreateToolbarEx(hMainWnd, WS_VISIBLE, 1, 16, GetModuleHandle(NULL), IDB_BITMAP_128x15, btns,
> +        3, 20, 20, 16, 16, sizeof(TBBUTTON));
> +    CHECK_IMAGELIST(16, 20, 20);
> +    compare((int)SendMessage(hToolbar, TB_GETBUTTONSIZE, 0, 0), 0x1a001b, "%x");
> +    DestroyWindow(hToolbar);
> +
> +    hToolbar = CreateToolbarEx(hMainWnd, WS_VISIBLE, 1, 16, GetModuleHandle(NULL), IDB_BITMAP_128x15, btns,
> +        3, 4, 4, 16, 16, sizeof(TBBUTTON));
> +    CHECK_IMAGELIST(32, 4, 4);
> +    compare((int)SendMessage(hToolbar, TB_GETBUTTONSIZE, 0, 0), 0xa000b, "%x");
> +    DestroyWindow(hToolbar);
> +
> +    hToolbar = CreateToolbarEx(hMainWnd, WS_VISIBLE, 1, 16, GetModuleHandle(NULL), IDB_BITMAP_128x15, btns,
> +        3, 0, 8, 12, 12, sizeof(TBBUTTON));
> +    CHECK_IMAGELIST(16, 12, 12);
> +    compare((int)SendMessage(hToolbar, TB_GETBUTTONSIZE, 0, 0), 0x120013, "%x");
> +    DestroyWindow(hToolbar);
> +
> +    hToolbar = CreateToolbarEx(hMainWnd, WS_VISIBLE, 1, 16, GetModuleHandle(NULL), IDB_BITMAP_128x15, btns,
> +        3, -1, 8, 12, 12, sizeof(TBBUTTON));
> +    CHECK_IMAGELIST(16, 12, 8);
> +    compare((int)SendMessage(hToolbar, TB_GETBUTTONSIZE, 0, 0), 0xe0013, "%x");
> +    DestroyWindow(hToolbar);
> +
> +    hToolbar = CreateToolbarEx(hMainWnd, WS_VISIBLE, 1, 16, GetModuleHandle(NULL), IDB_BITMAP_128x15, btns,
> +        3, -1, 8, -1, 12, sizeof(TBBUTTON));
> +    CHECK_IMAGELIST(16, 16, 8);
> +    compare((int)SendMessage(hToolbar, TB_GETBUTTONSIZE, 0, 0), 0xe0017, "%x");
> +    DestroyWindow(hToolbar);
> +}
> +
> +
>  START_TEST(toolbar)
>  {
>      WNDCLASSA wc;
> @@ -899,6 +938,7 @@ START_TEST(toolbar)
>      test_add_string();
>      test_hotitem();
>      test_sizes();
> +    test_createtoolbarex();
>
> PostQuitMessage(0);
>      while(GetMessageA(&msg,0,0,0)) {
> --
> 1.4.4.2

If you're testing what message is sent, why not use the message
sequence testing code?

-- 
James Hawkins

comctl32: CreateToolbarEx should send TB_SETBITMAPSIZE twice (fixes bug #6392)

[Mikołaj Zalewski]

> If you're testing what message is sent, why not use the message
> sequence testing code?

That's true that a message sequence test would be better. I haven't 
thought about it.

Mikolaj Zalewski