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

[James Hawkins]

On 3/19/07, Mikołaj Zalewski  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?= 
> 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?

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