[PATCH 1/9] user32/listbox: Resize the entire item array at once in SetCount

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---

user32 versions of the comctl32 series.

 dlls/user32/listbox.c | 43 ++++++++++++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/dlls/user32/listbox.c b/dlls/user32/listbox.c
index a2791eb..a9195dc 100644
--- a/dlls/user32/listbox.c
+++ b/dlls/user32/listbox.c
@@ -139,6 +139,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;
 }
 
@@ -1751,9 +1757,9 @@ 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))
     {
@@ -1761,19 +1767,30 @@ static LRESULT LISTBOX_SetCount( LB_DESCR *descr, INT count )
         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;
-- 
2.20.1

[PATCH 2/9] user32/listbox: Repaint the old selected item after setting the item selection

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/user32/listbox.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/user32/listbox.c b/dlls/user32/listbox.c
index a9195dc..ac301c1 100644
--- a/dlls/user32/listbox.c
+++ b/dlls/user32/listbox.c
@@ -1475,8 +1475,8 @@ static LRESULT LISTBOX_SetSelection( LB_DESCR *descr, INT index,
         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 );
         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 );
-- 
2.20.1

[PATCH 2/9] user32/listbox: Repaint the old selected item after setting the item selection

[Marvin]

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=47646

Your paranoid android.


=== debian9 (32 bit WoW report) ===

user32:
msg.c:8713: Test failed: WaitForSingleObject failed 102
msg.c:8719: Test failed: destroy child on thread exit: 0: the msg 0x0082 was expected, but got msg 0x000f instead
msg.c:8719: Test failed: destroy child on thread exit: 1: the msg 0x000f was expected, but got msg 0x0014 instead
msg.c:8719: Test failed: destroy child on thread exit: 2: the msg sequence is not complete: expected 0014 - actual 0000

=== debian9 (64 bit WoW report) ===

user32:
msg.c:8713: Test failed: WaitForSingleObject failed 102
msg.c:8719: Test failed: destroy child on thread exit: 0: the msg 0x0082 was expected, but got msg 0x000f instead
msg.c:8719: Test failed: destroy child on thread exit: 1: the msg 0x000f was expected, but got msg 0x0014 instead
msg.c:8719: Test failed: destroy child on thread exit: 2: the msg sequence is not complete: expected 0014 - actual 0000

[PATCH 2/9] user32/listbox: Repaint the old selected item after setting the item selection

[Huw Davies]

Signed-off-by: Huw Davies <huw(a)codeweavers.com>

[PATCH 3/9] user32/listbox: Use descr->selected_item for single-selection listboxes when checking selection

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/user32/listbox.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/dlls/user32/listbox.c b/dlls/user32/listbox.c
index ac301c1..0e0399f 100644
--- a/dlls/user32/listbox.c
+++ b/dlls/user32/listbox.c
@@ -150,6 +150,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;
 }
 
-- 
2.20.1

[PATCH 3/9] user32/listbox: Use descr->selected_item for single-selection listboxes when checking selection

[Huw Davies]

Signed-off-by: Huw Davies <huw(a)codeweavers.com>

[PATCH 4/9] user32/tests: Test changing the selection styles after listbox creation

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/user32/tests/listbox.c | 130 ++++++++++++++++++++++++++++++++++++
 1 file changed, 130 insertions(+)

