View Issue Details

IDProjectCategoryView StatusLast Update
0035362LazarusLCLpublic2020-04-04 17:56
ReporterBrettAssigned To 
PrioritynormalSeverityminorReproducibilityalways
Status acknowledgedResolutionopen 
PlatformWindowsOSOS Version
Product VersionProduct Build 
Target Version2.2Fixed in Version 
Summary0035362: ListView with MultiSelect incorrectly deselects items on MouseDown
DescriptionWhen using a ListView with MultiSelect=true, clicking the mouse button down on the selected list items to start a drag operation deselects all but the item being clicked on.

I believe this is caused by the fix for https://bugs.freepascal.org/view.php?id=33330, where ListViewProc() in win32wscustomlistview.inc is creating an LM_LBUTTONUP message. The problem is that this causes the list items to be deselected on MouseDown when you try to drag them, rather than on MouseUp.

I've attached a sample project to demonstrate, but it's not really necessary.
Steps To Reproduce1) Create TListView with some items and MultiSelect=true
2) Select some items with shift+click or ctrl+click
3) Mouse down on the selection to initiate a drag-drop operation
4) Watch all items except the current one get deselected
Additional InformationProblem is new in Lazarus 2.0, does not occur in 1.8.4.
TagsNo tags attached.
Fixed in Revision
LazTarget-
WidgetsetGTK 2, Win32/Win64
Attached Files
  • ListViewBug.zip (3,753 bytes)
  • win32wscustomlistview.inc.patch (4,238 bytes)
    Index: interfaces/win32/win32wscustomlistview.inc
    ===================================================================
    --- interfaces/win32/win32wscustomlistview.inc	(revision 62715)
    +++ interfaces/win32/win32wscustomlistview.inc	(working copy)
    @@ -231,6 +231,21 @@
         end;
       end;
     
    +// #35362
    +  function OwnMouseUpNeeded(ALV: TCustomListViewAccess): Boolean;
    +  var
    +    LVInfo: PWin32WindowInfo;
    +    ListItem: TListItem;
    +  begin
    +    LVInfo:= GetWin32WindowInfo(ALV.Handle);
    +    ListItem := ALV.GetItemAt(LVInfo^.MouseX, LVInfo^.MouseY);
    +    Result := Assigned(ListItem);
    +  end;
    +
    +// #35362
    +var
    +  Pos: TSmallPoint;
    +
     begin
       Result := False;
       case Msg of
    @@ -237,6 +252,26 @@
         WM_NOTIFY:
         begin
           case PNMHdr(LParam)^.code of
    +// #35362: NM_CLICK, NM_RCLICK taken from Lazarus 1.8.4 
    +        NM_CLICK, NM_RCLICK:
    +          if OwnMouseUpNeeded(TCustomListViewAccess(AWinControl)) then
    +          begin
    +            // A listview doesn't get a WM_LBUTTONUP, WM_RBUTTONUP message,
    +            // because it keeps the message in its own event loop,
    +            // see msdn article about "Default List-View Message Processing"
    +            // therefore we take this notification and create a
    +            // LM_LBUTTONUP, LM_RBUTTONUP message out of it
    +            WinProcess := False;
    +            if PNMHdr(LParam)^.code = NM_CLICK then
    +              Msg := LM_LBUTTONUP
    +            else
    +              Msg := LM_RBUTTONUP;
    +            Pos := GetClientCursorPos(PNMHdr(LParam)^.hwndFrom);
    +            // to make correct event sequence in LCL we should postpone this message
    +            // since we are here after call of CallDefaultWindowProc
    +            PostMessage(PNMHdr(LParam)^.hwndFrom, Msg, 0, MakeLParam(Pos.x, Pos.y));
    +            Result := True;
    +          end;
             LVN_GETDISPINFOA, LVN_GETDISPINFOW:
               HandleListViewOwnerData(TCustomListViewAccess(AWinControl));
             NM_CUSTOMDRAW:
    @@ -757,48 +792,6 @@
       ItemMsg := CN_DRAWITEM;
     end;
     
    -function ListViewProc(Window: HWnd; Msg: UInt; WParam: Windows.WParam;
    -    LParam: Windows.LParam): LResult; stdcall;
    -var
    -  WindowInfo: PWin32WindowInfo;
    -  ListItem: TListItem;
    -  ListView: TCustomListView;
    -  AMsg: UINT;
    -begin
    -  case Msg of
    -    WM_LBUTTONDOWN, WM_RBUTTONDOWN:
    -      begin
    -        // A ListView doesn't get a WM_LBUTTONUP, WM_RBUTTONUP message,
    -        // because it keeps the message in its own event loop,
    -        // see msdn article about "Default List-View Message Processing"
    -        // therefore we take this notification and create a
    -        // LM_LBUTTONUP, LM_RBUTTONUP message out of it
    -
    -        WindowInfo := GetWin32WindowInfo(Window);
    -        ListView := TListView(WindowInfo^.WinControl);
    -        ListItem := ListView.GetItemAt(WindowInfo^.MouseX, WindowInfo^.MouseY);
    -
    -        if Msg = WM_LBUTTONDOWN
    -        then AMsg := LM_LBUTTONUP
    -        else AMsg := LM_RBUTTONUP;
    -
    -        // for multiselected listbox the message has to be send after current
    -        // message or the list item selecting does not work, also see issue #33330
    -        if not Assigned(ListItem) and ListView.MultiSelect then
    -        begin
    -          Result := WindowProc(Window, Msg, WParam, LParam);
    -          if not (Msg = WM_LBUTTONDBLCLK) and ((Msg = WM_LBUTTONDOWN) or not Assigned(ListView.PopupMenu)) then
    -            PostMessage(Window, AMsg, 0, MakeLParam(WindowInfo^.MouseX, WindowInfo^.MouseY));
    -          Exit;
    -        end;
    -
    -        if Assigned(ListItem) then
    -          PostMessage(Window, AMsg, 0, MakeLParam(WindowInfo^.MouseX, WindowInfo^.MouseY));
    -      end;
    -  end;
    -  Result := WindowProc(Window, Msg, WParam, LParam);
    -end;
    -
     class function TWin32WSCustomListView.CreateHandle(const AWinControl: TWinControl;
       const AParams: TCreateParams): HWND;
     const
    @@ -813,7 +806,8 @@
       with Params do
       begin
         pClassName := WC_LISTVIEW;
    -    SubClassWndProc := @ListViewProc;
    +// #35362: Funtionality of ListViewProc moved to ListViewParentMsgHandler (NM_CLICK, NM_RCLICK)
    +//  SubClassWndProc := @ListViewProc;
         WindowTitle := StrCaption;
         Flags := Flags or LISTVIEWSTYLES[TListView(AWinControl).ViewStyle] or
           LVS_SINGLESEL or LVS_SHAREIMAGELISTS or
    
  • selection.zip (67,151 bytes)

