Nikolay Sivov : comctl32/listview: Fix LVM_INSERTITEM handling on LVS_SORTxxx styles.

[Alexandre Julliard]

Module: wine
Branch: master
Commit: 2829c5fd1e4d36274031c5e78543b4ec741ba8b2
URL:    http://source.winehq.org/git/wine.git/?a=commit;h=2829c5fd1e4d36274031c5e785...

Author: Nikolay Sivov <bunglehead(a)gmail.com>
Date:   Sun May  3 03:10:43 2009 +0400

comctl32/listview: Fix LVM_INSERTITEM handling on LVS_SORTxxx styles.

---

 dlls/comctl32/listview.c       |   64 ++++++++++++++++-----------------------
 dlls/comctl32/tests/listview.c |   45 +++++++++++++++++++++++++--
 2 files changed, 67 insertions(+), 42 deletions(-)

diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c
index 14eaa23..75556e6 100644
--- a/dlls/comctl32/listview.c
+++ b/dlls/comctl32/listview.c
@@ -63,8 +63,9 @@
  *   -- LISTVIEW_GetNextItem needs to be rewritten. It is currently
  *      linear in the number of items in the list, and this is
  *      unacceptable for large lists.
- *   -- in sorted mode, LISTVIEW_InsertItemT sorts the array,
- *      instead of inserting in the right spot
+ *   -- if list is sorted by item text LISTVIEW_InsertItemT could use
+ *      binary search to calculate item index (e.g. DPA_Search()).
+ *      This requires sorted state to be reliably tracked in item modifiers.
  *   -- we should keep an ordered array of coordinates in iconic mode
  *      this would allow to frame items (iterator_frameditems),
  *      and find nearest item (LVFI_NEARESTXY) a lot more efficiently
@@ -6592,33 +6593,6 @@ static INT LISTVIEW_HitTest(const LISTVIEW_INFO *infoPtr, LPLVHITTESTINFO lpht,
     return lpht->iItem = iItem;
 }
 
-
-/* LISTVIEW_InsertCompare:  callback routine for comparing pszText members of the LV_ITEMS
-   in a LISTVIEW on insert.  Passed to DPA_Sort in LISTVIEW_InsertItem.
-   This function should only be used for inserting items into a sorted list (LVM_INSERTITEM)
-   and not during the processing of a LVM_SORTITEMS message. Applications should provide
-   their own sort proc. when sending LVM_SORTITEMS.
-*/
-/* Platform SDK:
-    (remarks on LVITEM: LVM_INSERTITEM will insert the new item in the proper sort postion...
-        if:
-          LVS_SORTXXX must be specified,
-          LVS_OWNERDRAW is not set,
-          <item>.pszText is not LPSTR_TEXTCALLBACK.
-
-    (LVS_SORT* flags): "For the LVS_SORTASCENDING... styles, item indices
-    are sorted based on item text..."
-*/
-static INT WINAPI LISTVIEW_InsertCompare(  LPVOID first, LPVOID second,  LPARAM lParam)
-{
-    ITEM_INFO* lv_first = DPA_GetPtr( first, 0 );
-    ITEM_INFO* lv_second = DPA_GetPtr( second, 0 );
-    INT cmpv = textcmpWT(lv_first->hdr.pszText, lv_second->hdr.pszText, TRUE); 
-
-    /* if we're sorting descending, negate the return value */
-    return (((const LISTVIEW_INFO *)lParam)->dwStyle & LVS_SORTDESCENDING) ? -cmpv : cmpv;
-}
-
 /***
  * DESCRIPTION:
  * Inserts a new item in the listview control.
@@ -6663,7 +6637,29 @@ static INT LISTVIEW_InsertItemT(LISTVIEW_INFO *infoPtr, const LVITEMW *lpLVItem,
 
     if (lpLVItem->iItem < 0 && !is_sorted) return -1;
 
-    nItem = is_sorted ? infoPtr->nItemCount : min(lpLVItem->iItem, infoPtr->nItemCount);
+    /* calculate new item index */
+    if (is_sorted)
+    {
+        HDPA hItem;
+        ITEM_INFO *item_s;
+        INT i = 0, cmpv;
+
+        while (i < infoPtr->nItemCount)
+        {
+            hItem  = DPA_GetPtr( infoPtr->hdpaItems, i);
+            item_s = (ITEM_INFO*)DPA_GetPtr(hItem, 0);
+
+            cmpv = textcmpWT(item_s->hdr.pszText, lpLVItem->pszText, TRUE);
+            if (infoPtr->dwStyle & LVS_SORTDESCENDING) cmpv *= -1;
+
+            if (cmpv >= 0) break;
+            i++;
+        }
+        nItem = i;
+    }
+    else
+        nItem = min(lpLVItem->iItem, infoPtr->nItemCount);
+
     TRACE(" inserting at %d, sorted=%d, count=%d, iItem=%d\n", nItem, is_sorted, infoPtr->nItemCount, lpLVItem->iItem);
     nItem = DPA_InsertPtr( infoPtr->hdpaItems, nItem, hdpaSubItems );
     if (nItem == -1) goto fail;
