[PATCH 0/2] MR8165: comctl32/listview: Fix lbutton item selection with Shift+Ctrl.

[Nikolay Sivov (＠nsivov)]

Visual testing shows that using Shift+Ctrl adds to the selection,
which is what existing code already does. The problem is that it
updates selection mark when it shouldn't.

And the bigger problem of course is that it's using static variable
for state.

Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com>

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8165

[PATCH 1/2] comctl32/listview: Fix lbutton item selection with Shift+Ctrl.

[Nikolay Sivov]

From: Nikolay Sivov <nsivov(a)codeweavers.com>

Visual testing shows that using Shift+Ctrl adds to the selection,
which is what existing code already does. The problem is that it
updates selection mark when it shouldn't.

And the bigger problem of course is that it's using static variable
for state.

Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com>
---
 dlls/comctl32/listview.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c
index 83317ffe7eb..cf6445021e1 100644
--- a/dlls/comctl32/listview.c
+++ b/dlls/comctl32/listview.c
@@ -10345,7 +10345,6 @@ static LRESULT LISTVIEW_TrackMouse(const LISTVIEW_INFO *infoPtr, POINT pt)
 static LRESULT LISTVIEW_LButtonDown(LISTVIEW_INFO *infoPtr, WORD wKey, INT x, INT y)
 {
   LVHITTESTINFO lvHitTestInfo;
-  static BOOL bGroupSelect = TRUE;
   POINT pt = { x, y };
   INT nItem;
 
@@ -10387,30 +10386,17 @@ static LRESULT LISTVIEW_LButtonDown(LISTVIEW_INFO *infoPtr, WORD wKey, INT x, IN
     {
       if ((wKey & MK_CONTROL) && (wKey & MK_SHIFT))
       {
-        if (bGroupSelect)
-	{
-          if (!LISTVIEW_AddGroupSelection(infoPtr, nItem)) return 0;
-    	  LISTVIEW_SetItemFocus(infoPtr, nItem);
-          infoPtr->nSelectionMark = nItem;
-	}
-        else
-	{
-          LVITEMW item;
-
-	  item.state = LVIS_SELECTED | LVIS_FOCUSED;
-	  item.stateMask = LVIS_SELECTED | LVIS_FOCUSED;
-
-	  LISTVIEW_SetItemState(infoPtr,nItem,&item);
-	  infoPtr->nSelectionMark = nItem;
-	}
+        if (!LISTVIEW_AddGroupSelection(infoPtr, nItem)) return 0;
+        LISTVIEW_SetItemFocus(infoPtr, nItem);
       }
       else if (wKey & MK_CONTROL)
       {
         LVITEMW item;
+        BOOL select;
 
-	bGroupSelect = (LISTVIEW_GetItemState(infoPtr, nItem, LVIS_SELECTED) == 0);
+        select = (LISTVIEW_GetItemState(infoPtr, nItem, LVIS_SELECTED) == 0);
 	
-	item.state = (bGroupSelect ? LVIS_SELECTED : 0) | LVIS_FOCUSED;
+	item.state = (select ? LVIS_SELECTED : 0) | LVIS_FOCUSED;
         item.stateMask = LVIS_SELECTED | LVIS_FOCUSED;
 	LISTVIEW_SetItemState(infoPtr, nItem, &item);
         infoPtr->nSelectionMark = nItem;
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/8165

[PATCH 2/2] comctl32/listview: Do not consider key state when navigating with alphanumeric keys.

[Nikolay Sivov]

From: Nikolay Sivov <nsivov(a)codeweavers.com>

For example holding down Shift and using letter keys shouldn't produce
a selection range.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58275
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com>
---
 dlls/comctl32/listview.c | 47 ++++++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c
index cf6445021e1..31f8a933e56 100644
--- a/dlls/comctl32/listview.c
+++ b/dlls/comctl32/listview.c
@@ -422,6 +422,13 @@ typedef struct tagLISTVIEW_INFO
 
 static const WCHAR themeClass[] = L"ListView";
 
+enum key_state
+{
+    SHIFT_KEY = 0x1,
+    CTRL_KEY = 0x2,
+    SPACE_KEY = 0x4,
+};
+
 /*
  * forward declarations
  */
@@ -435,7 +442,7 @@ static BOOL LISTVIEW_GetViewRect(const LISTVIEW_INFO *, LPRECT);
 static void LISTVIEW_UpdateSize(LISTVIEW_INFO *);
 static LRESULT LISTVIEW_Command(LISTVIEW_INFO *, WPARAM, LPARAM);
 static INT LISTVIEW_GetStringWidthT(const LISTVIEW_INFO *, LPCWSTR, BOOL);
-static BOOL LISTVIEW_KeySelection(LISTVIEW_INFO *, INT, BOOL);
+static BOOL LISTVIEW_KeySelection(LISTVIEW_INFO *, INT, DWORD);
 static UINT LISTVIEW_GetItemState(const LISTVIEW_INFO *, INT, UINT);
 static BOOL LISTVIEW_SetItemState(LISTVIEW_INFO *, INT, const LVITEMW *);
 static VOID LISTVIEW_SetOwnerDataState(LISTVIEW_INFO *, INT, INT, const LVITEMW *);
@@ -1998,7 +2005,7 @@ static INT LISTVIEW_ProcessLetterKeys(LISTVIEW_INFO *infoPtr, WPARAM charCode, L
     }
 
     if (nItem != -1)
-        LISTVIEW_KeySelection(infoPtr, nItem, FALSE);
+        LISTVIEW_KeySelection(infoPtr, nItem, 0);
 
     return 0;
 }
@@ -3742,24 +3749,11 @@ static void LISTVIEW_SetSelection(LISTVIEW_INFO *infoPtr, INT nItem)
     infoPtr->nSelectionMark = nItem;
 }
 
-/***
- * DESCRIPTION:
- * Set selection(s) with keyboard.
- *
- * PARAMETER(S):
- * [I] infoPtr : valid pointer to the listview structure
- * [I] nItem : item index
- * [I] space : VK_SPACE code sent
- *
- * RETURN:
- *   SUCCESS : TRUE (needs to be repainted)
- *   FAILURE : FALSE (nothing has changed)
- */
-static BOOL LISTVIEW_KeySelection(LISTVIEW_INFO *infoPtr, INT nItem, BOOL space)
+/* Change item selection with key input. */
+static BOOL LISTVIEW_KeySelection(LISTVIEW_INFO *infoPtr, INT nItem, DWORD keys)
 {
-  /* FIXME: pass in the state */
-  WORD wShift = GetKeyState(VK_SHIFT) & 0x8000;
-  WORD wCtrl = GetKeyState(VK_CONTROL) & 0x8000;
+  WORD wShift = !!(keys & SHIFT_KEY);
+  WORD wCtrl = !!(keys & CTRL_KEY);
   BOOL bResult = FALSE;
 
   TRACE("nItem=%d, wShift=%d, wCtrl=%d\n", nItem, wShift, wCtrl);
@@ -3778,7 +3772,7 @@ static BOOL LISTVIEW_KeySelection(LISTVIEW_INFO *infoPtr, INT nItem, BOOL space)
         LVITEMW lvItem;
         lvItem.state = ~LISTVIEW_GetItemState(infoPtr, nItem, LVIS_SELECTED);
         lvItem.stateMask = LVIS_SELECTED;
-        if (space)
+        if (keys & SPACE_KEY)
         {
             LISTVIEW_SetItemState(infoPtr, nItem, &lvItem);
             if (lvItem.state & LVIS_SELECTED)
@@ -10189,7 +10183,18 @@ static LRESULT LISTVIEW_KeyDown(LISTVIEW_INFO *infoPtr, INT nVirtualKey, LONG lK
   }
 
   if ((nItem != -1) && (nItem != infoPtr->nFocusedItem || nVirtualKey == VK_SPACE))
-      LISTVIEW_KeySelection(infoPtr, nItem, nVirtualKey == VK_SPACE);
+  {
+      DWORD keys = 0;
+
+      if (GetKeyState(VK_SHIFT) & 0x8000)
+          keys |= SHIFT_KEY;
+      if (GetKeyState(VK_CONTROL) & 0x8000)
+          keys |= CTRL_KEY;
+      if (nVirtualKey == VK_SPACE)
+          keys |= SPACE_KEY;
+
+      LISTVIEW_KeySelection(infoPtr, nItem, keys);
+  }
 
   return 0;
 }
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/8165

[PATCH 0/2] MR8165: comctl32/listview: Fix lbutton item selection with Shift+Ctrl. - approved

[Zhiyi Zhang (＠zhiyi)]

This merge request was approved by Zhiyi Zhang.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8165