Relationships

related to 0033330 closedMichl Packages Mouse events do not fire properly when MultiSelect = True on TListView of win32/64 
related to 0033811 assignedMichl Lazarus Listview: DragMode dmAutomatic does not function with Multiselect enabled 
related to 0035917 new Lazarus MSWindows: TListView.OnContextPopup is called twice 

Activities

Brett

2019-04-11 07:24

reporter  

ListViewBug.zip (3,753 bytes)

Brett

2019-04-11 07:34

reporter   ~0115417

Apologies, this should have gone under Packages rather than FPC.

Cyrax

2019-04-11 07:53

reporter   ~0115418

Does this bug occur in the trunk or the fixes version?

Brett

2019-04-11 09:46

reporter   ~0115420

Yes, occurs in both Trunk and Fixes.

Juha Manninen

2019-04-12 13:09

developer   ~0115439

Last edited: 2019-04-12 13:11

View 2 revisions

TListView with GTK2 bindings has the same problem. It always had it, or at least since 2015.
QT works.

If this is a regression on Windows, I guess it should be fixed before the next dot release.
Brett, please make sure the bug is caused by r57906 from the related issue. Then we can reopen it.

Zeljan Rikalo

2019-04-12 14:14

developer   ~0115442

In that case both Windows and gtk2 must be fixed.

