[PATCH 0/2] MR6898: comctl32: Update combobox dropdown size.
When the combobox height is to be set by the application (as the CBS_NOINTEGRALHEIGHT style is on), and when a large size is specified but only a small number of items in the list, the height of the combobox should be set by the number of items rather than the size specified to avoid empty lines. The first commit fixes this and the second is a test for this behavior. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57360 Closes #7 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6898
From: Orin Varley <ovarley(a)codeweavers.com> When the combobox height is to be set by the application (as the CBS_NOINTEGRALHEIGHT style is on), and when a large size is specified but only a small number of items in the list, the height of the combobox should be set by the number of items rather than the size specified to avoid empty lines. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57360 --- dlls/comctl32/combo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/comctl32/combo.c b/dlls/comctl32/combo.c index cb0e9745b2a..09579952f22 100644 --- a/dlls/comctl32/combo.c +++ b/dlls/comctl32/combo.c @@ -971,7 +971,7 @@ static void CBDropDown( LPHEADCOMBO lphc ) if (lphc->dwStyle & CBS_NOINTEGRALHEIGHT) { - nDroppedHeight -= 1; + nDroppedHeight = min(nItems * nIHeight + COMBO_YBORDERSIZE(), nDroppedHeight - 1); } else { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6898
From: Orin Varley <ovarley(a)codeweavers.com> To catch bugs like the one bellow. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57360 --- dlls/comctl32/tests/combo.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/comctl32/tests/combo.c b/dlls/comctl32/tests/combo.c index 1040b863b13..515348feac9 100644 --- a/dlls/comctl32/tests/combo.c +++ b/dlls/comctl32/tests/combo.c @@ -1277,6 +1277,7 @@ static void test_combo_dropdown_size(DWORD style) {33, 50, -1}, {35, 100, 40}, {15, 50, 3}, + {7, 650, 40}, }; for (test = 0; test < ARRAY_SIZE(info_height); test++) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6898
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/combo.c:
if (lphc->dwStyle & CBS_NOINTEGRALHEIGHT) { - nDroppedHeight -= 1; + nDroppedHeight = min(nItems * nIHeight + COMBO_YBORDERSIZE(), nDroppedHeight - 1);
Shouldn't you consider if the items are visible like in the `else` branch? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6898#note_88896
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/combo.c:
if (lphc->dwStyle & CBS_NOINTEGRALHEIGHT)
The commit subject message is a bit vague. What about "comctl32: Make CBS_NOINTEGRALHEIGHT only set minimum comboxbox height." -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6898#note_88897
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/combo.c:
{33, 50, -1}, {35, 100, 40},
Rename the commit subject message to "comctl32/tests: ... " -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6898#note_88898
Zhiyi Zhang (@zhiyi) commented about dlls/comctl32/tests/combo.c:
{33, 50, -1}, {35, 100, 40}, {15, 50, 3}, + {7, 650, 40},
So we're aiming for fewer items, so why not 1 but 7? And let's add the tests first, mark the test failures with a todo_wine and then remove it after the fix. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6898#note_88899
On Tue Nov 26 10:19:24 2024 +0000, Zhiyi Zhang wrote:
So we're aiming for fewer items, so why not 1 but 7? And let's add the tests first, mark the test failures with a todo_wine and then remove it after the fix. We can use 1, I was just using 7 as the application I was looking at used 7 but it doesn't matter really as long as it's small enough.
I'll add those changes in now then. Do you want me to do two separate MRs then? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6898#note_88900
On Tue Nov 26 10:19:24 2024 +0000, Zhiyi Zhang wrote:
The commit subject message is a bit vague. What about "comctl32: Make CBS_NOINTEGRALHEIGHT only set minimum comboxbox height." Yep, good call
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6898#note_88901
On Tue Nov 26 10:19:24 2024 +0000, Zhiyi Zhang wrote:
Shouldn't you consider if the items are visible like in the `else` branch? Good point. From what I saw on Windows, it didn't seem to do that but I will spend some time checking this again as it is a bit strange. I also should have put another test in where limit \< num_items but they're both still smaller than the height so I will add that in.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6898#note_88902
participants (3)
-
Orin Varley -
Orin Varley (＠ovarley1)
-
Zhiyi Zhang (＠zhiyi)