comctl32/listview: Fix selection handling in LVM_SORTITEMS (with some tests).

dlls/comctl32/listview.c

@@ -57,7 +57,6 @@
  *   -- LISTVIEW_[GS]etColumnOrderArray stubs
  *   -- LISTVIEW_SetColumnWidth ignores header images & bitmap
  *   -- LISTVIEW_SetIconSpacing is incomplete
- *   -- LISTVIEW_SortItems is broken
  *   -- LISTVIEW_StyleChanged doesn't handle some changes too well
  *
  * Speedups
@@ -3575,21 +3574,38 @@ static BOOL set_main_item(LISTVIEW_INFO *infoPtr, const LVITEMW *lpLVItem, BOOL
 	{
 	    if (infoPtr->dwStyle & LVS_SINGLESEL) LISTVIEW_DeselectAllSkipItem(infoPtr, lpLVItem->iItem);
 	    ranges_additem(infoPtr->selectionRanges, lpLVItem->iItem);
+	    lpItem->state |= LVIS_SELECTED;
 	}
 	else if (stateMask & LVIS_SELECTED)
+	{
 	    ranges_delitem(infoPtr->selectionRanges, lpLVItem->iItem);
-	
+	    lpItem->state &= ~LVIS_SELECTED;
+	}
 	/* if we are asked to change focus, and we manage it, do it */
 	if (stateMask & ~infoPtr->uCallbackMask & LVIS_FOCUSED)
 	{
 	    if (lpLVItem->state & LVIS_FOCUSED)
 	    {
-		LISTVIEW_SetItemFocus(infoPtr, -1);
+		if (infoPtr->nFocusedItem != -1)
+		{
+		    /* remove current focus */
+		    item.mask  = LVIF_STATE;
+		    item.state = 0;
+		    item.stateMask = LVIS_FOCUSED;
+
+		    /* recurse with redrawing an item */
+		    LISTVIEW_SetItemState(infoPtr, infoPtr->nFocusedItem, &item);
+		}
+
+		lpItem->state |= LVIS_FOCUSED;
 		infoPtr->nFocusedItem = lpLVItem->iItem;
     	        LISTVIEW_EnsureVisible(infoPtr, lpLVItem->iItem, uView == LVS_LIST);
 	    }
 	    else if (infoPtr->nFocusedItem == lpLVItem->iItem)
-		infoPtr->nFocusedItem = -1;
+	    {
+	        lpItem->state &= ~LVIS_FOCUSED;
+	        infoPtr->nFocusedItem = -1;
+	    }
 	}
     }
 
@@ -7487,7 +7503,7 @@ static BOOL LISTVIEW_SetItemCount(LISTVIEW_INFO *infoPtr, INT nItems, DWORD dwFl
 	    ranges_del(infoPtr->selectionRanges, range);
 	    if (infoPtr->nFocusedItem >= nItems)
 	    {
-		infoPtr->nFocusedItem = -1;
+		LISTVIEW_SetItemFocus(infoPtr, -1);
 		SetRectEmpty(&infoPtr->rcFocus);
 	    }
 	}