Brett

2019-04-12 14:27

reporter   ~0115443

Actually, the bug appears to only be present from r58092 (on Windows).

Serge Anvarov

2019-08-11 19:16

reporter   ~0117644

In 0033330 issue last change by Michl.
In win32wscustomlistview.inc in procedure ListViewProc code is:
[code]
...
        // for multiselected listbox the message has to be send after current
        // message or the list item selecting does not work, also see issue 0033330
        if not Assigned(ListItem) and ListView.MultiSelect then
        begin
          Result := WindowProc(Window, Msg, WParam, LParam);
          if not (Msg = WM_LBUTTONDBLCLK) and ((Msg = WM_LBUTTONDOWN) or not Assigned(ListView.PopupMenu)) then
            PostMessage(Window, AMsg, 0, MakeLParam(WindowInfo^.MouseX, WindowInfo^.MouseY));
          Exit;
        end;
...
[/code]
I don't understand the purpose of the piece "not Assigned(ListItem)". If remove it, the bug is not reproduced.

Rolf Wetjen

2020-03-08 19:03

reporter   ~0121466

Hello Lazarus team,
I'm surprised that this issue is low level as a multiselect ListView object is nearly useless for drag&drop operations by now.
Anyway, here's a path for Lazarus SVN 62701:62715M.
The creation of the missing mouse up messages (LM_LBUTTONUP and LM_RBUTTONUP) is moved from ListViewProc to ListViewParentMsgHandler as is was in 1.8.4. No need to subclass the ListView window with ListViewProc any more.
Rolf

win32wscustomlistview.inc.patch (4,238 bytes)
Index: interfaces/win32/win32wscustomlistview.inc
===================================================================
--- interfaces/win32/win32wscustomlistview.inc	(revision 62715)
+++ interfaces/win32/win32wscustomlistview.inc	(working copy)
@@ -231,6 +231,21 @@
     end;
   end;
 
+// #35362
+  function OwnMouseUpNeeded(ALV: TCustomListViewAccess): Boolean;
+  var
+    LVInfo: PWin32WindowInfo;
+    ListItem: TListItem;
+  begin
+    LVInfo:= GetWin32WindowInfo(ALV.Handle);
+    ListItem := ALV.GetItemAt(LVInfo^.MouseX, LVInfo^.MouseY);
+    Result := Assigned(ListItem);
+  end;
+
+// #35362
+var
+  Pos: TSmallPoint;
+
 begin
   Result := False;
   case Msg of
@@ -237,6 +252,26 @@
     WM_NOTIFY:
     begin
       case PNMHdr(LParam)^.code of
+// #35362: NM_CLICK, NM_RCLICK taken from Lazarus 1.8.4 
+        NM_CLICK, NM_RCLICK:
+          if OwnMouseUpNeeded(TCustomListViewAccess(AWinControl)) then
+          begin
+            // A listview doesn't get a WM_LBUTTONUP, WM_RBUTTONUP message,
+            // because it keeps the message in its own event loop,
+            // see msdn article about "Default List-View Message Processing"
+            // therefore we take this notification and create a
+            // LM_LBUTTONUP, LM_RBUTTONUP message out of it
+            WinProcess := False;
+            if PNMHdr(LParam)^.code = NM_CLICK then
+              Msg := LM_LBUTTONUP
+            else
+              Msg := LM_RBUTTONUP;
+            Pos := GetClientCursorPos(PNMHdr(LParam)^.hwndFrom);
+            // to make correct event sequence in LCL we should postpone this message
+            // since we are here after call of CallDefaultWindowProc
+            PostMessage(PNMHdr(LParam)^.hwndFrom, Msg, 0, MakeLParam(Pos.x, Pos.y));
+            Result := True;
+          end;
         LVN_GETDISPINFOA, LVN_GETDISPINFOW:
           HandleListViewOwnerData(TCustomListViewAccess(AWinControl));
         NM_CUSTOMDRAW:
