[PATCH v6 1/7] comctl32/listbox: Check for out of bounds index using an unsigned index
Needed for LBS_NODATA. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/comctl32/listbox.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index cb645b4..71f1c05 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -489,7 +489,7 @@ static INT LISTBOX_GetItemFromPoint( const LB_DESCR *descr, INT x, INT y ) * Paint an item. */ static void LISTBOX_PaintItem( LB_DESCR *descr, HDC hdc, const RECT *rect, - INT index, UINT action, BOOL ignoreFocus ) + UINT index, UINT action, BOOL ignoreFocus ) { BOOL selected = FALSE, focused; LB_ITEMDATA *item = NULL; @@ -508,7 +508,7 @@ static void LISTBOX_PaintItem( LB_DESCR *descr, HDC hdc, const RECT *rect, RECT r; HRGN hrgn; - if (!item) + if (index >= descr->nb_items) { if (action == ODA_FOCUS) DrawFocusRect( hdc, rect ); -- 2.19.1
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/comctl32/listbox.c | 63 +++++++++++++++-------------------------- 1 file changed, 23 insertions(+), 40 deletions(-) diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index 71f1c05..498c549 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -36,12 +36,13 @@ #include "wine/unicode.h" #include "wine/exception.h" #include "wine/debug.h" +#include "wine/heap.h" #include "comctl32.h" WINE_DEFAULT_DEBUG_CHANNEL(listbox); -/* Items array granularity */ +/* Items array granularity (must be power of 2) */ #define LB_ARRAY_GRANULARITY 16 /* Scrolling timeout in ms */ @@ -126,6 +127,25 @@ static TIMER_DIRECTION LISTBOX_Timer = LB_TIMER_NONE; static LRESULT LISTBOX_GetItemRect( const LB_DESCR *descr, INT index, RECT *rect ); +static BOOL resize_storage(LB_DESCR *descr, UINT items_size) +{ + LB_ITEMDATA *items; + + if (items_size > descr->items_size) + { + items_size = (items_size + LB_ARRAY_GRANULARITY - 1) & ~(LB_ARRAY_GRANULARITY - 1); + items = heap_realloc(descr->items, items_size * sizeof(LB_ITEMDATA)); + if (!items) + { + SEND_NOTIFICATION(descr, LBN_ERRSPACE); + return FALSE; + } + descr->items_size = items_size; + descr->items = items; + } + return TRUE; +} + static BOOL is_item_selected( const LB_DESCR *descr, UINT index ) { return descr->items[index].selected; @@ -684,27 +704,8 @@ static void LISTBOX_DrawFocusRect( LB_DESCR *descr, BOOL on ) */ static LRESULT LISTBOX_InitStorage( LB_DESCR *descr, INT nb_items ) { - LB_ITEMDATA *item; - - nb_items += LB_ARRAY_GRANULARITY - 1; - nb_items -= (nb_items % LB_ARRAY_GRANULARITY); - if (descr->items) { - nb_items += descr->items_size; - item = HeapReAlloc( GetProcessHeap(), 0, descr->items, - nb_items * sizeof(LB_ITEMDATA)); - } - else { - item = HeapAlloc( GetProcessHeap(), 0, - nb_items * sizeof(LB_ITEMDATA)); - } - - if (!item) - { - SEND_NOTIFICATION( descr, LBN_ERRSPACE ); + if (!resize_storage(descr, descr->nb_items + nb_items)) return LB_ERRSPACE; - } - descr->items_size = nb_items; - descr->items = item; return LB_OKAY; } @@ -1525,29 +1526,11 @@ static LRESULT LISTBOX_InsertItem( LB_DESCR *descr, INT index, LPWSTR str, ULONG_PTR data ) { LB_ITEMDATA *item; - INT max_items; INT oldfocus = descr->focus_item; if (index == -1) index = descr->nb_items; else if ((index < 0) || (index > descr->nb_items)) return LB_ERR; - if (descr->nb_items == descr->items_size) - { - /* We need to grow the array */ - max_items = descr->items_size + LB_ARRAY_GRANULARITY; - if (descr->items) - item = HeapReAlloc( GetProcessHeap(), 0, descr->items, - max_items * sizeof(LB_ITEMDATA) ); - else - item = HeapAlloc( GetProcessHeap(), 0, - max_items * sizeof(LB_ITEMDATA) ); - if (!item) - { - SEND_NOTIFICATION( descr, LBN_ERRSPACE ); - return LB_ERRSPACE; - } - descr->items_size = max_items; - descr->items = item; - } + if (!resize_storage(descr, descr->nb_items + 1)) return LB_ERR; /* Insert the item structure */ -- 2.19.1
I did some testing on Windows, and LB_INITSTORAGE is not supposed to work the way it's implemented right now, before or after your patch. Summary of what I found: - LB_INITSTORAGE should return current capacity, 'items_size' in our case, not LB_OKAY; - it's not additive, and calling it for 0 count does not increase returned value, policy is items_size = max(items_size, wParam + 10); - it never shrinks as far as I can tell from return value, items_size is reset to 0 only on LB_RESETCONTENT, same for LB_DELETESTRING; - array grows by 32 items, not 16; - there is no alignment when growing on insert, so LB_RESETCONTENT -> LB_INITSTORAGE(1) -> LB_ADDSTRING twice -> items_size is 33. Currently we have no tests for this message.
On Tue, Dec 4, 2018 at 12:08 PM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
I did some testing on Windows, and LB_INITSTORAGE is not supposed to work the way it's implemented right now, before or after your patch.
Summary of what I found:
- LB_INITSTORAGE should return current capacity, 'items_size' in our case, not LB_OKAY; - it's not additive, and calling it for 0 count does not increase returned value, policy is items_size = max(items_size, wParam + 10); - it never shrinks as far as I can tell from return value, items_size is reset to 0 only on LB_RESETCONTENT, same for LB_DELETESTRING; - array grows by 32 items, not 16; - there is no alignment when growing on insert, so LB_RESETCONTENT -> LB_INITSTORAGE(1) -> LB_ADDSTRING twice -> items_size is 33.
Currently we have no tests for this message.
Other than fixing the return value, I don't think that it matters much how Windows does it internally or the granularity. It's an implementation detail, more or less. I mean, this has been broken for ages, and I'm not aware of any bug or broken app because of it. I think the policy you discovered sounds like a hack and probably there's something else (MSDN says it's also the number of items to add so it must be additive), I'll do more investigation on that. For now, can we just fix the return value and perhaps change the granularity to 32 without all the other stuff that's inconsequential so we can get at least single-selection LBS_NODATA into Wine 4.0? And if the whole granularity thing has to be changed (though I doubt it), it should just be fixed later, since it's totally separate issue anyway and more fragile. I mean this patch series won't break even current behavior (even if it's wrong), it will keep it the same so no regressions at all, and something always seems to pop up and may not make it to the code freeze at this rate,
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/comctl32/listbox.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index 498c549..1d881f1 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -131,7 +131,8 @@ static BOOL resize_storage(LB_DESCR *descr, UINT items_size) { LB_ITEMDATA *items; - if (items_size > descr->items_size) + if (items_size > descr->items_size || + items_size + LB_ARRAY_GRANULARITY * 2 < descr->items_size) { items_size = (items_size + LB_ARRAY_GRANULARITY - 1) & ~(LB_ARRAY_GRANULARITY - 1); items = heap_realloc(descr->items, items_size * sizeof(LB_ITEMDATA)); @@ -1666,7 +1667,6 @@ static void LISTBOX_DeleteItem( LB_DESCR *descr, INT index ) static LRESULT LISTBOX_RemoveItem( LB_DESCR *descr, INT index ) { LB_ITEMDATA *item; - INT max_items; if ((index < 0) || (index >= descr->nb_items)) return LB_ERR; @@ -1686,20 +1686,8 @@ static LRESULT LISTBOX_RemoveItem( LB_DESCR *descr, INT index ) (descr->nb_items - index) * sizeof(LB_ITEMDATA) ); if (descr->anchor_item == descr->nb_items) descr->anchor_item--; - /* Shrink the item array if possible */ + resize_storage(descr, descr->nb_items); - max_items = descr->items_size; - if (descr->nb_items < max_items - 2*LB_ARRAY_GRANULARITY) - { - max_items -= LB_ARRAY_GRANULARITY; - item = HeapReAlloc( GetProcessHeap(), 0, descr->items, - max_items * sizeof(LB_ITEMDATA) ); - if (item) - { - descr->items_size = max_items; - descr->items = item; - } - } /* Repaint the items */ LISTBOX_UpdateScroll( descr ); -- 2.19.1
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- This has been carefully done so that it matches the previous behavior when LBS_NODATA was used, since SetCount only works with it (so that means it also implies LBS_OWNERDRAWFIXED and so on). count was changed to unsigned so that it passes the last test in the patch series, apparently Windows treats it as unsigned. dlls/comctl32/listbox.c | 45 +++++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index 1d881f1..3d3dfef 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -1740,24 +1740,47 @@ static void LISTBOX_ResetContent( LB_DESCR *descr ) /*********************************************************************** * LISTBOX_SetCount */ -static LRESULT LISTBOX_SetCount( LB_DESCR *descr, INT count ) +static LRESULT LISTBOX_SetCount( LB_DESCR *descr, UINT count ) { - LRESULT ret; + UINT orig_num = descr->nb_items; if (!(descr->style & LBS_NODATA)) return LB_ERR; - /* FIXME: this is far from optimal... */ - if (count > descr->nb_items) + if (count > orig_num) { - while (count > descr->nb_items) - if ((ret = LISTBOX_InsertString( descr, -1, 0 )) < 0) - return ret; + if (!resize_storage(descr, count)) + return LB_ERRSPACE; + memset(&descr->items[orig_num], 0, (count - orig_num) * sizeof(LB_ITEMDATA)); + descr->nb_items = count; + + LISTBOX_UpdateScroll(descr); + LISTBOX_InvalidateItems(descr, orig_num); + + /* If listbox was empty, set focus to the first item */ + if (orig_num == 0) LISTBOX_SetCaretIndex(descr, 0, FALSE); } - else if (count < descr->nb_items) + else if (count < orig_num) { - while (count < descr->nb_items) - if ((ret = LISTBOX_RemoveItem( descr, (descr->nb_items - 1) )) < 0) - return ret; + LISTBOX_InvalidateItems(descr, count); + descr->nb_items = count; + + if (count == 0) SendMessageW(descr->self, LB_RESETCONTENT, 0, 0); + else + { + descr->anchor_item = min(descr->anchor_item, count - 1); + + resize_storage(descr, count); + if (descr->selected_item >= count) + descr->selected_item = -1; + + LISTBOX_UpdateScroll(descr); + + /* If we removed the scrollbar, reset the top of the list */ + if (count <= descr->page_size && orig_num > descr->page_size) + LISTBOX_SetTopItem(descr, 0, TRUE); + + descr->focus_item = min(descr->focus_item, count - 1); + } } InvalidateRect( descr->self, NULL, TRUE ); -- 2.19.1
The LBS_NODATA style's purpose is to drastically improve performance and memory usage on very large lists, since they should function as virtual lists. Thus, don't store any data for single-selection listboxes (descr->items always NULL). Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=32374 Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- Most of the changes are just indentation in InsertItem/RemoveItem. The rest are needed to solve subtle bugs (and because we have to special-case single-selection LBS_NODATA listboxes for storing no data). dlls/comctl32/listbox.c | 130 +++++++++++++++++++++++++--------------- 1 file changed, 82 insertions(+), 48 deletions(-) diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index 3d3dfef..b041d43 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -19,7 +19,7 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA * * TODO: - * - LBS_NODATA + * - LBS_NODATA for multi-selection listboxes */ #include <string.h> @@ -149,9 +149,17 @@ static BOOL resize_storage(LB_DESCR *descr, UINT items_size) static BOOL is_item_selected( const LB_DESCR *descr, UINT index ) { + if (!(descr->style & (LBS_MULTIPLESEL | LBS_EXTENDEDSEL))) + return index == descr->selected_item; return descr->items[index].selected; } +static BOOL is_singlesel_NODATA(const LB_DESCR *descr) +{ + return (descr->style & LBS_NODATA) && + !(descr->style & (LBS_MULTIPLESEL | LBS_EXTENDEDSEL)); +} + /*********************************************************************** * LISTBOX_GetCurrentPageSize * @@ -545,6 +553,7 @@ static void LISTBOX_PaintItem( LB_DESCR *descr, HDC hdc, const RECT *rect, GetClientRect(descr->self, &r); hrgn = set_control_clipping( hdc, &r ); + if (is_singlesel_NODATA(descr)) item = NULL; dis.CtlType = ODT_LISTBOX; dis.CtlID = GetWindowLongPtrW( descr->self, GWLP_ID ); dis.hwndItem = descr->self; @@ -705,6 +714,8 @@ static void LISTBOX_DrawFocusRect( LB_DESCR *descr, BOOL on ) */ static LRESULT LISTBOX_InitStorage( LB_DESCR *descr, INT nb_items ) { + if (is_singlesel_NODATA(descr)) + return LB_OKAY; if (!resize_storage(descr, descr->nb_items + nb_items)) return LB_ERRSPACE; return LB_OKAY; @@ -1456,10 +1467,13 @@ static LRESULT LISTBOX_SetSelection( LB_DESCR *descr, INT index, { INT oldsel = descr->selected_item; if (index == oldsel) return LB_OKAY; - if (oldsel != -1) descr->items[oldsel].selected = FALSE; - if (index != -1) descr->items[index].selected = TRUE; - if (oldsel != -1) LISTBOX_RepaintItem( descr, oldsel, ODA_SELECT ); + if (!(descr->style & LBS_NODATA)) + { + if (oldsel != -1) descr->items[oldsel].selected = FALSE; + if (index != -1) descr->items[index].selected = TRUE; + } descr->selected_item = index; + if (oldsel != -1) LISTBOX_RepaintItem( descr, oldsel, ODA_SELECT ); if (index != -1) LISTBOX_RepaintItem( descr, index, ODA_SELECT ); if (send_notify && descr->nb_items) SEND_NOTIFICATION( descr, (index != -1) ? LBN_SELCHANGE : LBN_SELCANCEL ); @@ -1531,36 +1545,41 @@ static LRESULT LISTBOX_InsertItem( LB_DESCR *descr, INT index, if (index == -1) index = descr->nb_items; else if ((index < 0) || (index > descr->nb_items)) return LB_ERR; - if (!resize_storage(descr, descr->nb_items + 1)) return LB_ERR; - - /* Insert the item structure */ - - item = &descr->items[index]; - if (index < descr->nb_items) - RtlMoveMemory( item + 1, item, - (descr->nb_items - index) * sizeof(LB_ITEMDATA) ); - item->str = str; - item->data = HAS_STRINGS(descr) ? 0 : data; - item->height = 0; - item->selected = FALSE; - descr->nb_items++; - - /* Get item height */ - - if (descr->style & LBS_OWNERDRAWVARIABLE) + if (is_singlesel_NODATA(descr)) { - MEASUREITEMSTRUCT mis; - UINT id = (UINT)GetWindowLongPtrW( descr->self, GWLP_ID ); + descr->nb_items++; + } + else + { + if (!resize_storage(descr, descr->nb_items + 1)) return LB_ERR; + + /* Insert the item structure */ + item = &descr->items[index]; + if (index < descr->nb_items) + RtlMoveMemory( item + 1, item, + (descr->nb_items - index) * sizeof(LB_ITEMDATA) ); + item->str = str; + item->data = HAS_STRINGS(descr) ? 0 : data; + item->height = 0; + item->selected = FALSE; + descr->nb_items++; + + /* Get item height */ + if (descr->style & LBS_OWNERDRAWVARIABLE) + { + MEASUREITEMSTRUCT mis; + UINT id = (UINT)GetWindowLongPtrW( descr->self, GWLP_ID ); - mis.CtlType = ODT_LISTBOX; - mis.CtlID = id; - mis.itemID = index; - mis.itemData = data; - mis.itemHeight = descr->item_height; - SendMessageW( descr->owner, WM_MEASUREITEM, id, (LPARAM)&mis ); - item->height = mis.itemHeight ? mis.itemHeight : 1; - TRACE("[%p]: measure item %d (%s) = %d\n", - descr->self, index, str ? debugstr_w(str) : "", item->height ); + mis.CtlType = ODT_LISTBOX; + mis.CtlID = id; + mis.itemID = index; + mis.itemData = data; + mis.itemHeight = descr->item_height; + SendMessageW( descr->owner, WM_MEASUREITEM, id, (LPARAM)&mis ); + item->height = mis.itemHeight ? mis.itemHeight : 1; + TRACE("[%p]: measure item %d (%s) = %d\n", + descr->self, index, str ? debugstr_w(str) : "", item->height ); + } } /* Repaint the items */ @@ -1674,19 +1693,29 @@ static LRESULT LISTBOX_RemoveItem( LB_DESCR *descr, INT index ) LISTBOX_InvalidateItems( descr, index ); descr->nb_items--; - LISTBOX_DeleteItem( descr, index ); - - if (!descr->nb_items) return LB_OKAY; + if (is_singlesel_NODATA(descr)) + { + if (!descr->nb_items) + { + SendMessageW(descr->self, LB_RESETCONTENT, 0, 0); + return LB_OKAY; + } + } + else + { + LISTBOX_DeleteItem( descr, index ); - /* Remove the item */ + if (!descr->nb_items) return LB_OKAY; - item = &descr->items[index]; - if (index < descr->nb_items) - RtlMoveMemory( item, item + 1, - (descr->nb_items - index) * sizeof(LB_ITEMDATA) ); - if (descr->anchor_item == descr->nb_items) descr->anchor_item--; + /* Remove the item */ + item = &descr->items[index]; + if (index < descr->nb_items) + RtlMoveMemory( item, item + 1, + (descr->nb_items - index) * sizeof(LB_ITEMDATA) ); - resize_storage(descr, descr->nb_items); + resize_storage(descr, descr->nb_items); + } + descr->anchor_item = min(descr->anchor_item, descr->nb_items - 1); /* Repaint the items */ @@ -1725,7 +1754,8 @@ static void LISTBOX_ResetContent( LB_DESCR *descr ) { INT i; - for(i = descr->nb_items - 1; i>=0; i--) LISTBOX_DeleteItem( descr, i); + if (descr->items) + for (i = descr->nb_items - 1; i >= 0; i--) LISTBOX_DeleteItem(descr, i); HeapFree( GetProcessHeap(), 0, descr->items ); descr->nb_items = 0; descr->top_item = 0; @@ -1748,9 +1778,12 @@ static LRESULT LISTBOX_SetCount( LB_DESCR *descr, UINT count ) if (count > orig_num) { - if (!resize_storage(descr, count)) - return LB_ERRSPACE; - memset(&descr->items[orig_num], 0, (count - orig_num) * sizeof(LB_ITEMDATA)); + if (descr->style & (LBS_MULTIPLESEL | LBS_EXTENDEDSEL)) + { + if (!resize_storage(descr, count)) + return LB_ERRSPACE; + memset(&descr->items[orig_num], 0, (count - orig_num) * sizeof(LB_ITEMDATA)); + } descr->nb_items = count; LISTBOX_UpdateScroll(descr); @@ -1769,8 +1802,9 @@ static LRESULT LISTBOX_SetCount( LB_DESCR *descr, UINT count ) { descr->anchor_item = min(descr->anchor_item, count - 1); - resize_storage(descr, count); - if (descr->selected_item >= count) + if (descr->style & (LBS_MULTIPLESEL | LBS_EXTENDEDSEL)) + resize_storage(descr, count); + else if (descr->selected_item >= descr->nb_items) descr->selected_item = -1; LISTBOX_UpdateScroll(descr); -- 2.19.1
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/comctl32/tests/listbox.c | 130 ++++++++++++++++++++++++++++++++++ 1 file changed, 130 insertions(+) diff --git a/dlls/comctl32/tests/listbox.c b/dlls/comctl32/tests/listbox.c index e789483..3628b5e 100644 --- a/dlls/comctl32/tests/listbox.c +++ b/dlls/comctl32/tests/listbox.c @@ -784,6 +784,135 @@ static void test_listbox_height(void) DestroyWindow( hList ); } +static void test_changing_selection_styles(void) +{ + static const DWORD styles[] = + { + 0, + LBS_NODATA | LBS_OWNERDRAWFIXED + }; + static const DWORD selstyles[] = + { + 0, + LBS_MULTIPLESEL, + LBS_EXTENDEDSEL, + LBS_MULTIPLESEL | LBS_EXTENDEDSEL + }; + static const LONG selexpect_single[] = { 0, 0, 1 }; + static const LONG selexpect_single2[] = { 1, 0, 0 }; + static const LONG selexpect_multi[] = { 1, 0, 1 }; + static const LONG selexpect_multi2[] = { 1, 1, 0 }; + + HWND parent, listbox; + DWORD style; + LONG ret; + UINT i, j, k; + + parent = create_parent(); + ok(parent != NULL, "Failed to create parent window.\n"); + for (i = 0; i < ARRAY_SIZE(styles); i++) + { + /* Test if changing selection styles affects selection storage */ + for (j = 0; j < ARRAY_SIZE(selstyles); j++) + { + LONG setcursel_expect, selitemrange_expect, getselcount_expect; + const LONG *selexpect; + + listbox = CreateWindowA(WC_LISTBOXA, "TestList", styles[i] | selstyles[j] | WS_CHILD | WS_VISIBLE, + 0, 0, 100, 100, parent, (HMENU)ID_LISTBOX, NULL, 0); + ok(listbox != NULL, "%u: Failed to create ListBox window.\n", j); + + if (selstyles[j] & (LBS_MULTIPLESEL | LBS_EXTENDEDSEL)) + { + setcursel_expect = LB_ERR; + selitemrange_expect = LB_OKAY; + getselcount_expect = 2; + selexpect = selexpect_multi; + } + else + { + setcursel_expect = 2; + selitemrange_expect = LB_ERR; + getselcount_expect = LB_ERR; + selexpect = selexpect_single; + } + + for (k = 0; k < ARRAY_SIZE(selexpect_multi); k++) + { + ret = SendMessageA(listbox, LB_INSERTSTRING, -1, (LPARAM)"x"); + ok(ret == k, "%u: Unexpected return value %d, expected %d.\n", j, ret, k); + } + ret = SendMessageA(listbox, LB_GETCOUNT, 0, 0); + ok(ret == ARRAY_SIZE(selexpect_multi), "%u: Unexpected count %d.\n", j, ret); + + /* Select items with different methods */ + ret = SendMessageA(listbox, LB_SETCURSEL, 2, 0); + ok(ret == setcursel_expect, "%u: Unexpected return value %d.\n", j, ret); + ret = SendMessageA(listbox, LB_SELITEMRANGE, TRUE, MAKELPARAM(0, 0)); + ok(ret == selitemrange_expect, "%u: Unexpected return value %d.\n", j, ret); + ret = SendMessageA(listbox, LB_SELITEMRANGE, TRUE, MAKELPARAM(2, 2)); + ok(ret == selitemrange_expect, "%u: Unexpected return value %d.\n", j, ret); + + /* Verify that the proper items are selected */ + for (k = 0; k < ARRAY_SIZE(selexpect_multi); k++) + { + ret = SendMessageA(listbox, LB_GETSEL, k, 0); + ok(ret == selexpect[k], "%u: Unexpected selection state %d, expected %d.\n", + j, ret, selexpect[k]); + } + + /* Now change the selection style */ + style = GetWindowLongA(listbox, GWL_STYLE); + ok((style & (LBS_MULTIPLESEL | LBS_EXTENDEDSEL)) == selstyles[j], + "%u: unexpected window styles %#x.\n", j, style); + if (selstyles[j] & (LBS_MULTIPLESEL | LBS_EXTENDEDSEL)) + style &= ~selstyles[j]; + else + style |= LBS_MULTIPLESEL | LBS_EXTENDEDSEL; + SetWindowLongA(listbox, GWL_STYLE, style); + style = GetWindowLongA(listbox, GWL_STYLE); + ok(!(style & selstyles[j]), "%u: unexpected window styles %#x.\n", j, style); + + /* Verify that the same items are selected */ + ret = SendMessageA(listbox, LB_GETSELCOUNT, 0, 0); + ok(ret == getselcount_expect, "%u: expected %d from LB_GETSELCOUNT, got %d\n", + j, getselcount_expect, ret); + + for (k = 0; k < ARRAY_SIZE(selexpect_multi); k++) + { + ret = SendMessageA(listbox, LB_GETSEL, k, 0); + ok(ret == selexpect[k], "%u: Unexpected selection state %d, expected %d.\n", + j, ret, selexpect[k]); + } + + /* Lastly see if we can still change the selection as before with old style */ + if (setcursel_expect != LB_ERR) setcursel_expect = 0; + ret = SendMessageA(listbox, LB_SETCURSEL, 0, 0); + ok(ret == setcursel_expect, "%u: Unexpected return value %d.\n", j, ret); + ret = SendMessageA(listbox, LB_SELITEMRANGE, TRUE, MAKELPARAM(1, 1)); + ok(ret == selitemrange_expect, "%u: Unexpected return value %d.\n", j, ret); + ret = SendMessageA(listbox, LB_SELITEMRANGE, FALSE, MAKELPARAM(2, 2)); + ok(ret == selitemrange_expect, "%u: Unexpected return value %d.\n", j, ret); + + /* And verify the selections */ + selexpect = (selstyles[j] & (LBS_MULTIPLESEL | LBS_EXTENDEDSEL)) ? selexpect_multi2 : selexpect_single2; + ret = SendMessageA(listbox, LB_GETSELCOUNT, 0, 0); + ok(ret == getselcount_expect, "%u: expected %d from LB_GETSELCOUNT, got %d\n", + j, getselcount_expect, ret); + + for (k = 0; k < ARRAY_SIZE(selexpect_multi); k++) + { + ret = SendMessageA(listbox, LB_GETSEL, k, 0); + ok(ret == selexpect[k], "%u: Unexpected selection state %d, expected %d.\n", + j, ret, selexpect[k]); + } + + DestroyWindow(listbox); + } + } + DestroyWindow(parent); +} + static void test_itemfrompoint(void) { /* WS_POPUP is required in order to have a more accurate size calculation ( @@ -2420,6 +2549,7 @@ START_TEST(listbox) test_LB_SELITEMRANGE(); test_LB_SETCURSEL(); test_listbox_height(); + test_changing_selection_styles(); test_itemfrompoint(); test_listbox_item_data(); test_listbox_LB_DIR(); -- 2.19.1
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/comctl32/tests/listbox.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dlls/comctl32/tests/listbox.c b/dlls/comctl32/tests/listbox.c index 3628b5e..5442000 100644 --- a/dlls/comctl32/tests/listbox.c +++ b/dlls/comctl32/tests/listbox.c @@ -1996,6 +1996,11 @@ static void test_set_count( void ) GetUpdateRect( listbox, &r, TRUE ); ok( !IsRectEmpty( &r ), "got empty rect\n"); + ret = SendMessageA( listbox, LB_SETCOUNT, -5, 0 ); + ok( ret == 0, "got %d\n", ret ); + ret = SendMessageA( listbox, LB_GETCOUNT, 0, 0 ); + ok( ret == -5, "got %d\n", ret ); + DestroyWindow( listbox ); for (i = 0; i < ARRAY_SIZE(styles); ++i) -- 2.19.1
participants (2)
-
Gabriel Ivăncescu -
Nikolay Sivov