[PATCH v3 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 -- v3: comctl32: Make CBS_NOINTEGRALHEIGHT only set minimum combobox height. comctl32/tests: Add tests for a small number of items but big size to the combobox dropdown size tests. https://gitlab.winehq.org/wine/wine/-/merge_requests/6898
From: Orin Varley <ovarley(a)codeweavers.com> To catch bugs like the one below. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57360 --- dlls/comctl32/tests/combo.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dlls/comctl32/tests/combo.c b/dlls/comctl32/tests/combo.c index 1040b863b13..ca65f35b1c9 100644 --- a/dlls/comctl32/tests/combo.c +++ b/dlls/comctl32/tests/combo.c @@ -1277,6 +1277,8 @@ static void test_combo_dropdown_size(DWORD style) {33, 50, -1}, {35, 100, 40}, {15, 50, 3}, + {1, 650, 40}, + {7, 650, 3}, }; for (test = 0; test < ARRAY_SIZE(info_height); test++) @@ -1350,6 +1352,7 @@ static void test_combo_dropdown_size(DWORD style) if (expected_height_list < 0) expected_height_list = 0; + todo_wine_if(height_item * info_test->num_items < list_height_calculated) ok(expected_height_list == height_list, "expected list height to be %d, got %d\n", expected_height_list, height_list); } -- GitLab 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 +- dlls/comctl32/tests/combo.c | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) 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 { diff --git a/dlls/comctl32/tests/combo.c b/dlls/comctl32/tests/combo.c index ca65f35b1c9..0ba720a03ce 100644 --- a/dlls/comctl32/tests/combo.c +++ b/dlls/comctl32/tests/combo.c @@ -1352,7 +1352,6 @@ static void test_combo_dropdown_size(DWORD style) if (expected_height_list < 0) expected_height_list = 0; - todo_wine_if(height_item * info_test->num_items < list_height_calculated) ok(expected_height_list == height_list, "expected list height to be %d, got %d\n", expected_height_list, height_list); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6898
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=149970 Your paranoid android. === debian11b (64 bit WoW report) === user32: win.c:4070: Test failed: Expected active window 00000000026B0174, got 0000000000000000. win.c:4071: Test failed: Expected focus window 00000000026B0174, got 0000000000000000.
On Tue Nov 26 10:34:06 2024 +0000, Orin Varley wrote:
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. Yep, so I've added a test for this to make sure but Windows seems to ignore the visible items when CBS_NOINTEGRALHEIGHT style is on for some reason.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6898#note_88928
On Tue Nov 26 12:07:38 2024 +0000, Orin Varley wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/6898/diffs?diff_id=145486&start_sha=a30b7f50318310d80329789457998a96b46f74cd#d363145366f39f7c551133bb42fbbea9c2ebcac0_1280_1280) Let's just update this MR.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6898#note_88931
On Tue Nov 26 13:28:31 2024 +0000, Zhiyi Zhang wrote:
Let's just update this MR. Sorry yes, I did do that in the end so it should be updated - let me know if it's okay.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6898#note_88932
This merge request was approved by Zhiyi Zhang. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6898
participants (4)
-
Marvin -
Orin Varley -
Orin Varley (＠ovarley1)
-
Zhiyi Zhang (＠zhiyi)