[PATCH v3 0/4] MR9427: combobox: Adjust MEASUREITEMSTRUCT.itemHeight by 2 instead of 6.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58932 -- v3: user32/combo: Adjust MEASUREITEMSTRUCT.itemHeight by 2 instead of 6. comctl32/tests: Add tests for WM_MEASUREITEM with combobox v5. https://gitlab.winehq.org/wine/wine/-/merge_requests/9427
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/comctl32/tests/combo.c | 68 +++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/dlls/comctl32/tests/combo.c b/dlls/comctl32/tests/combo.c index 5af530ad5be..c6812d5b10d 100644 --- a/dlls/comctl32/tests/combo.c +++ b/dlls/comctl32/tests/combo.c @@ -1689,6 +1689,70 @@ static void test_combo_keypresses(void) DestroyWindow(combo); } +static int test_wm_measureitem_count; + +static LRESULT CALLBACK test_measure_item_wnd_proc(HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam) +{ + if (msg == WM_MEASUREITEM) + { + MEASUREITEMSTRUCT *m = (MEASUREITEMSTRUCT *)lparam; + unsigned int expected; + + ++test_wm_measureitem_count; + ok(m->CtlType == ODT_COMBOBOX, "got %#x.\n", m->CtlType); + ok(m->CtlID == COMBO_ID, "got %u.\n", m->CtlID); + if (m->itemID == -1) + { + expected = get_font_height(GetStockObject(SYSTEM_FONT)) + 2; + todo_wine ok(m->itemHeight == expected, "got %u, expected %u.\n", m->itemHeight, expected); + m->itemHeight = expected + 4; + } + return TRUE; + } + return DefWindowProcW(hwnd, msg, wparam, lparam); +} + +static void test_combo_measureitem(DWORD style) +{ + unsigned int expected; + HWND parent, combo; + WNDCLASSW wc; + RECT r; + + wc.style = CS_HREDRAW | CS_VREDRAW; + wc.cbClsExtra = 0; + wc.cbWndExtra = 0; + wc.hInstance = GetModuleHandleA(NULL); + wc.hIcon = NULL; + wc.hCursor = LoadCursorA(NULL, (LPCSTR)IDC_ARROW); + wc.hbrBackground = GetSysColorBrush(COLOR_WINDOW); + wc.lpszMenuName = NULL; + wc.lpszClassName = L"test_measure_item"; + wc.lpfnWndProc = test_measure_item_wnd_proc; + RegisterClassW(&wc); + + winetest_push_context("style %#lx", style); + parent = CreateWindowA("test_measure_item", "Test measure", WS_OVERLAPPEDWINDOW | WS_VISIBLE, 10, 10, 300, 300, NULL, NULL, NULL, 0); + test_wm_measureitem_count = 0; + combo = CreateWindowA(WC_COMBOBOXA, "Combo", WS_VISIBLE | WS_CHILD | style, 5, 5, 100, 100, parent, (HMENU)COMBO_ID, NULL, 0); + if (style & CBS_OWNERDRAWFIXED) + ok(test_wm_measureitem_count == 2, "got %d.\n", test_wm_measureitem_count); + else if (style & CBS_OWNERDRAWVARIABLE) + ok(test_wm_measureitem_count == 1, "got %d.\n", test_wm_measureitem_count); + else + ok(!test_wm_measureitem_count, "got %d.\n", test_wm_measureitem_count); + GetClientRect(combo, &r); + expected = get_font_height(GetStockObject(SYSTEM_FONT)) + 8; + if (style & (CBS_OWNERDRAWFIXED | CBS_OWNERDRAWVARIABLE)) + expected += 4; + todo_wine_if(style & (CBS_OWNERDRAWFIXED | CBS_OWNERDRAWVARIABLE)) expect_rect(r, 0, 0, 100, expected); + + DestroyWindow(combo); + DestroyWindow(parent); + UnregisterClassW(L"test_measure_item", GetModuleHandleA(NULL)); + winetest_pop_context(); +} + START_TEST(combo) { ULONG_PTR ctx_cookie; @@ -1742,6 +1806,10 @@ START_TEST(combo) test_combo_dropdown_size(CBS_NOINTEGRALHEIGHT); test_combo_ctlcolor(); test_combo_keypresses(); + test_combo_measureitem(CBS_DROPDOWNLIST | CBS_OWNERDRAWFIXED); + test_combo_measureitem(CBS_DROPDOWNLIST | CBS_OWNERDRAWVARIABLE); + test_combo_measureitem(CBS_DROPDOWNLIST); + test_combo_measureitem(CBS_DROPDOWN | CBS_OWNERDRAWFIXED); cleanup(); unload_v6_module(ctx_cookie, hCtx); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9427
From: Paul Gofman <pgofman(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58932 --- dlls/comctl32/tests/combo.c | 4 ++-- dlls/comctl32_v6/combo.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/comctl32/tests/combo.c b/dlls/comctl32/tests/combo.c index c6812d5b10d..7662f9fdd95 100644 --- a/dlls/comctl32/tests/combo.c +++ b/dlls/comctl32/tests/combo.c @@ -1704,7 +1704,7 @@ static LRESULT CALLBACK test_measure_item_wnd_proc(HWND hwnd, UINT msg, WPARAM w if (m->itemID == -1) { expected = get_font_height(GetStockObject(SYSTEM_FONT)) + 2; - todo_wine ok(m->itemHeight == expected, "got %u, expected %u.\n", m->itemHeight, expected); + ok(m->itemHeight == expected, "got %u, expected %u.\n", m->itemHeight, expected); m->itemHeight = expected + 4; } return TRUE; @@ -1745,7 +1745,7 @@ static void test_combo_measureitem(DWORD style) expected = get_font_height(GetStockObject(SYSTEM_FONT)) + 8; if (style & (CBS_OWNERDRAWFIXED | CBS_OWNERDRAWVARIABLE)) expected += 4; - todo_wine_if(style & (CBS_OWNERDRAWFIXED | CBS_OWNERDRAWVARIABLE)) expect_rect(r, 0, 0, 100, expected); + expect_rect(r, 0, 0, 100, expected); DestroyWindow(combo); DestroyWindow(parent); diff --git a/dlls/comctl32_v6/combo.c b/dlls/comctl32_v6/combo.c index 453f1de7dd9..a8ecf9f74db 100644 --- a/dlls/comctl32_v6/combo.c +++ b/dlls/comctl32_v6/combo.c @@ -239,10 +239,10 @@ static INT CBGetTextAreaHeight(HEADCOMBO *lphc, BOOL clip_item_height) measureItem.CtlID = id; measureItem.itemID = -1; measureItem.itemWidth = clientRect.right; - measureItem.itemHeight = item_height - 6; /* ownerdrawn cb is taller */ + measureItem.itemHeight = item_height - 2; /* ownerdrawn cb is taller */ measureItem.itemData = 0; SendMessageW(lphc->owner, WM_MEASUREITEM, id, (LPARAM)&measureItem); - item_height = 6 + measureItem.itemHeight; + item_height = 2 + measureItem.itemHeight; /* * Send a second one in the case of a fixed ownerdraw list to calculate the -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9427
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/comctl32/tests/combo.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dlls/comctl32/tests/combo.c b/dlls/comctl32/tests/combo.c index 7662f9fdd95..8618a4d8f4f 100644 --- a/dlls/comctl32/tests/combo.c +++ b/dlls/comctl32/tests/combo.c @@ -1773,6 +1773,10 @@ START_TEST(combo) test_comboex_subclass(); test_comboex_get_set_item(); test_comboex_CBEN_GETDISPINFO(); + test_combo_measureitem(CBS_DROPDOWNLIST | CBS_OWNERDRAWFIXED); + test_combo_measureitem(CBS_DROPDOWNLIST | CBS_OWNERDRAWVARIABLE); + test_combo_measureitem(CBS_DROPDOWNLIST); + test_combo_measureitem(CBS_DROPDOWN | CBS_OWNERDRAWFIXED); if (!load_v6_module(&ctx_cookie, &hCtx)) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9427
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/user32/combo.c | 4 +-- dlls/user32/tests/combo.c | 68 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/dlls/user32/combo.c b/dlls/user32/combo.c index 5f2ddb3dcdf..0d32c9a3f2d 100644 --- a/dlls/user32/combo.c +++ b/dlls/user32/combo.c @@ -230,10 +230,10 @@ static INT CBGetTextAreaHeight(HEADCOMBO *lphc, BOOL clip_item_height) measureItem.CtlID = id; measureItem.itemID = -1; measureItem.itemWidth = clientRect.right; - measureItem.itemHeight = item_height - 6; /* ownerdrawn cb is taller */ + measureItem.itemHeight = item_height - 2; /* ownerdrawn cb is taller */ measureItem.itemData = 0; SendMessageW(lphc->owner, WM_MEASUREITEM, id, (LPARAM)&measureItem); - item_height = 6 + measureItem.itemHeight; + item_height = 2 + measureItem.itemHeight; /* * Send a second one in the case of a fixed ownerdraw list to calculate the diff --git a/dlls/user32/tests/combo.c b/dlls/user32/tests/combo.c index 77dec2382bd..8d93369d79a 100644 --- a/dlls/user32/tests/combo.c +++ b/dlls/user32/tests/combo.c @@ -962,6 +962,70 @@ static void test_combo_setfont(void) DeleteObject(hf); } +static int test_wm_measureitem_count; + +static LRESULT CALLBACK test_measure_item_wnd_proc(HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam) +{ + if (msg == WM_MEASUREITEM) + { + MEASUREITEMSTRUCT *m = (MEASUREITEMSTRUCT *)lparam; + unsigned int expected; + + ++test_wm_measureitem_count; + ok(m->CtlType == ODT_COMBOBOX, "got %#x.\n", m->CtlType); + ok(m->CtlID == COMBO_ID, "got %u.\n", m->CtlID); + if (m->itemID == -1) + { + expected = get_font_height(GetStockObject(SYSTEM_FONT)) + 2; + ok(m->itemHeight == expected, "got %u, expected %u.\n", m->itemHeight, expected); + m->itemHeight = expected + 4; + } + return TRUE; + } + return DefWindowProcW(hwnd, msg, wparam, lparam); +} + +static void test_combo_measureitem(DWORD style) +{ + unsigned int expected; + HWND parent, combo; + WNDCLASSW wc; + RECT r; + + wc.style = CS_HREDRAW | CS_VREDRAW; + wc.cbClsExtra = 0; + wc.cbWndExtra = 0; + wc.hInstance = GetModuleHandleA(NULL); + wc.hIcon = NULL; + wc.hCursor = LoadCursorA(NULL, (LPCSTR)IDC_ARROW); + wc.hbrBackground = GetSysColorBrush(COLOR_WINDOW); + wc.lpszMenuName = NULL; + wc.lpszClassName = L"test_measure_item"; + wc.lpfnWndProc = test_measure_item_wnd_proc; + RegisterClassW(&wc); + + winetest_push_context("style %#lx", style); + parent = CreateWindowA("test_measure_item", "Test measure", WS_OVERLAPPEDWINDOW | WS_VISIBLE, 10, 10, 300, 300, NULL, NULL, NULL, 0); + test_wm_measureitem_count = 0; + combo = CreateWindowA("ComboBox", "Combo", WS_VISIBLE | WS_CHILD | style, 5, 5, 100, 100, parent, (HMENU)COMBO_ID, NULL, 0); + if (style & CBS_OWNERDRAWFIXED) + ok(test_wm_measureitem_count == 2, "got %d.\n", test_wm_measureitem_count); + else if (style & CBS_OWNERDRAWVARIABLE) + ok(test_wm_measureitem_count == 1, "got %d.\n", test_wm_measureitem_count); + else + ok(!test_wm_measureitem_count, "got %d.\n", test_wm_measureitem_count); + GetClientRect(combo, &r); + expected = get_font_height(GetStockObject(SYSTEM_FONT)) + 8; + if (style & (CBS_OWNERDRAWFIXED | CBS_OWNERDRAWVARIABLE)) + expected += 4; + expect_rect(r, 0, 0, 100, expected); + + DestroyWindow(combo); + DestroyWindow(parent); + UnregisterClassW(L"test_measure_item", GetModuleHandleA(NULL)); + winetest_pop_context(); +} + START_TEST(combo) { brush_red = CreateSolidBrush(RGB(255, 0, 0)); @@ -986,6 +1050,10 @@ START_TEST(combo) test_listbox_size(CBS_DROPDOWN); test_combo_ctlcolor(); test_combo_setfont(); + test_combo_measureitem(CBS_DROPDOWNLIST | CBS_OWNERDRAWFIXED); + test_combo_measureitem(CBS_DROPDOWNLIST | CBS_OWNERDRAWVARIABLE); + test_combo_measureitem(CBS_DROPDOWNLIST); + test_combo_measureitem(CBS_DROPDOWN | CBS_OWNERDRAWFIXED); DestroyWindow(hMainWnd); DeleteObject(brush_red); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9427
v3: - add test with combobox v5. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9427#note_122554
On Tue Nov 18 02:48:24 2025 +0000, Paul Gofman wrote:
v3: - add test with combobox v5. Let's merge the v5 tests with the first patch.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9427#note_122684
On Tue Nov 18 02:48:24 2025 +0000, Zhiyi Zhang wrote:
Let's merge the v5 tests with the first patch. I tried to this at once, but currently it is inconvenient. combo.c does not exist in comctl32 v5 at least in Wine, so v5 test actually works with user32 control. So those todo_wine and removing those become version dependent. I am not sure offhand if that user32 fallback for combobox is correct or not but it is unrelated here. Maybe we could leave that test as is and just add after?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9427#note_122687
On Tue Nov 18 03:10:10 2025 +0000, Paul Gofman wrote:
I tried to this at once, but currently it is inconvenient. combo.c does not exist in comctl32 v5 at least in Wine, so v5 test actually works with user32 control. So those todo_wine and removing those become version dependent. I am not sure offhand if that user32 fallback for combobox is correct or not but it is unrelated here. Maybe we could leave that test as is and just add after? Oh, I see. Yes, there is no combo class in comctl32 v5. So we don't need to add tests for it. Let's leave it to the user32 combo tests, since they already cover it.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9427#note_122689
participants (3)
-
Paul Gofman -
Paul Gofman (＠gofman)
-
Zhiyi Zhang (＠zhiyi)