Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
v3: Fix the tests on broken Windows versions (only on WOW64) and place the tests at the beginning of the patchset. Hopefully this should be enough now.
dlls/comctl32/tests/listbox.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/dlls/comctl32/tests/listbox.c b/dlls/comctl32/tests/listbox.c index d4bb2cc..60aa59a 100644 --- a/dlls/comctl32/tests/listbox.c +++ b/dlls/comctl32/tests/listbox.c @@ -1766,6 +1766,13 @@ static void test_set_count( void ) GetUpdateRect( listbox, &r, TRUE ); ok( !IsRectEmpty( &r ), "got empty rect\n");
+ DestroyWindow( listbox ); + + listbox = create_listbox( LBS_OWNERDRAWFIXED | WS_CHILD | WS_VISIBLE, parent ); + + ret = SendMessageA( listbox, LB_SETCOUNT, 100, 0 ); + todo_wine ok( ret == LB_ERR, "expected %d, got %d\n", LB_ERR, ret ); + DestroyWindow( listbox ); DestroyWindow( parent ); }
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/comctl32/tests/listbox.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
diff --git a/dlls/comctl32/tests/listbox.c b/dlls/comctl32/tests/listbox.c index 60aa59a..f46b4f3 100644 --- a/dlls/comctl32/tests/listbox.c +++ b/dlls/comctl32/tests/listbox.c @@ -436,6 +436,32 @@ static void test_ownerdraw(void) DestroyWindow(parent); }
+static void test_both_ownerdraw(void) +{ + HWND hLB; + INT ret; + + /* test both FIXED and VARIABLE at once, since FIXED should override VARIABLE */ + hLB = CreateWindowA(WC_LISTBOXA, "TestList", LBS_OWNERDRAWFIXED | LBS_OWNERDRAWVARIABLE, 0, 0, 100, 100, NULL, NULL, NULL, 0); + ok(hLB != NULL, "last error 0x%08x\n", GetLastError()); + if (!hLB) return; + + ret = SendMessageA(hLB, LB_INSERTSTRING, -1, 0); + ok(ret == 0, "expected 0, got %d\n", ret); + ret = SendMessageA(hLB, LB_INSERTSTRING, -1, 0); + ok(ret == 1, "expected 1, got %d\n", ret); + + ret = SendMessageA(hLB, LB_SETITEMHEIGHT, 0, 13); + ok(ret == LB_OKAY, "LB_SETITEMHEIGHT failed: %d\n", ret); + ret = SendMessageA(hLB, LB_SETITEMHEIGHT, 1, 42); + ok(ret == LB_OKAY, "LB_SETITEMHEIGHT failed: %d\n", ret); + + ret = SendMessageA(hLB, LB_GETITEMHEIGHT, 0, 0); + todo_wine ok(ret == 42, "LBS_OWNERDRAWFIXED did not override LBS_OWNERDRAWVARIABLE, got height %d\n", ret); + + DestroyWindow (hLB); +} + #define listbox_test_query(exp, got) \ ok(exp.selected == got.selected, "expected selected %d, got %d\n", exp.selected, got.selected); \ ok(exp.anchor == got.anchor, "expected anchor %d, got %d\n", exp.anchor, got.anchor); \ @@ -2210,6 +2236,7 @@ START_TEST(listbox) test_listbox(); test_item_height(); test_ownerdraw(); + test_both_ownerdraw(); test_LB_SELITEMRANGE(); test_LB_SETCURSEL(); test_listbox_height();
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/comctl32/tests/listbox.c | 103 ++++++++++++++++++++++++++++++---- 1 file changed, 91 insertions(+), 12 deletions(-)
diff --git a/dlls/comctl32/tests/listbox.c b/dlls/comctl32/tests/listbox.c index f46b4f3..5fdfaf8 100644 --- a/dlls/comctl32/tests/listbox.c +++ b/dlls/comctl32/tests/listbox.c @@ -167,10 +167,10 @@ static void keypress(HWND handle, WPARAM keycode, BYTE scancode, BOOL extended) listbox_todo_field_ok(t, s, caret, got); \ listbox_todo_field_ok(t, s, selcount, got)
-static void run_test(const struct listbox_test test) +static void run_test(const struct listbox_test test, DWORD extra_styles) { struct listbox_stat answer; - HWND hLB=create_listbox (test.prop.add_style, 0); + HWND hLB=create_listbox (test.prop.add_style | extra_styles, 0); RECT second_item; int i, res;
@@ -190,13 +190,13 @@ static void run_test(const struct listbox_test test)
DestroyWindow(hLB);
- hLB = create_listbox(test.prop.add_style, 0); + hLB = create_listbox(test.prop.add_style | extra_styles, 0);
SendMessageA(hLB, LB_SELITEMRANGE, TRUE, MAKELPARAM(1, 2)); listbox_query(hLB, &answer); listbox_ok(test, sel, answer);
- for (i = 0; i < 4; i++) + if (!(extra_styles & LBS_NODATA)) for (i = 0; i < 4; i++) { DWORD size = SendMessageA(hLB, LB_GETTEXTLEN, i, 0); int resA, resW; @@ -1803,6 +1803,75 @@ static void test_set_count( void ) DestroyWindow( parent ); }
+static void test_nodata( void ) +{ + static const UINT invalid_idx[] = { -2, 2 }; + static const UINT valid_idx[] = { 0, 1 }; + HWND listbox; + INT i, ret; + + /* On some buggy 64-bit Windows versions via WOW64 (32-bit app), + the data retrieved from LB_GETTEXT is always 8 for LBS_NODATA, + so we can't rely on sizeof(void*) and have to hardcode it */ + UINT64 data; + + listbox = CreateWindowA(WC_LISTBOXA, "TestList", LBS_NODATA | LBS_OWNERDRAWFIXED | WS_VISIBLE, + 0, 0, 100, 100, NULL, NULL, NULL, 0); + ok(listbox != NULL, "last error 0x%08x\n", GetLastError()); + if (!listbox) return; + + ret = SendMessageA(listbox, LB_INSERTSTRING, -1, 0); + ok(ret == 0, "expected 0, got %d\n", ret); + ret = SendMessageA(listbox, LB_INSERTSTRING, -1, 0); + ok(ret == 1, "expected 1, got %d\n", ret); + ret = SendMessageA(listbox, LB_GETCOUNT, 0, 0); + ok(ret == 2, "Expected 2 items, got %d\n", ret); + + /* try invalid indices */ + for (i = 0; i < ARRAY_SIZE(invalid_idx); i++) + { + ret = SendMessageA(listbox, LB_SETITEMDATA, invalid_idx[i], 42); + ok(ret == LB_ERR, "got %d\n", ret); + ret = SendMessageA(listbox, LB_GETTEXTLEN, invalid_idx[i], 0); + ok(ret == LB_ERR, "got %d\n", ret); + if (ret == LB_ERR) + { + ret = SendMessageA(listbox, LB_GETTEXT, invalid_idx[i], (LPARAM)&data); + ok(ret == LB_ERR, "got %d\n", ret); + } + ret = SendMessageA(listbox, LB_GETITEMDATA, invalid_idx[i], 0); + ok(ret == LB_ERR, "got %d\n", ret); + } + + /* valid indices should always retreive zeros */ + for (i = 0; i < ARRAY_SIZE(valid_idx); i++) + { + ret = SendMessageA(listbox, LB_SETITEMDATA, valid_idx[i], 42); + ok(ret == TRUE, "got %d\n", ret); + ret = SendMessageA(listbox, LB_GETTEXTLEN, valid_idx[i], 0); + ok(ret <= sizeof(data), "got %d\n", ret); + if (ret <= sizeof(data)) + { + data = 0xdeadbeef; + ret = SendMessageA(listbox, LB_GETTEXT, valid_idx[i], (LPARAM)&data); + ok(ret <= sizeof(data), "got %d\n", ret); + todo_wine ok(data == 0, "LB_GETTEXT should retrieve 0 with LBS_NODATA, got 0x%llx\n", data); + } + ret = SendMessageA(listbox, LB_GETITEMDATA, valid_idx[i], 0); + todo_wine ok(ret == 0, "LB_GETITEMDATA should return 0 with LBS_NODATA, got %d\n", ret); + } + + /* test more invalid messages with LBS_NODATA */ + ret = SendMessageA(listbox, LB_FINDSTRING, 1, 42); + todo_wine ok(ret == LB_ERR, "got %d\n", ret); + ret = SendMessageA(listbox, LB_FINDSTRINGEXACT, 1, 42); + todo_wine ok(ret == LB_ERR, "got %d\n", ret); + ret = SendMessageA(listbox, LB_SELECTSTRING, 1, 42); + todo_wine ok(ret == LB_ERR, "got %d\n", ret); + + DestroyWindow(listbox); +} + static int lb_getlistboxinfo;
static LRESULT WINAPI listbox_subclass_proc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) @@ -2098,14 +2167,23 @@ static void test_listbox(void) { 2, 2, 2, LB_ERR}, {0,0,0,0}, {LB_ERR, LB_ERR, 0, LB_ERR}, {0,0,0,0}};
- run_test(SS); - run_test(SS_NS); - run_test(MS); - run_test(MS_NS); - run_test(ES); - run_test(ES_NS); - run_test(EMS); - run_test(EMS_NS); + run_test(SS, 0); + run_test(SS_NS, 0); + run_test(MS, 0); + run_test(MS_NS, 0); + run_test(ES, 0); + run_test(ES_NS, 0); + run_test(EMS, 0); + run_test(EMS_NS, 0); + + run_test(SS, LBS_NODATA | LBS_OWNERDRAWFIXED); + run_test(SS_NS, LBS_NODATA | LBS_OWNERDRAWFIXED); + run_test(MS, LBS_NODATA | LBS_OWNERDRAWFIXED); + run_test(MS_NS, LBS_NODATA | LBS_OWNERDRAWFIXED); + run_test(ES, LBS_NODATA | LBS_OWNERDRAWFIXED); + run_test(ES_NS, LBS_NODATA | LBS_OWNERDRAWFIXED); + run_test(EMS, LBS_NODATA | LBS_OWNERDRAWFIXED); + run_test(EMS_NS, LBS_NODATA | LBS_OWNERDRAWFIXED); }
static const struct message lb_addstring_ownerdraw_parent_seq[] = @@ -2245,6 +2323,7 @@ START_TEST(listbox) test_listbox_LB_DIR(); test_listbox_dlgdir(); test_set_count(); + test_nodata(); test_GetListBoxInfo(); test_missing_lbuttonup(); test_extents();
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/comctl32/listbox.c | 5 ++++- dlls/comctl32/tests/listbox.c | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index 2137ef8..0a7c341 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -1752,7 +1752,7 @@ static LRESULT LISTBOX_SetCount( LB_DESCR *descr, INT count ) { LRESULT ret;
- if (HAS_STRINGS(descr)) + if (!(descr->style & LBS_NODATA)) { SetLastError(ERROR_SETCOUNT_ON_BAD_LB); return LB_ERR; @@ -2518,6 +2518,9 @@ static BOOL LISTBOX_Create( HWND hwnd, LPHEADCOMBO lphc ) if (descr->style & LBS_OWNERDRAWVARIABLE) descr->style |= LBS_NOINTEGRALHEIGHT; descr->item_height = LISTBOX_SetFont( descr, 0 );
+ if ((descr->style & (LBS_OWNERDRAWFIXED | LBS_HASSTRINGS | LBS_SORT)) != LBS_OWNERDRAWFIXED) + descr->style &= ~LBS_NODATA; + if (descr->style & LBS_OWNERDRAWFIXED) { if( descr->lphc && (descr->lphc->dwStyle & CBS_DROPDOWN)) diff --git a/dlls/comctl32/tests/listbox.c b/dlls/comctl32/tests/listbox.c index 5fdfaf8..b4be05c 100644 --- a/dlls/comctl32/tests/listbox.c +++ b/dlls/comctl32/tests/listbox.c @@ -1797,7 +1797,7 @@ static void test_set_count( void ) listbox = create_listbox( LBS_OWNERDRAWFIXED | WS_CHILD | WS_VISIBLE, parent );
ret = SendMessageA( listbox, LB_SETCOUNT, 100, 0 ); - todo_wine ok( ret == LB_ERR, "expected %d, got %d\n", LB_ERR, ret ); + ok( ret == LB_ERR, "expected %d, got %d\n", LB_ERR, ret );
DestroyWindow( listbox ); DestroyWindow( parent );
On 11/8/18 2:39 PM, Gabriel Ivăncescu wrote:
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com
dlls/comctl32/listbox.c | 5 ++++- dlls/comctl32/tests/listbox.c | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index 2137ef8..0a7c341 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -1752,7 +1752,7 @@ static LRESULT LISTBOX_SetCount( LB_DESCR *descr, INT count ) { LRESULT ret;
- if (HAS_STRINGS(descr))
- if (!(descr->style & LBS_NODATA))
I think HAS_STRINGS should consider LBS_NODATA instead.
On Sat, Nov 10, 2018 at 11:46 PM Nikolay Sivov nsivov@codeweavers.com wrote:
On 11/8/18 2:39 PM, Gabriel Ivăncescu wrote:
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com
dlls/comctl32/listbox.c | 5 ++++- dlls/comctl32/tests/listbox.c | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index 2137ef8..0a7c341 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -1752,7 +1752,7 @@ static LRESULT LISTBOX_SetCount( LB_DESCR *descr, INT count ) { LRESULT ret;
- if (HAS_STRINGS(descr))
- if (!(descr->style & LBS_NODATA))
I think HAS_STRINGS should consider LBS_NODATA instead.
How is that going to work? Consider these listboxes:
(1) LBS_OWNERDRAWFIXED (2) LBS_OWNERDRAWFIXED | LBS_HASSTRINGS (3) LBS_OWNERDRAWFIXED | LBS_NODATA (4) LBS_OWNERDRAWFIXED | LBS_NODATA | LBS_HASSTRINGS
SetCount must only succeed with (3). For (4) the clear of LBS_NODATA is enough, since LBS_HASSTRINGS overrides and disables it.
I don't see how adding anything to HAS_STRINGS will solve the issue of (1) being a distinct listbox with no strings and without LBS_NODATA. I mean, it doesn't have strings, but it should also fail in SetCount.
Of course, I can separate the patch that clears LBS_NODATA and place it before this patch, if you want.
On 11/11/18 1:01 PM, Gabriel Ivăncescu wrote:
On Sat, Nov 10, 2018 at 11:46 PM Nikolay Sivov nsivov@codeweavers.com wrote:
On 11/8/18 2:39 PM, Gabriel Ivăncescu wrote:
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com
dlls/comctl32/listbox.c | 5 ++++- dlls/comctl32/tests/listbox.c | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index 2137ef8..0a7c341 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -1752,7 +1752,7 @@ static LRESULT LISTBOX_SetCount( LB_DESCR *descr, INT count ) { LRESULT ret;
- if (HAS_STRINGS(descr))
- if (!(descr->style & LBS_NODATA))
I think HAS_STRINGS should consider LBS_NODATA instead.
How is that going to work? Consider these listboxes:
(1) LBS_OWNERDRAWFIXED (2) LBS_OWNERDRAWFIXED | LBS_HASSTRINGS (3) LBS_OWNERDRAWFIXED | LBS_NODATA (4) LBS_OWNERDRAWFIXED | LBS_NODATA | LBS_HASSTRINGS
SetCount must only succeed with (3). For (4) the clear of LBS_NODATA is enough, since LBS_HASSTRINGS overrides and disables it.
I don't see how adding anything to HAS_STRINGS will solve the issue of (1) being a distinct listbox with no strings and without LBS_NODATA. I mean, it doesn't have strings, but it should also fail in SetCount.
Ok, I see.
Of course, I can separate the patch that clears LBS_NODATA and place it before this patch, if you want.
Yes, please send them in that order, after v5 of tests is committed.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/comctl32/tests/listbox.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/dlls/comctl32/tests/listbox.c b/dlls/comctl32/tests/listbox.c index b4be05c..47498ca 100644 --- a/dlls/comctl32/tests/listbox.c +++ b/dlls/comctl32/tests/listbox.c @@ -1803,6 +1803,25 @@ static void test_set_count( void ) DestroyWindow( parent ); }
+static void test_nodata_invalid_styles( void ) +{ + static const DWORD style[] = + { + 0, LBS_OWNERDRAWVARIABLE, LBS_SORT, LBS_HASSTRINGS, + LBS_OWNERDRAWFIXED | LBS_SORT, LBS_OWNERDRAWFIXED | LBS_HASSTRINGS + }; + UINT i; + + for (i = 0; i < ARRAY_SIZE(style); i++) + { + HWND listbox = CreateWindowA(WC_LISTBOXA, "TestList", LBS_NODATA | style[i], + 0, 0, 100, 100, NULL, NULL, NULL, 0); + LRESULT ret = SendMessageA(listbox, LB_SETCOUNT, 100, 0); + ok(ret == LB_ERR, "LBS_NODATA enabled with incompatible styles 0x%X\n", style[i]); + DestroyWindow(listbox); + } +} + static void test_nodata( void ) { static const UINT invalid_idx[] = { -2, 2 }; @@ -1815,6 +1834,8 @@ static void test_nodata( void ) so we can't rely on sizeof(void*) and have to hardcode it */ UINT64 data;
+ test_nodata_invalid_styles(); + listbox = CreateWindowA(WC_LISTBOXA, "TestList", LBS_NODATA | LBS_OWNERDRAWFIXED | WS_VISIBLE, 0, 0, 100, 100, NULL, NULL, NULL, 0); ok(listbox != NULL, "last error 0x%08x\n", GetLastError());
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/comctl32/listbox.c | 3 +++ dlls/comctl32/tests/listbox.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index 0a7c341..5e33466 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -2523,6 +2523,9 @@ static BOOL LISTBOX_Create( HWND hwnd, LPHEADCOMBO lphc )
if (descr->style & LBS_OWNERDRAWFIXED) { + /* Windows accepts both, but FIXED overrides VARIABLE */ + descr->style &= ~LBS_OWNERDRAWVARIABLE; + if( descr->lphc && (descr->lphc->dwStyle & CBS_DROPDOWN)) { /* WinWord gets VERY unhappy if we send WM_MEASUREITEM from here */ diff --git a/dlls/comctl32/tests/listbox.c b/dlls/comctl32/tests/listbox.c index 47498ca..7429144 100644 --- a/dlls/comctl32/tests/listbox.c +++ b/dlls/comctl32/tests/listbox.c @@ -457,7 +457,7 @@ static void test_both_ownerdraw(void) ok(ret == LB_OKAY, "LB_SETITEMHEIGHT failed: %d\n", ret);
ret = SendMessageA(hLB, LB_GETITEMHEIGHT, 0, 0); - todo_wine ok(ret == 42, "LBS_OWNERDRAWFIXED did not override LBS_OWNERDRAWVARIABLE, got height %d\n", ret); + ok(ret == 42, "LBS_OWNERDRAWFIXED did not override LBS_OWNERDRAWVARIABLE, got height %d\n", ret);
DestroyWindow (hLB); }
With LBS_NODATA listboxes, LB_GETTEXT always retrieves the value zero, LB_GETITEMDATA returns zero, and LB_SETITEMDATA does nothing. However, all of them do check for valid indices and return LB_ERR if not valid.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/comctl32/listbox.c | 7 ++++--- dlls/comctl32/tests/listbox.c | 10 +++++----- 2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index 5e33466..ab01430 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -762,7 +762,8 @@ static LRESULT LISTBOX_GetText( LB_DESCR *descr, INT index, LPWSTR buffer, BOOL } else { if (buffer) - *((DWORD *)buffer) = *(DWORD *)&descr->items[index].data; + *((DWORD *)buffer) = (descr->style & LBS_NODATA) ? 0 : + *(DWORD *)&descr->items[index].data; len = sizeof(DWORD); } return len; @@ -2635,7 +2636,7 @@ static LRESULT CALLBACK LISTBOX_WindowProc( HWND hwnd, UINT msg, WPARAM wParam, SetLastError(ERROR_INVALID_INDEX); return LB_ERR; } - return descr->items[wParam].data; + return (descr->style & LBS_NODATA) ? 0 : descr->items[wParam].data;
case LB_SETITEMDATA: if (((INT)wParam < 0) || ((INT)wParam >= descr->nb_items)) @@ -2643,7 +2644,7 @@ static LRESULT CALLBACK LISTBOX_WindowProc( HWND hwnd, UINT msg, WPARAM wParam, SetLastError(ERROR_INVALID_INDEX); return LB_ERR; } - descr->items[wParam].data = lParam; + if (!(descr->style & LBS_NODATA)) descr->items[wParam].data = lParam; /* undocumented: returns TRUE, not LB_OKAY (0) */ return TRUE;
diff --git a/dlls/comctl32/tests/listbox.c b/dlls/comctl32/tests/listbox.c index 7429144..87b3ec0 100644 --- a/dlls/comctl32/tests/listbox.c +++ b/dlls/comctl32/tests/listbox.c @@ -1876,19 +1876,19 @@ static void test_nodata( void ) data = 0xdeadbeef; ret = SendMessageA(listbox, LB_GETTEXT, valid_idx[i], (LPARAM)&data); ok(ret <= sizeof(data), "got %d\n", ret); - todo_wine ok(data == 0, "LB_GETTEXT should retrieve 0 with LBS_NODATA, got 0x%llx\n", data); + ok(data == 0, "LB_GETTEXT should retrieve 0 with LBS_NODATA, got 0x%llx\n", data); } ret = SendMessageA(listbox, LB_GETITEMDATA, valid_idx[i], 0); - todo_wine ok(ret == 0, "LB_GETITEMDATA should return 0 with LBS_NODATA, got %d\n", ret); + ok(ret == 0, "LB_GETITEMDATA should return 0 with LBS_NODATA, got %d\n", ret); }
/* test more invalid messages with LBS_NODATA */ ret = SendMessageA(listbox, LB_FINDSTRING, 1, 42); - todo_wine ok(ret == LB_ERR, "got %d\n", ret); + ok(ret == LB_ERR, "got %d\n", ret); ret = SendMessageA(listbox, LB_FINDSTRINGEXACT, 1, 42); - todo_wine ok(ret == LB_ERR, "got %d\n", ret); + ok(ret == LB_ERR, "got %d\n", ret); ret = SendMessageA(listbox, LB_SELECTSTRING, 1, 42); - todo_wine ok(ret == LB_ERR, "got %d\n", ret); + ok(ret == LB_ERR, "got %d\n", ret);
DestroyWindow(listbox); }
This one is changing too much at once, really it should cover each message separately, or, if it's problematic, related messages in the same patch. Like GETTEXTLEN and GETTEXT.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/comctl32/listbox.c | 5 ++++- dlls/comctl32/tests/listbox.c | 6 ++++++ 2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index ab01430..7747c33 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -919,6 +919,9 @@ static INT LISTBOX_FindString( LB_DESCR *descr, INT start, LPCWSTR str, BOOL exa } else { + if (descr->style & LBS_NODATA) + return LB_ERR; + if (exact && (descr->style & LBS_SORT)) /* If sorted, use a WM_COMPAREITEM binary search */ return LISTBOX_FindStringPos( descr, str, TRUE ); @@ -2450,7 +2453,7 @@ static LRESULT LISTBOX_HandleChar( LB_DESCR *descr, WCHAR charW ) (LPARAM)descr->self ); if (caret == -2) return 0; } - if (caret == -1) + if (caret == -1 && !(descr->style & LBS_NODATA)) caret = LISTBOX_FindString( descr, descr->focus_item, str, FALSE); if (caret != -1) { diff --git a/dlls/comctl32/tests/listbox.c b/dlls/comctl32/tests/listbox.c index 87b3ec0..6a3c345 100644 --- a/dlls/comctl32/tests/listbox.c +++ b/dlls/comctl32/tests/listbox.c @@ -1885,10 +1885,16 @@ static void test_nodata( void ) /* test more invalid messages with LBS_NODATA */ ret = SendMessageA(listbox, LB_FINDSTRING, 1, 42); ok(ret == LB_ERR, "got %d\n", ret); + ret = SendMessageA(listbox, LB_FINDSTRING, 1, 0); + ok(ret == LB_ERR, "got %d\n", ret); ret = SendMessageA(listbox, LB_FINDSTRINGEXACT, 1, 42); ok(ret == LB_ERR, "got %d\n", ret); + ret = SendMessageA(listbox, LB_FINDSTRINGEXACT, 1, 0); + ok(ret == LB_ERR, "got %d\n", ret); ret = SendMessageA(listbox, LB_SELECTSTRING, 1, 42); ok(ret == LB_ERR, "got %d\n", ret); + ret = SendMessageA(listbox, LB_SELECTSTRING, 1, 0); + ok(ret == LB_ERR, "got %d\n", ret);
DestroyWindow(listbox); }
On 11/8/18 2:39 PM, Gabriel Ivăncescu wrote:
@@ -919,6 +919,9 @@ static INT LISTBOX_FindString( LB_DESCR *descr, INT start, LPCWSTR str, BOOL exa } else {
if (descr->style & LBS_NODATA)return LB_ERR;if (exact && (descr->style & LBS_SORT)) /* If sorted, use a WM_COMPAREITEM binary search */ return LISTBOX_FindStringPos( descr, str, TRUE );@@ -2450,7 +2453,7 @@ static LRESULT LISTBOX_HandleChar( LB_DESCR *descr, WCHAR charW ) (LPARAM)descr->self ); if (caret == -2) return 0; }
- if (caret == -1)
- if (caret == -1 && !(descr->style & LBS_NODATA)) caret = LISTBOX_FindString( descr, descr->focus_item, str, FALSE); if (caret != -1) {
Does it have to be deep in message handler? Can we skip right away if we have to?
Also same issue, these are easily separable changes.
On Sat, Nov 10, 2018 at 11:46 PM Nikolay Sivov nsivov@codeweavers.com wrote:
On 11/8/18 2:39 PM, Gabriel Ivăncescu wrote:
@@ -919,6 +919,9 @@ static INT LISTBOX_FindString( LB_DESCR *descr, INT start, LPCWSTR str, BOOL exa } else {
if (descr->style & LBS_NODATA)return LB_ERR;if (exact && (descr->style & LBS_SORT)) /* If sorted, use a WM_COMPAREITEM binary search */ return LISTBOX_FindStringPos( descr, str, TRUE );@@ -2450,7 +2453,7 @@ static LRESULT LISTBOX_HandleChar( LB_DESCR *descr, WCHAR charW ) (LPARAM)descr->self ); if (caret == -2) return 0; }
- if (caret == -1)
- if (caret == -1 && !(descr->style & LBS_NODATA)) caret = LISTBOX_FindString( descr, descr->focus_item, str, FALSE); if (caret != -1) {
Does it have to be deep in message handler? Can we skip right away if we have to?
No it doesn't have to be, it can be moved to the top of the function, I'll do that.
Also same issue, these are easily separable changes.
If you are referring to the caret == -1 change in HandleChar, then it's actually useless and can be skipped completely, since LB_ERR is -1 already.
I had it done probably because when I also made the user32 changes, it had to set last error (but *only* the user32 variant does that, when I tested), so it only makes sense there. I'll remove it for comctl32.
On 11/11/18 1:05 PM, Gabriel Ivăncescu wrote:
On Sat, Nov 10, 2018 at 11:46 PM Nikolay Sivov nsivov@codeweavers.com wrote:
On 11/8/18 2:39 PM, Gabriel Ivăncescu wrote:
@@ -919,6 +919,9 @@ static INT LISTBOX_FindString( LB_DESCR *descr, INT start, LPCWSTR str, BOOL exa } else {
if (descr->style & LBS_NODATA)return LB_ERR;if (exact && (descr->style & LBS_SORT)) /* If sorted, use a WM_COMPAREITEM binary search */ return LISTBOX_FindStringPos( descr, str, TRUE );@@ -2450,7 +2453,7 @@ static LRESULT LISTBOX_HandleChar( LB_DESCR *descr, WCHAR charW ) (LPARAM)descr->self ); if (caret == -2) return 0; }
- if (caret == -1)
- if (caret == -1 && !(descr->style & LBS_NODATA)) caret = LISTBOX_FindString( descr, descr->focus_item, str, FALSE); if (caret != -1) {
Does it have to be deep in message handler? Can we skip right away if we have to?
No it doesn't have to be, it can be moved to the top of the function, I'll do that.
Thanks.
Also same issue, these are easily separable changes.
If you are referring to the caret == -1 change in HandleChar, then it's actually useless and can be skipped completely, since LB_ERR is -1 already.
I had it done probably because when I also made the user32 changes, it had to set last error (but *only* the user32 variant does that, when I tested), so it only makes sense there. I'll remove it for comctl32.
Right, I included some last error tests in v5 patches. It's clearly different between user32 and newer variant, and I don't see any existing tests for that in user32.
On Sun, Nov 11, 2018 at 4:03 PM Nikolay Sivov nsivov@codeweavers.com wrote:
Right, I included some last error tests in v5 patches. It's clearly different between user32 and newer variant, and I don't see any existing tests for that in user32.
There aren't any yet, I didn't send them since I was focusing on sending the comctl32 patchset first :-)
(but I did write the user32 tests at the same time so that's where it was from)
-
Gabriel Ivăncescu -
Nikolay Sivov