@@ -757,48 +792,6 @@
   ItemMsg := CN_DRAWITEM;
 end;
 
-function ListViewProc(Window: HWnd; Msg: UInt; WParam: Windows.WParam;
-    LParam: Windows.LParam): LResult; stdcall;
-var
-  WindowInfo: PWin32WindowInfo;
-  ListItem: TListItem;
-  ListView: TCustomListView;
-  AMsg: UINT;
-begin
-  case Msg of
-    WM_LBUTTONDOWN, WM_RBUTTONDOWN:
-      begin
-        // A ListView doesn't get a WM_LBUTTONUP, WM_RBUTTONUP message,
-        // because it keeps the message in its own event loop,
-        // see msdn article about "Default List-View Message Processing"
-        // therefore we take this notification and create a
-        // LM_LBUTTONUP, LM_RBUTTONUP message out of it
-
-        WindowInfo := GetWin32WindowInfo(Window);
-        ListView := TListView(WindowInfo^.WinControl);
-        ListItem := ListView.GetItemAt(WindowInfo^.MouseX, WindowInfo^.MouseY);
-
-        if Msg = WM_LBUTTONDOWN
-        then AMsg := LM_LBUTTONUP
-        else AMsg := LM_RBUTTONUP;
-
-        // for multiselected listbox the message has to be send after current
-        // message or the list item selecting does not work, also see issue #33330
-        if not Assigned(ListItem) and ListView.MultiSelect then
-        begin
-          Result := WindowProc(Window, Msg, WParam, LParam);
-          if not (Msg = WM_LBUTTONDBLCLK) and ((Msg = WM_LBUTTONDOWN) or not Assigned(ListView.PopupMenu)) then
-            PostMessage(Window, AMsg, 0, MakeLParam(WindowInfo^.MouseX, WindowInfo^.MouseY));
-          Exit;
-        end;
-
-        if Assigned(ListItem) then
-          PostMessage(Window, AMsg, 0, MakeLParam(WindowInfo^.MouseX, WindowInfo^.MouseY));
-      end;
-  end;
-  Result := WindowProc(Window, Msg, WParam, LParam);
-end;
-
 class function TWin32WSCustomListView.CreateHandle(const AWinControl: TWinControl;
   const AParams: TCreateParams): HWND;
 const
@@ -813,7 +806,8 @@
   with Params do
   begin
     pClassName := WC_LISTVIEW;
-    SubClassWndProc := @ListViewProc;
+// #35362: Funtionality of ListViewProc moved to ListViewParentMsgHandler (NM_CLICK, NM_RCLICK)
+//  SubClassWndProc := @ListViewProc;
     WindowTitle := StrCaption;
     Flags := Flags or LISTVIEWSTYLES[TListView(AWinControl).ViewStyle] or
       LVS_SINGLESEL or LVS_SHAREIMAGELISTS or

Juha Manninen

2020-03-16 14:32

developer   ~0121621

Rolf, did you also test DragMode dmAutomatic? See related issue 0033811.
I personally don't have Windows right now to test.

Rolf Wetjen

2020-03-22 19:13

reporter   ~0121685

Hi Juha,
yes, working. I've attached a small test project if someone else want to test to.
Regards
Rolf

selection.zip (67,151 bytes)

Martin Friebe

2020-04-04 03:11

manager   ~0121880

@RolfW