@@ -6698,14 +6694,6 @@ static INT LISTVIEW_InsertItemT(LISTVIEW_INFO *infoPtr, const LVITEMW *lpLVItem,
     }
     if (!set_main_item(infoPtr, &item, TRUE, isW, &has_changed)) goto undo;
 
-    /* if we're sorted, sort the list, and update the index */
-    if (is_sorted)
-    {
-	DPA_Sort( infoPtr->hdpaItems, LISTVIEW_InsertCompare, (LPARAM)infoPtr );
-	nItem = DPA_GetPtrIndex( infoPtr->hdpaItems, hdpaSubItems );
-	assert(nItem != -1);
-    }
-
     /* make room for the position, if we are in the right mode */
     if ((uView == LVS_SMALLICON) || (uView == LVS_ICON))
     {
diff --git a/dlls/comctl32/tests/listview.c b/dlls/comctl32/tests/listview.c
index 7be6b71..ee5eb7f 100644
--- a/dlls/comctl32/tests/listview.c
+++ b/dlls/comctl32/tests/listview.c
@@ -1629,7 +1629,7 @@ static void test_sorting(void)
     LVITEMA item = {0};
     DWORD r;
     LONG_PTR style;
-    static CHAR names[][4] = {"A", "B", "C", "D"};
+    static CHAR names[][5] = {"A", "B", "C", "D", "0"};
     CHAR buff[10];
 
     hwnd = create_listview_control(0);
@@ -1744,23 +1744,60 @@ static void test_sorting(void)
     item.cchTextMax = sizeof(buff);
     r = SendMessage(hwnd, LVM_GETITEM, 0, (LPARAM) &item);
     expect(TRUE, r);
-    todo_wine ok(lstrcmp(buff, names[1]) == 0, "Expected '%s', got '%s'\n", names[1], buff);
+    ok(lstrcmp(buff, names[1]) == 0, "Expected '%s', got '%s'\n", names[1], buff);
 
     item.iItem = 1;
     r = SendMessage(hwnd, LVM_GETITEM, 0, (LPARAM) &item);
     expect(TRUE, r);
-    todo_wine ok(lstrcmp(buff, names[2]) == 0, "Expected '%s', got '%s'\n", names[2], buff);
+    ok(lstrcmp(buff, names[2]) == 0, "Expected '%s', got '%s'\n", names[2], buff);
 
     item.iItem = 2;
     r = SendMessage(hwnd, LVM_GETITEM, 0, (LPARAM) &item);
     expect(TRUE, r);
-    todo_wine ok(lstrcmp(buff, names[0]) == 0, "Expected '%s', got '%s'\n", names[0], buff);
+    ok(lstrcmp(buff, names[0]) == 0, "Expected '%s', got '%s'\n", names[0], buff);
 
     item.iItem = 3;
     r = SendMessage(hwnd, LVM_GETITEM, 0, (LPARAM) &item);
     expect(TRUE, r);
     ok(lstrcmp(buff, names[3]) == 0, "Expected '%s', got '%s'\n", names[3], buff);
 
+    /* corner case - item should be placed at first position */
+    item.mask = LVIF_TEXT;
+    item.iItem = 4;
+    item.iSubItem = 0;
+    item.pszText = names[4];
+    r = SendMessage(hwnd, LVM_INSERTITEM, 0, (LPARAM) &item);
+    expect(0, r);
+
+    item.iItem = 0;
+    item.pszText = buff;
+    item.cchTextMax = sizeof(buff);
+    r = SendMessage(hwnd, LVM_GETITEM, 0, (LPARAM) &item);
+    expect(TRUE, r);
+    ok(lstrcmp(buff, names[4]) == 0, "Expected '%s', got '%s'\n", names[4], buff);
+
+    item.iItem = 1;
+    item.pszText = buff;
+    item.cchTextMax = sizeof(buff);
+    r = SendMessage(hwnd, LVM_GETITEM, 0, (LPARAM) &item);
+    expect(TRUE, r);
+    ok(lstrcmp(buff, names[1]) == 0, "Expected '%s', got '%s'\n", names[1], buff);
+
+    item.iItem = 2;
+    r = SendMessage(hwnd, LVM_GETITEM, 0, (LPARAM) &item);
+    expect(TRUE, r);
+    ok(lstrcmp(buff, names[2]) == 0, "Expected '%s', got '%s'\n", names[2], buff);
+
+    item.iItem = 3;
+    r = SendMessage(hwnd, LVM_GETITEM, 0, (LPARAM) &item);
+    expect(TRUE, r);
+    ok(lstrcmp(buff, names[0]) == 0, "Expected '%s', got '%s'\n", names[0], buff);
+
+    item.iItem = 4;
+    r = SendMessage(hwnd, LVM_GETITEM, 0, (LPARAM) &item);
+    expect(TRUE, r);
+    ok(lstrcmp(buff, names[3]) == 0, "Expected '%s', got '%s'\n", names[3], buff);
+
     DestroyWindow(hwnd);
 }