diff --git a/dlls/user32/tests/listbox.c b/dlls/user32/tests/listbox.c
index 7f1e9cf..5bdffc5 100644
--- a/dlls/user32/tests/listbox.c
+++ b/dlls/user32/tests/listbox.c
@@ -665,6 +665,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("listbox", "TestList", styles[i] | selstyles[j] | WS_CHILD | WS_VISIBLE,
+                                    0, 0, 100, 100, parent, (HMENU)1, 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 (
@@ -2309,6 +2438,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.20.1

[PATCH 4/9] user32/tests: Test changing the selection styles after listbox creation

[Marvin]

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=47648

Your paranoid android.


=== debian9 (32 bit Chinese:China report) ===

user32:
input.c:2573: Test failed: 200: wrong deviceType 2/0
input.c:2576: Test failed: 200: wrong originId 1/4
input.c:2582: Test failed: 200: wrong deviceType 2/0
input.c:2585: Test failed: 200: wrong originId 1/4

[PATCH 4/9] user32/tests: Test changing the selection styles after listbox creation

[Huw Davies]

Signed-off-by: Huw Davies <huw(a)codeweavers.com>

[PATCH 5/9] user32/listbox: Use a helper to retrieve item data by index

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/user32/listbox.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/dlls/user32/listbox.c b/dlls/user32/listbox.c
index 0e0399f..70b55e1 100644
--- a/dlls/user32/listbox.c
+++ b/dlls/user32/listbox.c
@@ -148,6 +148,11 @@ static BOOL resize_storage(LB_DESCR *descr, UINT items_size)
     return TRUE;
 }
 
+static ULONG_PTR get_item_data( const LB_DESCR *descr, UINT index )
+{
+    return (descr->style & LBS_NODATA) ? 0 : descr->items[index].data;
+}
+
 static BOOL is_item_selected( const LB_DESCR *descr, UINT index )
 {
     if (!(descr->style & (LBS_MULTIPLESEL | LBS_EXTENDEDSEL)))
@@ -590,7 +595,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,
@@ -825,8 +830,7 @@ static LRESULT LISTBOX_GetText( LB_DESCR *descr, INT index, LPWSTR buffer, BOOL
         __ENDTRY
     } 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;
@@ -874,7 +878,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;
@@ -1650,7 +1654,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)
@@ -2701,7 +2705,7 @@ LRESULT ListBoxWndProc_common( HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam
             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))
-- 
2.20.1

[PATCH 5/9] user32/listbox: Use a helper to retrieve item data by index

[Huw Davies]

Signed-off-by: Huw Davies <huw(a)codeweavers.com>

[PATCH 6/9] user32/listbox: Use a helper to retrieve item string by index

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/user32/listbox.c | 51 ++++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 23 deletions(-)

diff --git a/dlls/user32/listbox.c b/dlls/user32/listbox.c
index 70b55e1..7646486 100644
--- a/dlls/user32/listbox.c
+++ b/dlls/user32/listbox.c
@@ -153,6 +153,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 HAS_STRINGS(descr) ? descr->items[index].str : NULL;
+}
+
 static BOOL is_item_selected( const LB_DESCR *descr, UINT index )
 {
     if (!(descr->style & (LBS_MULTIPLESEL | LBS_EXTENDEDSEL)))
@@ -551,11 +556,11 @@ static void LISTBOX_PaintItem( LB_DESCR *descr, HDC hdc, const RECT *rect,
 			       INT index, UINT action, BOOL ignoreFocus )
 {
     BOOL selected = FALSE, focused;
-    LB_ITEMDATA *item = NULL;
+    WCHAR *item_str = NULL;
 
     if (index < descr->nb_items)
     {
-        item = &descr->items[index];
+        item_str = get_item_string(descr, index);
         selected = is_item_selected(descr, index);
     }
 
@@ -598,7 +603,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(item_str), action,
               dis.itemState, wine_dbgstr_rect(rect) );
         SendMessageW(descr->owner, WM_DRAWITEM, dis.CtlID, (LPARAM)&dis);
         SelectClipRgn( hdc, hrgn );
@@ -620,22 +625,22 @@ static void LISTBOX_PaintItem( LB_DESCR *descr, HDC hdc, const RECT *rect,
         }
 
         TRACE("[%p]: painting %d (%s) action=%02x rect=%s\n",
-              descr->self, index, item ? debugstr_w(item->str) : "", action,
+              descr->self, index, debugstr_w(item_str), action,
               wine_dbgstr_rect(rect) );
-        if (!item)
+        if (!item_str)
             ExtTextOutW( hdc, rect->left + 1, rect->top,
                            ETO_OPAQUE | ETO_CLIPPED, rect, NULL, 0, NULL );
         else if (!(descr->style & LBS_USETABSTOPS))
             ExtTextOutW( hdc, rect->left + 1, rect->top,
-                         ETO_OPAQUE | ETO_CLIPPED, rect, item->str,
-                         strlenW(item->str), NULL );
+                         ETO_OPAQUE | ETO_CLIPPED, rect, item_str,
+                         strlenW(item_str), NULL );
         else
 	{
 	    /* Output empty string to paint background in the full width. */
             ExtTextOutW( hdc, rect->left + 1, rect->top,
                          ETO_OPAQUE | ETO_CLIPPED, rect, NULL, 0, NULL );
             TabbedTextOutW( hdc, rect->left + 1 , rect->top,
-                            item->str, strlenW(item->str),
+                            item_str, strlenW(item_str),
                             descr->nb_tabs, descr->tabs, 0);
 	}
         if (selected)
