Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/comctl32/listbox.c | 43 ++++++++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 13 deletions(-)
diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index c92c2a2..db7d474 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -144,6 +144,12 @@ static BOOL resize_storage(LB_DESCR *descr, UINT items_size) descr->items_size = items_size; descr->items = items; } + + if ((descr->style & LBS_NODATA) && items_size > descr->nb_items) + { + memset(&descr->items[descr->nb_items], 0, + (items_size - descr->nb_items) * sizeof(LB_ITEMDATA)); + } return TRUE; }
@@ -1742,25 +1748,36 @@ 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) - { - while (count > descr->nb_items) - if ((ret = LISTBOX_InsertString( descr, -1, 0 )) < 0) - return ret; - } - else if (count < descr->nb_items) + if (!resize_storage(descr, count)) + return LB_ERRSPACE; + descr->nb_items = count; + + if (count) { - while (count < descr->nb_items) - if ((ret = LISTBOX_RemoveItem( descr, (descr->nb_items - 1) )) < 0) - return ret; + LISTBOX_UpdateScroll(descr); + if (count < orig_num) + { + descr->anchor_item = min(descr->anchor_item, count - 1); + if (descr->selected_item >= count) + descr->selected_item = -1; + + /* 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); + } + + /* If it was empty before growing, set focus to the first item */ + else if (orig_num == 0) LISTBOX_SetCaretIndex(descr, 0, FALSE); } + else SendMessageW(descr->self, LB_RESETCONTENT, 0, 0);
InvalidateRect( descr->self, NULL, TRUE ); return LB_OKAY;
Needed for LBS_NODATA.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/comctl32/listbox.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index db7d474..c84d317 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -155,6 +155,8 @@ 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; }
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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 68e4073..0cdcdea 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 ( @@ -2483,6 +2612,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();
Hi,
While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=47513
Your paranoid android.
=== w7pro64 (32 bit report) ===
comctl32: listbox.c:1235: Test failed: SendMessage(LB_DIR, DDL_DRIVES, *) filled with 26 entries, expected 25 listbox.c:1368: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_EXCLUSIVE) filled with 4 entries, expected 5 listbox.c:1411: Test failed: SendMessage(LB_DIR, DDL_DIRECTORY|DDL_DRIVES|DDL_EXCLUSIVE) filled with 6 entries, expected 7
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/comctl32/listbox.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index c84d317..34d555a 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -127,6 +127,11 @@ static TIMER_DIRECTION LISTBOX_Timer = LB_TIMER_NONE;
static LRESULT LISTBOX_GetItemRect( const LB_DESCR *descr, INT index, RECT *rect );
+static ULONG_PTR get_item_data( const LB_DESCR *descr, UINT index ) +{ + return (descr->style & LBS_NODATA) ? 0 : descr->items[index].data; +} + static BOOL resize_storage(LB_DESCR *descr, UINT items_size) { LB_ITEMDATA *items; @@ -565,7 +570,7 @@ static void LISTBOX_PaintItem( LB_DESCR *descr, HDC hdc, const RECT *rect, if (focused) dis.itemState |= ODS_FOCUS; if (!IsWindowEnabled(descr->self)) dis.itemState |= ODS_DISABLED; - dis.itemData = item ? item->data : 0; + dis.itemData = get_item_data(descr, index); dis.rcItem = *rect; TRACE("[%p]: drawitem %d (%s) action=%02x state=%02x rect=%s\n", descr->self, index, item ? debugstr_w(item->str) : "", action, @@ -788,8 +793,7 @@ static LRESULT LISTBOX_GetText( LB_DESCR *descr, INT index, LPWSTR buffer, BOOL } else { if (buffer) - *((ULONG_PTR *)buffer) = (descr->style & LBS_NODATA) - ? 0 : descr->items[index].data; + *((ULONG_PTR *)buffer) = get_item_data(descr, index); len = sizeof(ULONG_PTR); } return len; @@ -837,7 +841,7 @@ static INT LISTBOX_FindStringPos( LB_DESCR *descr, LPCWSTR str, BOOL exact ) /* note that some application (MetaStock) expects the second item * to be in the listbox */ cis.itemID1 = index; - cis.itemData1 = descr->items[index].data; + cis.itemData1 = get_item_data(descr, index); cis.itemID2 = -1; cis.itemData2 = (ULONG_PTR)str; cis.dwLocaleId = descr->locale; @@ -1641,7 +1645,7 @@ static LRESULT LISTBOX_InsertString( LB_DESCR *descr, INT index, LPCWSTR str ) static void LISTBOX_DeleteItem( LB_DESCR *descr, INT index ) { /* save the item data before it gets freed by LB_RESETCONTENT */ - ULONG_PTR item_data = descr->items[index].data; + ULONG_PTR item_data = get_item_data(descr, index); LPWSTR item_str = descr->items[index].str;
if (!descr->nb_items) @@ -2643,7 +2647,7 @@ static LRESULT CALLBACK LISTBOX_WindowProc( HWND hwnd, UINT msg, WPARAM wParam, SetLastError(ERROR_INVALID_INDEX); return LB_ERR; } - return (descr->style & LBS_NODATA) ? 0 : descr->items[wParam].data; + return get_item_data(descr, wParam);
case LB_SETITEMDATA: if (((INT)wParam < 0) || ((INT)wParam >= descr->nb_items))
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
I've left FindStringPos alone because I'm not 100% sure if a listbox with strings is allowed to have a NULL str (which would give a different code path than currently, if I were to check for NULL from get_item_string instead). LISTBOX_lstrcmpiW ends up using CompareStringW which does check for NULL, so I'm not entirely certain about it.
dlls/comctl32/listbox.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index 34d555a..d526a2b 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -132,6 +132,11 @@ static ULONG_PTR get_item_data( const LB_DESCR *descr, UINT index ) return (descr->style & LBS_NODATA) ? 0 : descr->items[index].data; }
+static WCHAR *get_item_string( const LB_DESCR *descr, UINT index ) +{ + return (descr->style & LBS_NODATA) ? NULL : descr->items[index].str; +} + static BOOL resize_storage(LB_DESCR *descr, UINT items_size) { LB_ITEMDATA *items; @@ -573,7 +578,7 @@ static void LISTBOX_PaintItem( LB_DESCR *descr, HDC hdc, const RECT *rect, dis.itemData = get_item_data(descr, index); dis.rcItem = *rect; TRACE("[%p]: drawitem %d (%s) action=%02x state=%02x rect=%s\n", - descr->self, index, item ? debugstr_w(item->str) : "", action, + descr->self, index, debugstr_w(get_item_string(descr, index)), action, dis.itemState, wine_dbgstr_rect(rect) ); SendMessageW(descr->owner, WM_DRAWITEM, dis.CtlID, (LPARAM)&dis); SelectClipRgn( hdc, hrgn ); @@ -1646,7 +1651,7 @@ static void LISTBOX_DeleteItem( LB_DESCR *descr, INT index ) { /* save the item data before it gets freed by LB_RESETCONTENT */ ULONG_PTR item_data = get_item_data(descr, index); - LPWSTR item_str = descr->items[index].str; + LPWSTR item_str = get_item_string(descr, index);
if (!descr->nb_items) SendMessageW( descr->self, LB_RESETCONTENT, 0, 0 ); @@ -1668,8 +1673,7 @@ static void LISTBOX_DeleteItem( LB_DESCR *descr, INT index ) dis.itemData = item_data; SendMessageW( descr->owner, WM_DELETEITEM, id, (LPARAM)&dis ); } - if (HAS_STRINGS(descr)) - HeapFree( GetProcessHeap(), 0, item_str ); + HeapFree( GetProcessHeap(), 0, item_str ); }
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
is_singlesel_NODATA is needed otherwise it fails the tests for multi-selection nodata, because it's not implemented yet.
dlls/comctl32/listbox.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index d526a2b..374b1df 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -127,6 +127,11 @@ static TIMER_DIRECTION LISTBOX_Timer = LB_TIMER_NONE;
static LRESULT LISTBOX_GetItemRect( const LB_DESCR *descr, INT index, RECT *rect );
+static BOOL is_singlesel_NODATA(const LB_DESCR *descr) +{ + return (descr->style & (LBS_NODATA | LBS_MULTIPLESEL | LBS_EXTENDEDSEL)) == LBS_NODATA; +} + static ULONG_PTR get_item_data( const LB_DESCR *descr, UINT index ) { return (descr->style & LBS_NODATA) ? 0 : descr->items[index].data; @@ -170,6 +175,11 @@ static BOOL is_item_selected( const LB_DESCR *descr, UINT index ) return descr->items[index].selected; }
+static void set_item_selected_state(LB_DESCR *descr, UINT index, BOOL state) +{ + if (!is_singlesel_NODATA(descr)) descr->items[index].selected = state; +} + /*********************************************************************** * LISTBOX_GetCurrentPageSize * @@ -1433,7 +1443,7 @@ static LRESULT LISTBOX_SelectItemRange( LB_DESCR *descr, INT first, for (i = first; i <= last; i++) { if (descr->items[i].selected) continue; - descr->items[i].selected = TRUE; + set_item_selected_state(descr, i, TRUE); LISTBOX_InvalidateItemRect(descr, i); } } @@ -1442,7 +1452,7 @@ static LRESULT LISTBOX_SelectItemRange( LB_DESCR *descr, INT first, for (i = first; i <= last; i++) { if (!descr->items[i].selected) continue; - descr->items[i].selected = FALSE; + set_item_selected_state(descr, i, FALSE); LISTBOX_InvalidateItemRect(descr, i); } } @@ -1475,10 +1485,10 @@ 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 (oldsel != -1) set_item_selected_state(descr, oldsel, FALSE); + if (index != -1) set_item_selected_state(descr, index, 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 );
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/comctl32/listbox.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index 374b1df..37cbbc6 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -1659,19 +1659,12 @@ static LRESULT LISTBOX_InsertString( LB_DESCR *descr, INT index, LPCWSTR str ) */ static void LISTBOX_DeleteItem( LB_DESCR *descr, INT index ) { - /* save the item data before it gets freed by LB_RESETCONTENT */ - ULONG_PTR item_data = get_item_data(descr, index); - LPWSTR item_str = get_item_string(descr, index); - - if (!descr->nb_items) - SendMessageW( descr->self, LB_RESETCONTENT, 0, 0 ); - /* Note: Win 3.1 only sends DELETEITEM on owner-draw items, * while Win95 sends it for all items with user data. * It's probably better to send it too often than not * often enough, so this is what we do here. */ - if (IS_OWNERDRAW(descr) || item_data) + if (IS_OWNERDRAW(descr) || get_item_data(descr, index)) { DELETEITEMSTRUCT dis; UINT id = (UINT)GetWindowLongPtrW( descr->self, GWLP_ID ); @@ -1680,10 +1673,10 @@ static void LISTBOX_DeleteItem( LB_DESCR *descr, INT index ) dis.CtlID = id; dis.itemID = index; dis.hwndItem = descr->self; - dis.itemData = item_data; + dis.itemData = get_item_data(descr, index); SendMessageW( descr->owner, WM_DELETEITEM, id, (LPARAM)&dis ); } - HeapFree( GetProcessHeap(), 0, item_str ); + HeapFree( GetProcessHeap(), 0, get_item_string(descr, index) ); }
@@ -1701,11 +1694,14 @@ static LRESULT LISTBOX_RemoveItem( LB_DESCR *descr, INT index ) /* We need to invalidate the original rect instead of the updated one. */ LISTBOX_InvalidateItems( descr, index );
+ if (descr->nb_items == 1) + { + SendMessageW(descr->self, LB_RESETCONTENT, 0, 0); + return LB_OKAY; + } descr->nb_items--; LISTBOX_DeleteItem( descr, index );
- if (!descr->nb_items) return LB_OKAY; - /* Remove the item */
item = &descr->items[index];
-
Gabriel Ivăncescu -
Marvin