I am doing some testing.

Not important, but curious: Why this comment? It implies wrong message order, but the order seems fine. (The msg should go to the end of the queue).
+ // to make correct event sequence in LCL we should postpone this message
+ // since we are here after call of CallDefaultWindowProc

The more serious issue:
With the patch, for some reason the MouseDown event is hold back.
All 3 events (down, click, up) occur when the mouse is released.

Mouse down should occur as soon as the button is pressed. That worked before the patch.

Martin Friebe

2020-04-04 16:00

manager   ~0121900

Unfortunately changing the order (LCL first) will kill dragging completely / https://github.com/User4martin/lazarus/tree/f-test-listview-event-order-drag

Martin Friebe

2020-04-04 17:29

manager   ~0121903

I have updated the changes published on github.

This *appears* to work.
But it needs serious testing.

To Anyone who wants to test: Please apply it. And leave feedback.
Ensure, that
 - OnMouseDown
 - OnClick (except, when dragging)
 - OnMouseUp
are always called, and always in this order.

---
This is NOT a candidate for merging to fixes branch.
But if successful in tests, then it can make it for 2.2

Before merging to trunk, debugln needs to be removed

Martin Friebe

2020-04-04 17:56

manager   ~0121904

There is another issue, but that existed before the patch too: While over the listview the mouse cursor flickers between 2 states (also happens if drag would be accepted)

Issue History

Date Modified Username Field Change
2019-04-11 07:24 Brett New Issue
2019-04-11 07:24 Brett File Added: ListViewBug.zip
2019-04-11 07:34 Brett Note Added: 0115417
2019-04-11 07:53 Cyrax Note Added: 0115418
2019-04-11 08:50 Jonas Maebe Project FPC => Lazarus
2019-04-11 09:46 Brett Note Added: 0115420
2019-04-12 10:10 Juha Manninen Relationship added related to 0033330
2019-04-12 13:09 Juha Manninen Note Added: 0115439
2019-04-12 13:11 Juha Manninen Note Edited: 0115439 View Revisions
2019-04-12 14:14 Zeljan Rikalo Note Added: 0115442
2019-04-12 14:27 Brett Note Added: 0115443
2019-04-12 14:54 Juha Manninen LazTarget => -
2019-04-12 14:54 Juha Manninen Widgetset => GTK 2, Win32/Win64
2019-04-12 14:54 Juha Manninen Product Version 3.0.4 =>
2019-04-12 14:56 Juha Manninen Relationship added related to 0033811
2019-08-11 11:41 Juha Manninen Relationship added related to 0035917
2019-08-11 19:16 Serge Anvarov Note Added: 0117644
2019-08-11 21:16 Juha Manninen Assigned To => Michl
2019-08-11 21:16 Juha Manninen Status new => assigned
2020-03-08 19:03 Rolf Wetjen File Added: win32wscustomlistview.inc.patch
2020-03-08 19:03 Rolf Wetjen Note Added: 0121466
2020-03-16 09:09 Juha Manninen Target Version => 2.2
2020-03-16 09:09 Juha Manninen Widgetset GTK 2, Win32/Win64 => GTK 2, Win32/Win64
2020-03-16 14:32 Juha Manninen Note Added: 0121621
2020-03-22 19:13 Rolf Wetjen File Added: selection.zip
2020-03-22 19:13 Rolf Wetjen Note Added: 0121685
2020-04-03 10:55 Juha Manninen Assigned To Michl => Martin Friebe
2020-04-04 03:11 Martin Friebe Note Added: 0121880
2020-04-04 16:00 Martin Friebe Note Added: 0121900
2020-04-04 16:01 Martin Friebe Assigned To Martin Friebe =>
2020-04-04 16:01 Martin Friebe Status assigned => acknowledged
2020-04-04 17:29 Martin Friebe Note Added: 0121903
2020-04-04 17:56 Martin Friebe Note Added: 0121904