@@ -797,28 +802,29 @@ static LRESULT LISTBOX_GetText( LB_DESCR *descr, INT index, LPWSTR buffer, BOOL
     }
     if (HAS_STRINGS(descr))
     {
+        WCHAR *str = get_item_string(descr, index);
+
         if (!buffer)
         {
-            len = strlenW(descr->items[index].str);
+            len = strlenW(str);
             if( unicode )
                 return len;
-            return WideCharToMultiByte( CP_ACP, 0, descr->items[index].str, len,
-                                        NULL, 0, NULL, NULL );
+            return WideCharToMultiByte( CP_ACP, 0, str, len, NULL, 0, NULL, NULL );
         }
 
-	TRACE("index %d (0x%04x) %s\n", index, index, debugstr_w(descr->items[index].str));
+	TRACE("index %d (0x%04x) %s\n", index, index, debugstr_w(str));
 
         __TRY  /* hide a Delphi bug that passes a read-only buffer */
         {
             if(unicode)
             {
-                strcpyW( buffer, descr->items[index].str );
+                strcpyW(buffer, str);
                 len = strlenW(buffer);
             }
             else
             {
-                len = WideCharToMultiByte(CP_ACP, 0, descr->items[index].str, -1,
-                                          (LPSTR)buffer, 0x7FFFFFFF, NULL, NULL) - 1;
+                len = WideCharToMultiByte(CP_ACP, 0, str, -1, (LPSTR)buffer,
+                                          0x7FFFFFFF, NULL, NULL) - 1;
             }
         }
         __EXCEPT_PAGE_FAULT
@@ -866,7 +872,7 @@ static INT LISTBOX_FindStringPos( LB_DESCR *descr, LPCWSTR str, BOOL exact )
     {
         index = (min + max) / 2;
         if (HAS_STRINGS(descr))
-            res = LISTBOX_lstrcmpiW( descr->locale, descr->items[index].str, str );
+            res = LISTBOX_lstrcmpiW( descr->locale, get_item_string(descr, index), str );
         else
         {
             COMPAREITEMSTRUCT cis;
@@ -909,7 +915,7 @@ static INT LISTBOX_FindFileStrPos( LB_DESCR *descr, LPCWSTR str )
     while (min != max)
     {
         INT index = (min + max) / 2;
-        LPCWSTR p = descr->items[index].str;
+        LPCWSTR p = get_item_string(descr, index);
         if (*p == '[')  /* drive or directory */
         {
             if (*str != '[') res = -1;
@@ -1655,7 +1661,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 );
@@ -1677,8 +1683,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 );
 }
 
 
@@ -2730,9 +2735,9 @@ LRESULT ListBoxWndProc_common( HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam
             return LB_ERR;
         }
         if (!HAS_STRINGS(descr)) return sizeof(ULONG_PTR);
-        if (unicode) return strlenW( descr->items[wParam].str );
-        return WideCharToMultiByte( CP_ACP, 0, descr->items[wParam].str,
-                                    strlenW(descr->items[wParam].str), NULL, 0, NULL, NULL );
+        if (unicode) return strlenW(get_item_string(descr, wParam));
+        return WideCharToMultiByte( CP_ACP, 0, get_item_string(descr, wParam),
+                                    strlenW(get_item_string(descr, wParam)), NULL, 0, NULL, NULL );
 
     case LB_GETCURSEL:
         if (descr->nb_items == 0)
-- 
2.20.1

[PATCH 6/9] user32/listbox: Use a helper to retrieve item string by index

[Marvin]

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=47650

Your paranoid android.


=== debian9 (32 bit report) ===

user32:
msg.c:8713: Test failed: WaitForSingleObject failed 102
msg.c:8719: Test failed: destroy child on thread exit: 0: the msg 0x0082 was expected, but got msg 0x000f instead
msg.c:8719: Test failed: destroy child on thread exit: 1: the msg 0x000f was expected, but got msg 0x0014 instead
msg.c:8719: Test failed: destroy child on thread exit: 2: the msg sequence is not complete: expected 0014 - actual 0000

[PATCH 6/9] user32/listbox: Use a helper to retrieve item string by index

[Huw Davies]

Signed-off-by: Huw Davies <huw(a)codeweavers.com>

[PATCH 7/9] user32/listbox: Use is_item_selected in more places

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/user32/listbox.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/dlls/user32/listbox.c b/dlls/user32/listbox.c
index 7646486..f2d12ed 100644
--- a/dlls/user32/listbox.c
+++ b/dlls/user32/listbox.c
@@ -1442,7 +1442,7 @@ static LRESULT LISTBOX_SelectItemRange( LB_DESCR *descr, INT first,
     {
         for (i = first; i <= last; i++)
         {
-            if (descr->items[i].selected) continue;
+            if (is_item_selected(descr, i)) continue;
             descr->items[i].selected = TRUE;
             LISTBOX_InvalidateItemRect(descr, i);
         }
@@ -1451,7 +1451,7 @@ static LRESULT LISTBOX_SelectItemRange( LB_DESCR *descr, INT first,
     {
         for (i = first; i <= last; i++)
         {
-            if (!descr->items[i].selected) continue;
+            if (!is_item_selected(descr, i)) continue;
             descr->items[i].selected = FALSE;
             LISTBOX_InvalidateItemRect(descr, i);
         }
@@ -2091,7 +2091,7 @@ static LRESULT LISTBOX_HandleLButtonDown( LB_DESCR *descr, DWORD keys, INT x, IN
         {
             LISTBOX_SetCaretIndex( descr, index, FALSE );
             LISTBOX_SetSelection( descr, index,
-                                  !descr->items[index].selected,
+                                  !is_item_selected(descr, index),
                                   (descr->style & LBS_NOTIFY) != 0);
         }
         else
@@ -2101,13 +2101,13 @@ static LRESULT LISTBOX_HandleLButtonDown( LB_DESCR *descr, DWORD keys, INT x, IN
             if (descr->style & LBS_EXTENDEDSEL)
             {
                 LISTBOX_SetSelection( descr, index,
-                               descr->items[index].selected,
+                               is_item_selected(descr, index),
                               (descr->style & LBS_NOTIFY) != 0 );
             }
             else
             {
                 LISTBOX_SetSelection( descr, index,
-                               !descr->items[index].selected,
+                               !is_item_selected(descr, index),
                               (descr->style & LBS_NOTIFY) != 0 );
             }
         }
@@ -2427,7 +2427,7 @@ static LRESULT LISTBOX_HandleKeyDown( LB_DESCR *descr, DWORD key )
         else if (descr->style & LBS_MULTIPLESEL)
         {
             LISTBOX_SetSelection( descr, descr->focus_item,
-                                  !descr->items[descr->focus_item].selected,
+                                  !is_item_selected(descr, descr->focus_item),
                                   (descr->style & LBS_NOTIFY) != 0 );
         }
         break;
-- 
2.20.1

[PATCH 7/9] user32/listbox: Use is_item_selected in more places

[Huw Davies]

Signed-off-by: Huw Davies <huw(a)codeweavers.com>

[PATCH 8/9] user32/listbox: Use a helper to set the selected item state

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/user32/listbox.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/dlls/user32/listbox.c b/dlls/user32/listbox.c
index f2d12ed..7f9076f 100644
--- a/dlls/user32/listbox.c
+++ b/dlls/user32/listbox.c
@@ -165,6 +165,12 @@ 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 (descr->style & (LBS_MULTIPLESEL | LBS_EXTENDEDSEL))
+        descr->items[index].selected = state;
+}
+
 /*********************************************************************
  * listbox class descriptor
  */
@@ -1443,7 +1449,7 @@ static LRESULT LISTBOX_SelectItemRange( LB_DESCR *descr, INT first,
         for (i = first; i <= last; i++)
         {
             if (is_item_selected(descr, i)) continue;
-            descr->items[i].selected = TRUE;
+            set_item_selected_state(descr, i, TRUE);
             LISTBOX_InvalidateItemRect(descr, i);
         }
     }
@@ -1452,7 +1458,7 @@ static LRESULT LISTBOX_SelectItemRange( LB_DESCR *descr, INT first,
         for (i = first; i <= last; i++)
         {
             if (!is_item_selected(descr, i)) continue;
-            descr->items[i].selected = FALSE;
+            set_item_selected_state(descr, i, FALSE);
             LISTBOX_InvalidateItemRect(descr, i);
         }
     }
@@ -1485,8 +1491,8 @@ 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) 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 );
-- 
2.20.1

[PATCH 8/9] user32/listbox: Use a helper to set the selected item state

[Marvin]

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=47652

Your paranoid android.


=== debian9 (32 bit WoW report) ===

user32:
menu.c:2354: Test failed: test 27
menu: Timeout

[PATCH 8/9] user32/listbox: Use a helper to set the selected item state

[Huw Davies]

Signed-off-by: Huw Davies <huw(a)codeweavers.com>

[PATCH 9/9] user32/listbox: Send LB_RESETCONTENT from RemoveItem rather than DeleteItem

[Gabriel Ivăncescu]

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com>
---
 dlls/user32/listbox.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/dlls/user32/listbox.c b/dlls/user32/listbox.c
index 7f9076f..2e0f17b 100644
--- a/dlls/user32/listbox.c
+++ b/dlls/user32/listbox.c
@@ -1665,19 +1665,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 );
@@ -1686,10 +1679,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) );
 }
 
 
@@ -1707,11 +1700,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];
-- 
2.20.1

[PATCH 9/9] user32/listbox: Send LB_RESETCONTENT from RemoveItem rather than DeleteItem

[Huw Davies]

Signed-off-by: Huw Davies <huw(a)codeweavers.com>

[Marvin]

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=47645

Your paranoid android.


=== debian9 (32 bit WoW report) ===

user32:
input.c:2576: Test failed: 200: wrong originId 1/2
input.c:2585: Test failed: 200: wrong originId 1/2
input.c:2573: Test failed: 200: wrong deviceType 2/0
input.c:2576: Test failed: 200: wrong originId 1/4
input.c:2582: Test failed: 200: wrong deviceType 2/0
input.c:2585: Test failed: 200: wrong originId 1/4
input.c:2573: Test failed: 200: wrong deviceType 2/0
input.c:2576: Test failed: 200: wrong originId 1/4
input.c:2582: Test failed: 200: wrong deviceType 2/0
input.c:2585: Test failed: 200: wrong originId 1/4

[Huw Davies]

Signed-off-by: Huw Davies <huw(a)codeweavers.com>