[PATCH v7 0/4] MR10261: Fix toolbar wrapping issues
This MR resolves two bugs with toolbar wrapping I have discovered. 1. Toolbar missing four buttons when app window is maximized (wrap should not appear at all in this case). 2. Last toolbar button disappears when window size (toolbar size) is slightly narrower then button right border. Reasons of this behavior: 1. Toolbar buttons have BTNS_AUTOSIZE style, toolbar width is set using CCS_NORESIZE and equals to widths of all autosized buttons combined. cx (button width) value in TOOLBAR_WrapToolbar is set from nButtonWidth, which contains widest button width. Since buttons in toolbar have different widths, TOOLBAR_WrapToolbar miscalculates total width, because actual buttons widths are less then nButtonWidth. This results in TOOLBAR_WrapToolbar setting TBSTATE_WRAP when it is not not needed, and since app does not paint second button row, wrapped buttons just disappear. 2. After I fixed first issue, I noticed, that last button disappears, when toolbar become narrower then its right border. Then, when toolbar right border approximately in the middle of disappeared button, wrap happens. In windows system, wrapping occurs immediately when toolbar becomes narrower. Found solutions: 1. I noticed, that TOOLBAR_LayoutToolbar calculates buttons rects differently, thats because it checks if button have BTNS_AUTOSIZE style and then calculate size. So i moved this BTNS_AUTOSIZE size calculation in separate function for it to then be used in TOOLBAR_LayoutToolbar and TOOLBAR_WrapToolbar to guarantee that cx values for specific button will be equal in both functions. 2. TOOLBAR_WrapToolbar function decides to wrap in case `if ((x + cx - (infoPtr->nButtonWidth - infoPtr->nBitmapWidth) / 2 > width).` This `(infoPtr->nButtonWidth - infoPtr->nBitmapWidth) / 2` turns out, makes this gap, where button right border (x + cx) exceed toolbar right border (width), but wrap wont appear. I did not found any reasons to make this half-button gap, so I just removed it and it solved this issue. The comment above says /\* The layout makes sure the bitmap is visible, but not the button. \*/, but it was introduced in 1999, when TOOLBAR_LayoutToolbar did not existed, and I think its irrelevant now, since layout calculates cx taking both bitmap and string into account. I wrote tests for both fixes: 1. Creates CCS_NORESIZE toolbar, then add BTNS_AUTOSIZE buttons of different widths one by one and checks if wrap happened. In normal case (in windows) wrap should only happen on fourth button, but without fix wrap happens on third. 2. Creates CCS_NORESIZE toolbar with 4 BTNS_AUTOSIZE buttons. Toolbar width at the creation is calculated beforehand and equals exactly buttons widths combined. Then test reduces toolbar width by 1 and checks if wrap happened, in normal case wrap happens, without the fix its not. -- v7: comctl32/toolbar: Remove redundant toolbar wrapping logic. comctl32/toolbar: Use TOOLBAR_AutoSizeButtonWidth in WrapToolbar. comctl32/toolbar: Add TOOLBAR_AutoSizeButtonWidth from LayoutToolbar as a separate function. https://gitlab.winehq.org/wine/wine/-/merge_requests/10261
From: Ivan Ivlev <iviv@etersoft.ru> Signed-off-by: Ivan Ivlev <iviv@etersoft.ru> --- dlls/comctl32/tests/toolbar.c | 52 +++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/dlls/comctl32/tests/toolbar.c b/dlls/comctl32/tests/toolbar.c index eed70df2a64..b25a030bbb2 100644 --- a/dlls/comctl32/tests/toolbar.c +++ b/dlls/comctl32/tests/toolbar.c @@ -3102,6 +3102,56 @@ static void test_WM_PAINT(BOOL v6) UnregisterClassW(wc.lpszClassName, 0); } +static void test_wrap(void) +{ + TBBUTTON buttons[] = + { + { I_IMAGENONE, 0, TBSTATE_ENABLED, BTNS_BUTTON | BTNS_AUTOSIZE, { 0 }, 0, (INT_PTR)".." }, + { I_IMAGENONE, 1, TBSTATE_ENABLED, BTNS_BUTTON | BTNS_AUTOSIZE, { 0 }, 0, (INT_PTR)"................" }, + }; + HWND hToolbar; + int result; + int toolbar_width = 0; + int i; + + hToolbar = CreateWindowExA(0, TOOLBARCLASSNAMEA, NULL, WS_CHILD | CCS_NORESIZE, + 0, 0, 500, 30, hMainWnd, NULL, GetModuleHandleA(NULL), NULL); + SendMessageA(hToolbar, TB_BUTTONSTRUCTSIZE, sizeof(TBBUTTON), 0); + SendMessageA(hToolbar, TB_ADDBUTTONSA, 2, (LPARAM)buttons); + for (i = 0; i < 2; i++) + { + RECT rect; + SendMessageA(hToolbar, TB_GETITEMRECT, i, (LPARAM)&rect); + toolbar_width += rect.right - rect.left; + } + DestroyWindow(hToolbar); + + hToolbar = CreateWindowExA(0, TOOLBARCLASSNAMEA, NULL, WS_CHILD | CCS_NORESIZE | TBSTYLE_WRAPABLE, + 0, 0, toolbar_width, 30, hMainWnd, NULL, GetModuleHandleA(NULL), NULL); + SendMessageA(hToolbar, TB_BUTTONSTRUCTSIZE, sizeof(TBBUTTON), 0); + + SendMessageA(hToolbar, TB_ADDBUTTONSA, 2, (LPARAM)buttons); + SendMessageA(hToolbar, TB_AUTOSIZE, 0, 0); + result = SendMessageA(hToolbar, TB_GETROWS, 0, 0); + todo_wine ok(result == 1, "Got unexpected nRows: %d.\n", result); + + DestroyWindow(hToolbar); + + hToolbar = CreateWindowExA(0, TOOLBARCLASSNAMEA, NULL, WS_CHILD | CCS_NORESIZE | TBSTYLE_WRAPABLE, + 0, 0, toolbar_width, 30, hMainWnd, NULL, GetModuleHandleA(NULL), NULL); + SendMessageA(hToolbar, TB_BUTTONSTRUCTSIZE, sizeof(TBBUTTON), 0); + + SendMessageA(hToolbar, TB_ADDBUTTONSA, 2, (LPARAM)buttons); + SendMessageA(hToolbar, TB_AUTOSIZE, 0, 0); + + SetWindowPos(hToolbar, NULL, 0, 0, toolbar_width - 1, 30, SWP_NOMOVE | SWP_NOZORDER); + SendMessageA(hToolbar, TB_AUTOSIZE, 0, 0); + result = SendMessageA(hToolbar, TB_GETROWS, 0, 0); + ok(result == 2, "Got unexpected nRows: %d.\n", result); + + DestroyWindow(hToolbar); +} + START_TEST(toolbar) { ULONG_PTR ctx_cookie; @@ -3157,6 +3207,7 @@ START_TEST(toolbar) test_unicode_format(); test_WM_ERASEBKGND(FALSE); test_WM_PAINT(FALSE); + test_wrap(); if (!load_v6_module(&ctx_cookie, &ctx)) return; @@ -3167,6 +3218,7 @@ START_TEST(toolbar) test_unicode_format(); test_WM_ERASEBKGND(TRUE); test_WM_PAINT(TRUE); + test_wrap(); PostQuitMessage(0); while(GetMessageA(&msg,0,0,0)) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10261
From: Ivan Ivlev <iviv@etersoft.ru> Signed-off-by: Ivan Ivlev <iviv@etersoft.ru> --- dlls/comctl32/toolbar.c | 43 ++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/dlls/comctl32/toolbar.c b/dlls/comctl32/toolbar.c index 3826b18d4c6..21e60dd4d09 100644 --- a/dlls/comctl32/toolbar.c +++ b/dlls/comctl32/toolbar.c @@ -248,6 +248,7 @@ static void TOOLBAR_TooltipAddTool(const TOOLBAR_INFO *infoPtr, const TBUTTON_IN static void TOOLBAR_TooltipSetRect(const TOOLBAR_INFO *infoPtr, const TBUTTON_INFO *button); static LRESULT TOOLBAR_SetButtonInfo(TOOLBAR_INFO *infoPtr, INT Id, const TBBUTTONINFOW *lptbbi, BOOL isW); +static int TOOLBAR_AutoSizeButtonWidth(const TOOLBAR_INFO *infoPtr, const TBUTTON_INFO *btnPtr); static inline int default_top_margin(const TOOLBAR_INFO *infoPtr) @@ -1687,6 +1688,27 @@ static inline SIZE TOOLBAR_MeasureButton(const TOOLBAR_INFO *infoPtr, SIZE sizeS return sizeButton; } +static int +TOOLBAR_AutoSizeButtonWidth(const TOOLBAR_INFO *infoPtr, const TBUTTON_INFO *btnPtr) +{ + SIZE sz, sizeButton; + HDC hdc; + HFONT hOldFont; + BOOL validImageList = TOOLBAR_IsValidImageList(infoPtr, 0); + + hdc = GetDC (infoPtr->hwndSelf); + hOldFont = SelectObject (hdc, infoPtr->hFont); + + TOOLBAR_MeasureString(infoPtr, btnPtr, hdc, &sz); + + SelectObject (hdc, hOldFont); + ReleaseDC (infoPtr->hwndSelf, hdc); + + sizeButton = TOOLBAR_MeasureButton(infoPtr, sz, + TOOLBAR_IsValidBitmapIndex(infoPtr, btnPtr->iBitmap), + validImageList); + return sizeButton.cx; +} /*********************************************************************** * TOOLBAR_CalcToolbar @@ -1727,11 +1749,9 @@ static void TOOLBAR_LayoutToolbar(TOOLBAR_INFO *infoPtr) { TBUTTON_INFO *btnPtr; - SIZE sizeButton; INT i, nRows, nSepRows; INT x, y, cx, cy; BOOL bWrap; - BOOL validImageList = TOOLBAR_IsValidImageList(infoPtr, 0); TOOLBAR_WrapToolbar(infoPtr); @@ -1777,24 +1797,7 @@ TOOLBAR_LayoutToolbar(TOOLBAR_INFO *infoPtr) if (btnPtr->cx) cx = btnPtr->cx; else if (btnPtr->fsStyle & BTNS_AUTOSIZE) - { - SIZE sz; - HDC hdc; - HFONT hOldFont; - - hdc = GetDC (infoPtr->hwndSelf); - hOldFont = SelectObject (hdc, infoPtr->hFont); - - TOOLBAR_MeasureString(infoPtr, btnPtr, hdc, &sz); - - SelectObject (hdc, hOldFont); - ReleaseDC (infoPtr->hwndSelf, hdc); - - sizeButton = TOOLBAR_MeasureButton(infoPtr, sz, - TOOLBAR_IsValidBitmapIndex(infoPtr, infoPtr->buttons[i].iBitmap), - validImageList); - cx = sizeButton.cx; - } + cx = TOOLBAR_AutoSizeButtonWidth(infoPtr, btnPtr); else cx = infoPtr->nButtonWidth; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10261
From: Ivan Ivlev <iviv@etersoft.ru> Signed-off-by: Ivan Ivlev <iviv@etersoft.ru> --- dlls/comctl32/tests/toolbar.c | 4 ++-- dlls/comctl32/toolbar.c | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/dlls/comctl32/tests/toolbar.c b/dlls/comctl32/tests/toolbar.c index b25a030bbb2..ece556a338d 100644 --- a/dlls/comctl32/tests/toolbar.c +++ b/dlls/comctl32/tests/toolbar.c @@ -3133,7 +3133,7 @@ static void test_wrap(void) SendMessageA(hToolbar, TB_ADDBUTTONSA, 2, (LPARAM)buttons); SendMessageA(hToolbar, TB_AUTOSIZE, 0, 0); result = SendMessageA(hToolbar, TB_GETROWS, 0, 0); - todo_wine ok(result == 1, "Got unexpected nRows: %d.\n", result); + ok(result == 1, "Got unexpected nRows: %d.\n", result); DestroyWindow(hToolbar); @@ -3147,7 +3147,7 @@ static void test_wrap(void) SetWindowPos(hToolbar, NULL, 0, 0, toolbar_width - 1, 30, SWP_NOMOVE | SWP_NOZORDER); SendMessageA(hToolbar, TB_AUTOSIZE, 0, 0); result = SendMessageA(hToolbar, TB_GETROWS, 0, 0); - ok(result == 2, "Got unexpected nRows: %d.\n", result); + todo_wine ok(result == 2, "Got unexpected nRows: %d.\n", result); DestroyWindow(hToolbar); } diff --git a/dlls/comctl32/toolbar.c b/dlls/comctl32/toolbar.c index 21e60dd4d09..4993a5c16cc 100644 --- a/dlls/comctl32/toolbar.c +++ b/dlls/comctl32/toolbar.c @@ -1415,6 +1415,8 @@ TOOLBAR_WrapToolbar(TOOLBAR_INFO *infoPtr) else if ((btnPtr[i].fsStyle & BTNS_SEP) && !(infoPtr->dwStyle & CCS_VERT)) cx = (btnPtr[i].iBitmap > 0) ? btnPtr[i].iBitmap : SEPARATOR_WIDTH; + else if (btnPtr[i].fsStyle & BTNS_AUTOSIZE) + cx = TOOLBAR_AutoSizeButtonWidth(infoPtr, btnPtr + i); else cx = infoPtr->nButtonWidth; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10261
From: Ivan Ivlev <iviv@etersoft.ru> Signed-off-by: Ivan Ivlev <iviv@etersoft.ru> --- dlls/comctl32/tests/toolbar.c | 2 +- dlls/comctl32/toolbar.c | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/dlls/comctl32/tests/toolbar.c b/dlls/comctl32/tests/toolbar.c index ece556a338d..0ff0ebced76 100644 --- a/dlls/comctl32/tests/toolbar.c +++ b/dlls/comctl32/tests/toolbar.c @@ -3147,7 +3147,7 @@ static void test_wrap(void) SetWindowPos(hToolbar, NULL, 0, 0, toolbar_width - 1, 30, SWP_NOMOVE | SWP_NOZORDER); SendMessageA(hToolbar, TB_AUTOSIZE, 0, 0); result = SendMessageA(hToolbar, TB_GETROWS, 0, 0); - todo_wine ok(result == 2, "Got unexpected nRows: %d.\n", result); + ok(result == 2, "Got unexpected nRows: %d.\n", result); DestroyWindow(hToolbar); } diff --git a/dlls/comctl32/toolbar.c b/dlls/comctl32/toolbar.c index 4993a5c16cc..52c0a18ed8a 100644 --- a/dlls/comctl32/toolbar.c +++ b/dlls/comctl32/toolbar.c @@ -1439,11 +1439,7 @@ TOOLBAR_WrapToolbar(TOOLBAR_INFO *infoPtr) continue; } - /* The layout makes sure the bitmap is visible, but not the button. */ - /* Test added to also wrap after a button that starts a row but */ - /* is bigger than the area. - GA 8/01 */ - if ((x + cx - (infoPtr->nButtonWidth - infoPtr->nBitmapWidth) / 2 > width) || - ((x == infoPtr->nIndent) && (cx > width))) + if (x + cx > width) { BOOL bFound = FALSE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10261
I noticed that pipeline failed because of: `error: 'TOOLBAR_AutoSizeButtonWidth' defined but not used [-Werror=unused-function]` So I edited commit that adds TOOLBAR_AutoSizeButtonWidth() - now this function is used in LayoutToolbar immediately. And the next commit just uses this function in WrapToolbar. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10261#note_146759
On Thu Jul 23 14:50:19 2026 +0000, Ivan Ivlev wrote:
changed this line in [version 6 of the diff](/wine/wine/-/merge_requests/10261/diffs?diff_id=283927&start_sha=a229f6334305b3ae06b15dba5f1c28f0562b9c93#80bec5fb0f9a667beeb7953ce870eb92dd1cfbdc_1693_1690) Did it. At first, I just made separate commit that adds this function, but now I understood that it has to be used in LayoutToolbar, so "function unused" warning does not occur.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10261#note_146760
On Tue Aug 4 07:54:31 2026 +0000, Ivan Ivlev wrote:
I noticed that pipeline failed because of: `error: 'TOOLBAR_AutoSizeButtonWidth' defined but not used [-Werror=unused-function]` So I edited commit that adds TOOLBAR_AutoSizeButtonWidth() - now this function is used in LayoutToolbar immediately. And the next commit just uses this function in WrapToolbar. I don't think you've updated the MR. The last CI run failed.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10261#note_147798
participants (3)
-
Ivan Ivlev -
Ivan Ivlev (＠iviv)
-
Zhiyi Zhang (＠zhiyi)