@@ -7855,8 +7871,8 @@ static BOOL LISTVIEW_SortItems(LISTVIEW_INFO *infoPtr, PFNLVCOMPARE pfnCompare,
     UINT uView = infoPtr->dwStyle & LVS_TYPEMASK;
     HDPA hdpaSubItems;
     ITEM_INFO *lpItem;
-    LPVOID selectionMarkItem;
+    LPVOID selectionMarkItem = NULL;
-    LVITEMW item;
+    LPVOID focusedItem = NULL;
     int i;
 
     TRACE("(pfnCompare=%p, lParamSort=%lx)\n", pfnCompare, lParamSort);
@@ -7869,45 +7885,32 @@ static BOOL LISTVIEW_SortItems(LISTVIEW_INFO *infoPtr, PFNLVCOMPARE pfnCompare,
     /* if there are 0 or 1 items, there is no need to sort */
     if (infoPtr->nItemCount < 2) return TRUE;
 
+    /* clear selection */
+    ranges_clear(infoPtr->selectionRanges);
+
+    /* save selection mark and focused item */
+    if (infoPtr->nSelectionMark >= 0)
+        selectionMarkItem = DPA_GetPtr(infoPtr->hdpaItems, infoPtr->nSelectionMark);
     if (infoPtr->nFocusedItem >= 0)
-    {
+        focusedItem = DPA_GetPtr(infoPtr->hdpaItems, infoPtr->nFocusedItem);
-        hdpaSubItems = DPA_GetPtr(infoPtr->hdpaItems, infoPtr->nFocusedItem);
+
-        lpItem = DPA_GetPtr(hdpaSubItems, 0);
-	if (lpItem) lpItem->state |= LVIS_FOCUSED;
-    }
-    /* FIXME: go thorugh selected items and mark them so in lpItem->state */
-    /*        clear the lpItem->state for non-selected ones */
-    /*        remove the selection ranges */
-    
     infoPtr->pfnCompare = pfnCompare;
     infoPtr->lParamSort = lParamSort;
     DPA_Sort(infoPtr->hdpaItems, LISTVIEW_CallBackCompare, (LPARAM)infoPtr);
 
-    /* Adjust selections and indices so that they are the way they should
+    /* restore selection ranges */
-     * be after the sort (otherwise, the list items move around, but
-     * whatever is at the item's previous original position will be
-     * selected instead)
-     */
-    selectionMarkItem=(infoPtr->nSelectionMark>=0)?DPA_GetPtr(infoPtr->hdpaItems, infoPtr->nSelectionMark):NULL;
     for (i=0; i < infoPtr->nItemCount; i++)
     {
         hdpaSubItems = DPA_GetPtr(infoPtr->hdpaItems, i);
         lpItem = DPA_GetPtr(hdpaSubItems, 0);
 
 	if (lpItem->state & LVIS_SELECTED)
-	{
+	    ranges_additem(infoPtr->selectionRanges, i);
-	    item.state = LVIS_SELECTED;
-	    item.stateMask = LVIS_SELECTED;
-	    LISTVIEW_SetItemState(infoPtr, i, &item);
-	}
-	if (lpItem->state & LVIS_FOCUSED)
-	{
-            infoPtr->nFocusedItem = i;
-	    lpItem->state &= ~LVIS_FOCUSED;
-	}
     }
-    if (selectionMarkItem != NULL)
+    /* restore selection mark and focused item */
-	infoPtr->nSelectionMark = DPA_GetPtrIndex(infoPtr->hdpaItems, selectionMarkItem);
+    infoPtr->nSelectionMark = DPA_GetPtrIndex(infoPtr->hdpaItems, selectionMarkItem);
+    infoPtr->nFocusedItem   = DPA_GetPtrIndex(infoPtr->hdpaItems, focusedItem);
+
     /* I believe nHotItem should be left alone, see LISTVIEW_ShiftIndices */
 
     /* refresh the display */

dlls/comctl32/tests/listview.c

@@ -1441,6 +1441,69 @@ todo_wine{
     DestroyWindow(hwnd);
 }
 
+/* comparison callback for test_sorting */
+static INT WINAPI test_CallBackCompare(LPARAM first, LPARAM second, LPARAM lParam)
+{
+    if (first == second) return 0;
+    return (first > second ? 1 : -1);
+}
+
+static void test_sorting(void)
+{
+    HWND hwnd;
+    LVITEMA item = {0};
+    DWORD r;
+
+    hwnd = create_listview_control();
+    ok(hwnd != NULL, "failed to create a listview window\n");
+
+    /* insert some items */
+    item.mask = LVIF_PARAM | LVIF_STATE;
+    item.state = LVIS_SELECTED;
+    item.iItem = 0;
+    item.iSubItem = 0;
+    item.lParam = 3;
+    r = SendMessage(hwnd, LVM_INSERTITEM, 0, (LPARAM) &item);
+    expect(0, r);
+
+    item.mask = LVIF_PARAM;
+    item.iItem = 1;
+    item.iSubItem = 0;
+    item.lParam = 2;
+    r = SendMessage(hwnd, LVM_INSERTITEM, 0, (LPARAM) &item);
+    expect(1, r);
+
+    item.mask = LVIF_STATE | LVIF_PARAM;
+    item.state = LVIS_SELECTED;
+    item.iItem = 2;
+    item.iSubItem = 0;
+    item.lParam = 4;
+    r = SendMessage(hwnd, LVM_INSERTITEM, 0, (LPARAM) &item);
+    expect(2, r);
+
+    r = SendMessage(hwnd, LVM_GETSELECTIONMARK, 0, 0);
+    expect(-1, r);
+
+    r = SendMessage(hwnd, LVM_GETSELECTEDCOUNT, 0, 0);
+    expect(2, r);
+
+    r = SendMessage(hwnd, LVM_SORTITEMS, 0, (LPARAM)test_CallBackCompare);
+    expect(TRUE, r);
+
+    r = SendMessage(hwnd, LVM_GETSELECTEDCOUNT, 0, 0);
+    expect(2, r);
+    r = SendMessage(hwnd, LVM_GETSELECTIONMARK, 0, 0);
+    expect(-1, r);
+    r = SendMessage(hwnd, LVM_GETITEMSTATE, 0, LVIS_SELECTED);
+    expect(0, r);
+    r = SendMessage(hwnd, LVM_GETITEMSTATE, 1, LVIS_SELECTED);
+    expect(LVIS_SELECTED, r);
+    r = SendMessage(hwnd, LVM_GETITEMSTATE, 2, LVIS_SELECTED);
+    expect(LVIS_SELECTED, r);
+
+    DestroyWindow(hwnd);
+}
+
 START_TEST(listview)
 {
     HMODULE hComctl32;
@@ -1479,4 +1542,5 @@ START_TEST(listview)
     test_getorigin();
     test_multiselect();
     test_subitem_rect();
+    test_sorting();
 }