View Issue Details

IDProjectCategoryView StatusLast Update
0035917LazarusLCLpublic2020-04-20 19:38
Reporternanobit Assigned ToMartin Friebe  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionreopened 
Platformwin32OSWindows 
Product Version2.0.2 
Fixed in Version2.2 
Summary0035917: MSWindows: TListView.OnContextPopup is called twice
DescriptionIn TListView (with Multiselect = true),
on empty space (where no item is hit), right click to trigger OnContextPopup.
TListView.OnContextPopup (TControl.WMContextMenu()) is called two times,
thus the menu is shown twice.

The reason is in win32wscustomlistview.inc / ListViewProc() which ignores OnContextPopup.

A solution is: replace
"not Assigned(ListView.PopupMenu))"
with:
"not (Assigned(ListView.PopupMenu) or Assigned(ListView.OnContextPopup)))"

This also requires:
TcustomListView.OnContextPopup to be public instead of protected.

This fixes the reported bug, but the source section is probably not enough overall,
if this section is also related to: https://bugs.freepascal.org/view.php?id=35362
TagsNo tags attached.
Fixed in Revision63012,63013,63022,63023
LazTarget2.2
WidgetsetWin32/Win64
Attached Files

Relationships

related to 0035362 resolvedMartin Friebe ListView with MultiSelect incorrectly deselects items on MouseDown 

Activities

nanobit

2019-08-03 11:41

reporter   ~0117552

Last edited: 2020-04-20 09:24

View 9 revisions

I changed ListViewProc() to address more issues (also 0035362).
I'm not a LCL author, so please check/improve, and on many systems.

win32wscustomlistview.inc: added:
function isMouseBtnDown: Boolean;
// Other Lazarus Listview versions detect this state indirectly:
// They found an indication for downstate after WM_BUTTONDOWN
// (Windows never changed this undocumented behavior):
// result := not (assigned( hitListItem) or listview.multiSelect);
begin
  result := (getAsyncKeyState(VK_LBUTTON) < 0) or
            (getAsyncKeyState(VK_RBUTTON) < 0) or
            (getAsyncKeyState(VK_MBUTTON) < 0);
end;

win32wscustomlistview.inc: changed:
function ListViewProc(Window: HWnd; Msg: UInt; WParam: Windows.WParam;
    LParam: Windows.LParam): LResult; stdcall;
var
  WindowInfo: PWin32WindowInfo;
  BtnUpMsg: UINT; LMessage: TLMessage;
  LV: TCustomListView;
begin
  case Msg of
    WM_LBUTTONDOWN, WM_RBUTTONDOWN:
    begin
      WindowInfo := GetWin32WindowInfo(Window);
      WindowInfo^.LastLVNMsg := 0;
      Result := WindowProc(Window, Msg, WParam, LParam);
      case (-WindowInfo^.LastLVNMsg) of
        NM_Click, NM_RClick, NM_SETFOCUS:
        begin
          WindowInfo^.LastLVNMsg := 0;
          if not isMouseBtnDown then
          begin
            if Msg = WM_LBUTTONDOWN
            then BtnUpMsg := LM_LBUTTONUP
            else BtnUpMsg := LM_RBUTTONUP;

            // only LCL (not Windows) needs this message
            LMessage.msg := BtnUpMsg;
            LMessage.wParam := 0; // mouseUp-keys (pressed in WindowProc()) unknown
            LMessage.lParam := windows.MakeLParam( WindowInfo^.MouseX, WindowInfo^.MouseY);
            LMessage.Result := 0;
            DeliverMessage( WindowInfo^.WinControl, LMessage);
          end;
        end;
        LVN_BeginDrag:; // control.beginDrag (again) not needed here if capture-concept works
      end;
      exit;
    end;

    WM_CONTEXTMENU, WM_LBUTTONUP, WM_RBUTTONUP:
    begin // show menu at time of Windows request (might be during RDown)
      WindowInfo := GetWin32WindowInfo(Window);
      LV := TCustomListView(WindowInfo^.WinControl);
      if Assigned(LV.PopupMenu) or
        Assigned(TCustomListViewAccess(LV).OnContextPopup) or
        (msg <> WM_CONTEXTMENU) then
          WindowInfo^.LastLVNMsg := 0;
    end;
  end;
  Result := WindowProc(Window, Msg, WParam, LParam);
end;

------------------------
win32wscustomlistview.inc:
Added StoreListViewMsg() at the end of ListViewParentMsgHandler():

  procedure StoreListViewMsg(ALV: TCustomListViewAccess);
  var
    LVInfo: PWin32WindowInfo;
  begin
    LVInfo := GetWin32WindowInfo(ALV.Handle);
    LVInfo^.LastLVNMsg := Byte( abs(Longint(NMHdr^.code)));
    WinProcess := False;
  end;

begin
  Result := False;
  case Msg of
    WM_NOTIFY:
    begin
      case PNMHdr(LParam)^.code of
        LVN_GETDISPINFOA, LVN_GETDISPINFOW:
          HandleListViewOwnerData(TCustomListViewAccess(AWinControl));
        NM_CUSTOMDRAW:
          HandleListViewCustomDraw(TCustomListViewAccess(AWinControl));
        NM_SETFOCUS: // separate case for easier debugging
          StoreListViewMsg(TCustomListViewAccess(AWinControl));
        NM_CLICK, NM_RCLICK, LVN_BEGINDRAG, LVN_BEGINRDRAG:
          StoreListViewMsg(TCustomListViewAccess(AWinControl));
      end;
    end;
  end;
end;

------------------------
and in win32proc.pp:

TWin32WindowInfo = record
....
IMEComposed: Boolean;
LastLVNMsg: byte; // added one field
DrawItemHandler: TDrawItemHandlerProc;
....
end;

------------------------
Edited: In the meantime, I used this solution on Win10.
The only known remaining issue is:
Apps which use (assign) the listview.onMouseDown property,
will get the onMouseDown call delayed (on mouse release), but still prior to onMouseUp.
Since most apps don't need this property, we could keep this alternative on the shelf.
A perfect solution (as noted in 00353620035362:0121903) requires more global change.

Juha Manninen

2019-08-11 12:01

reporter   ~0117641

Last edited: 2019-08-11 12:02

View 3 revisions

If you know how to fix it, please create a proper patch.
 https://wiki.lazarus.freepascal.org/Creating_A_Patch
Use the trunk (development version) when creating patches.

Martin Friebe

2020-04-05 21:20

manager   ~0121951

Please check against https://github.com/User4martin/lazarus/compare/f-test-listview-event-order-drag
as diff: https://github.com/User4martin/lazarus/compare/f-test-listview-event-order-drag.diff

That patch fixes the delay of OnMouseDown.

Any changes made should:
- Always respect the order of MouseDown, Click, MouseUp
    and for right mouse, somewhere in that list (after down??) the popUpMenu
- drag / auto drag works
  with/without multiselect

And tested fol all other related issues, please

nanobit

2020-04-06 10:42

reporter   ~0121954

Thank you, will report findings (if any) in 0035362

nanobit

2020-04-11 00:10

reporter   ~0122066

Last edited: 2020-04-11 10:50

View 3 revisions

@Martin: In my given ListViewProc(), I replaced the line:
Result := WindowProc(Window, Msg, WParam, LParam);

with your block:
CurrentListViewInClick := Window;
CurrentListViewInClickState := clsClicked;
Result := WindowProcEx(Window, Msg, WParam, LParam, [wpfLclBeforeWin]);
if CurrentListViewInClickState = clsCaptureLost then SetCapture(window);
CurrentListViewInClickState := clsNone;

And also added your WM_CAPTURECHANGED block.
OnMouseDown and OnMouseUp seem to work as intended.
What is a bit surprising that all Lazarus versions (up to now) ran without capture checks,
but they are definitely needed after WindowProcEx() call.

On the other hand, now with the new order (LCL (MouseDown),
Windows (EatenMouseUp), LCL (LCLMouseUp)), a new possibility arises:
Move NotifyMsg-handling back into ListViewParentMsgHandler().
It's not required, but could simplify things.

nanobit

2020-04-11 08:17

reporter  

ListViewProc_captureChecked.txt (2,966 bytes)   
function isMouseBtnDown: Boolean;
begin 
  result := (getAsyncKeyState(VK_LBUTTON) < 0) or
            (getAsyncKeyState(VK_RBUTTON) < 0) or
            (getAsyncKeyState(VK_MBUTTON) < 0);
end;

var
  CurrentListViewInClick: HWND;
  CurrentListViewInClickState: (clsNone, clsClicked, clsCaptured, clsCaptureLost);

function ListViewProc(Window: HWnd; Msg: UInt; WParam: Windows.WParam;
    LParam: Windows.LParam): LResult; stdcall;
var
  WindowInfo: PWin32WindowInfo;
  BtnUpMsg: UINT; LMessage: TLMessage;
begin
  if Window <> CurrentListViewInClick then
    CurrentListViewInClickState := clsNone;
  case Msg of
    WM_LBUTTONDOWN, WM_RBUTTONDOWN:
    begin
      WindowInfo := GetWin32WindowInfo(Window);
      WindowInfo^.LastLVNMsg := 0;

      CurrentListViewInClick := Window;
      CurrentListViewInClickState := clsClicked;
      Result := WindowProc(Window, Msg, WParam, LParam);
//    Result := WindowProcEx(Window, Msg, WParam, LParam, [wpfLclBeforeWin]); // better alternative
      if CurrentListViewInClickState = clsCaptureLost then
        SetCapture(window);
      CurrentListViewInClickState := clsNone;

      case (-WindowInfo^.LastLVNMsg) of
        NM_Click, NM_RClick, NM_SETFOCUS:
        begin
          WindowInfo^.LastLVNMsg := 0;
          if not isMouseBtnDown then
          begin
            if Msg = WM_LBUTTONDOWN
            then BtnUpMsg := LM_LBUTTONUP
            else BtnUpMsg := LM_RBUTTONUP;

           // only LCL (not Windows) needs this message
            LMessage.msg := BtnUpMsg;
            LMessage.wParam := 0; // mouseUp-keys (pressed in WindowProc()) unknown
            LMessage.lParam := MakeLParam( longint(WindowInfo^.MouseX), longint(WindowInfo^.MouseY));
            LMessage.Result := 0;
            DeliverMessage( WindowInfo^.WinControl, LMessage);
          end;
        end;
        LVN_BeginDrag:
        begin
          // WindowInfo^.WinControl.beginDrag unneeded (double) if capture-concept works
        end;
      end;
    end;

    WM_CAPTURECHANGED:
    begin
      case CurrentListViewInClickState of
        clsClicked:
          if GetCapture = Window then
            CurrentListViewInClickState := clsCaptured;
        clsCaptured:
          if GetCapture = 0 then
            CurrentListViewInClickState := clsCaptureLost;
        clsCaptureLost:
          if GetCapture <> 0 then
            CurrentListViewInClickState := clsNone;
      end;
      if not (CurrentListViewInClickState in [clsCaptured, clsCaptureLost]) then
        Result := WindowProc(Window, Msg, WParam, LParam);
    end;

    WM_CONTEXTMENU, WM_LBUTTONUP, WM_RBUTTONUP:
    begin
      WindowInfo := GetWin32WindowInfo(Window);
      WindowInfo^.LastLVNMsg := 0;
      Result := WindowProc(Window, Msg, WParam, LParam);
    end;

    else
    begin
      Result := WindowProc(Window, Msg, WParam, LParam);
    end;
  end;
end;

nanobit

2020-04-11 15:31

reporter   ~0122073

Last edited: 2020-04-12 07:39

View 4 revisions

tiny addition for menuless RMouseUp.
Adding file again; file replace seems unsupported for reporters.

Some background:
Currently, Windows does not know whether we want a menu, thus always sends WM_ContextMenu.
NM_RClick almost obviously allows (with message-result <> 0) the prevention of this message sending:
"Return nonzero to not allow the default processing, or zero to allow the default processing."
So they gave us an option to disable "default processing" and the api documentation gave us the task
to interpret what "default processing" could mean. "Undefined" is a feature. I kept this.

ListViewProc_captureChecked2.txt (3,186 bytes)   
function isMouseBtnDown: Boolean;
begin 
  result := (getAsyncKeyState(VK_LBUTTON) < 0) or
            (getAsyncKeyState(VK_RBUTTON) < 0) or
            (getAsyncKeyState(VK_MBUTTON) < 0);
end;

var
  CurrentListViewInClick: HWND;
  CurrentListViewInClickState: (clsNone, clsClicked, clsCaptured, clsCaptureLost);

function ListViewProc(Window: HWnd; Msg: UInt; WParam: Windows.WParam;
    LParam: Windows.LParam): LResult; stdcall;
var
  WindowInfo: PWin32WindowInfo;
  BtnUpMsg: UINT; LMessage: TLMessage;
  LV: TCustomListView;
begin
  if Window <> CurrentListViewInClick then
    CurrentListViewInClickState := clsNone;
  case Msg of
    WM_LBUTTONDOWN, WM_RBUTTONDOWN:
    begin
      WindowInfo := GetWin32WindowInfo(Window);
      WindowInfo^.LastLVNMsg := 0;

      CurrentListViewInClick := Window;
      CurrentListViewInClickState := clsClicked;
      Result := WindowProc(Window, Msg, WParam, LParam);
//    Result := WindowProcEx(Window, Msg, WParam, LParam, [wpfLclBeforeWin]); // better alternative
      if CurrentListViewInClickState = clsCaptureLost then
        SetCapture(window);
      CurrentListViewInClickState := clsNone;

      case (-WindowInfo^.LastLVNMsg) of
        NM_Click, NM_RClick, NM_SETFOCUS:
        begin
          WindowInfo^.LastLVNMsg := 0;
          if not isMouseBtnDown then
          begin
            if Msg = WM_LBUTTONDOWN
            then BtnUpMsg := LM_LBUTTONUP
            else BtnUpMsg := LM_RBUTTONUP;

           // only LCL (not Windows) needs this message
            LMessage.msg := BtnUpMsg;
            LMessage.wParam := 0; // mouseUp-keys (pressed in WindowProc()) unknown
            LMessage.lParam := MakeLParam( longint(WindowInfo^.MouseX), longint(WindowInfo^.MouseY));
            LMessage.Result := 0;
            DeliverMessage( WindowInfo^.WinControl, LMessage);
          end;
        end;
        LVN_BeginDrag:
        begin
          // WindowInfo^.WinControl.beginDrag unneeded (double) if capture-concept works
        end;
      end;
    end;

    WM_CAPTURECHANGED:
    begin
      case CurrentListViewInClickState of
        clsClicked:
          if GetCapture = Window then
            CurrentListViewInClickState := clsCaptured;
        clsCaptured:
          if GetCapture = 0 then
            CurrentListViewInClickState := clsCaptureLost;
        clsCaptureLost:
          if GetCapture <> 0 then
            CurrentListViewInClickState := clsNone;
      end;
      if not (CurrentListViewInClickState in [clsCaptured, clsCaptureLost]) then
        Result := WindowProc(Window, Msg, WParam, LParam);
    end;

    WM_CONTEXTMENU, WM_LBUTTONUP, WM_RBUTTONUP:
    begin
      WindowInfo := GetWin32WindowInfo(Window);
      LV := TCustomListView(WindowInfo^.WinControl);
      if Assigned(LV.PopupMenu) or
        Assigned(TCustomListViewAccess(LV).OnContextPopup) or
        (msg <> WM_CONTEXTMENU) then
          WindowInfo^.LastLVNMsg := 0;
      Result := WindowProc(Window, Msg, WParam, LParam); 
    end;

    else
    begin
      Result := WindowProc(Window, Msg, WParam, LParam);
    end;
  end;
end;

Juha Manninen

2020-04-11 16:37

reporter   ~0122075

Again :
 https://wiki.lazarus.freepascal.org/Creating_A_Patch

Martin Friebe

2020-04-18 21:54

manager   ~0122234

Last edited: 2020-04-18 21:58

View 2 revisions

Please test with revision 63012.
Please if possible also run the testcase (test/runtestgui.lpi), only run the lcl/listview tests.
(Note: Do not move your mouse, once the test case runs)

This fix keeps the delayed OnMouseDown.

If any further issues exists, please also test with/without Manifest (project options)

Also see details in 0035362:0122233

--------
The Context menu event that was received originally came in the wrong order: context, mouse down, mouse up

The fix now stops the early event, generates a mouse-up and with that cause one context-menu event, which should be at the correct timing.

nanobit

2020-04-19 13:51

reporter   ~0122252

Thank you. But I prefer my current solution to show
contextmenu when requested by Windows (in first WM_CONTEXTMENU),
and thus no reliance on manual LM_RBUTTONUP.

In trunk version, on right click on empty space in SingleSelect-listview:
Sometimes the menu shows on RightDown (if done outside of already visible itemmenu)
and in other cases after RightUp. If the menu was shown after RightUp and afterwards I click on item (not menu),
then the menu hides (correctly), but also item-dragmove starts (wrongly on mouseup).

Martin Friebe

2020-04-19 16:34

manager   ~0122257

@nanobit: There have been previous reports about event order (for listview and other components). So for a final fix, it is necessary to include the event order (as good as can be done).
It appears to depend on an item being selected, or maybe the previous menu having been opened above an item; and then going to the empty space.

I will look into it.

I also noted, that on multi-select re-opening the menu this way does get the old x/y.

If you find any further incorrect behaviour please report.
Thanks

Martin Friebe

2020-04-19 21:49

manager   ~0122261

Please test with 63022
Thanks

nanobit

2020-04-20 09:27

reporter   ~0122265

Last edited: 2020-04-20 10:02

View 2 revisions

I've found no further enduser-issues,
just internal refinements:

1) Prefer packed together:
try Result := WindowProc(Window, Msg, WParam, LParam);
finally ListViewWindProcInfo.ActiveListView:= nil; end;
Thus any following send-routine (not only postmessage(msgUp)) would work.

2) Removal of "send after" comment, was relict from
ListViewParentMsgHandler() which called postmessage( ownMsgUp),
thus before deliverMessage( msgDown) in windowProc( msgDown).
ListViewProc() contains msgDown/ownMsgUp order naturally.

3) qualified calling windows.makeLParam()
can work around FPC/LCL limit under rangechecking and neg. inputs.

Martin Friebe

2020-04-20 19:38

manager   ~0122294

Changed.

If you do find further issues, either *re-open* this issue, or open a new issue.

Comments on closed issues can easily be missed

Issue History

Date Modified Username Field Change
2019-07-31 20:01 nanobit New Issue
2019-08-03 11:41 nanobit Note Added: 0117552
2019-08-04 15:53 nanobit Note Edited: 0117552 View Revisions
2019-08-08 18:37 nanobit Note Edited: 0117552 View Revisions
2019-08-11 11:41 Juha Manninen Relationship added related to 0035362
2019-08-11 12:01 Juha Manninen Note Added: 0117641
2019-08-11 12:02 Juha Manninen Note Edited: 0117641 View Revisions
2019-08-11 12:02 Juha Manninen Note Edited: 0117641 View Revisions
2020-04-05 20:57 nanobit Note Edited: 0117552 View Revisions
2020-04-05 21:20 Martin Friebe Note Added: 0121951
2020-04-06 10:41 nanobit Note Edited: 0117552 View Revisions
2020-04-06 10:42 nanobit Note Added: 0121954
2020-04-09 14:03 nanobit Note Edited: 0117552 View Revisions
2020-04-11 00:09 nanobit Note Edited: 0117552 View Revisions
2020-04-11 00:10 nanobit Note Added: 0122066
2020-04-11 02:30 nanobit Note Edited: 0122066 View Revisions
2020-04-11 08:17 nanobit File Added: ListViewProc_captureChecked.txt
2020-04-11 10:50 nanobit Note Edited: 0122066 View Revisions
2020-04-11 14:52 nanobit Note Edited: 0117552 View Revisions
2020-04-11 15:31 nanobit File Added: ListViewProc_captureChecked2.txt
2020-04-11 15:31 nanobit Note Added: 0122073
2020-04-11 16:37 Juha Manninen Note Added: 0122075
2020-04-12 06:21 nanobit Note Edited: 0122073 View Revisions
2020-04-12 07:08 nanobit Note Edited: 0122073 View Revisions
2020-04-12 07:39 nanobit Note Edited: 0122073 View Revisions
2020-04-17 17:16 Martin Friebe Assigned To => Martin Friebe
2020-04-17 17:16 Martin Friebe Status new => assigned
2020-04-18 21:54 Martin Friebe Status assigned => resolved
2020-04-18 21:54 Martin Friebe Resolution open => fixed
2020-04-18 21:54 Martin Friebe Fixed in Version => 2.2
2020-04-18 21:54 Martin Friebe Fixed in Revision => 63012,63013
2020-04-18 21:54 Martin Friebe LazTarget => 2.2
2020-04-18 21:54 Martin Friebe Widgetset Win32/Win64 => Win32/Win64
2020-04-18 21:54 Martin Friebe Note Added: 0122234
2020-04-18 21:58 Martin Friebe Note Edited: 0122234 View Revisions
2020-04-19 13:51 nanobit Note Added: 0122252
2020-04-19 16:28 Martin Friebe Status resolved => assigned
2020-04-19 16:28 Martin Friebe Resolution fixed => reopened
2020-04-19 16:34 Martin Friebe Note Added: 0122257
2020-04-19 21:49 Martin Friebe Status assigned => resolved
2020-04-19 21:49 Martin Friebe Fixed in Revision 63012,63013 => 63012,63013,63022,63023
2020-04-19 21:49 Martin Friebe Widgetset Win32/Win64 => Win32/Win64
2020-04-19 21:49 Martin Friebe Note Added: 0122261
2020-04-20 09:24 nanobit Note Edited: 0117552 View Revisions
2020-04-20 09:27 nanobit Note Added: 0122265
2020-04-20 10:02 nanobit Note Edited: 0122265 View Revisions
2020-04-20 19:38 Martin Friebe Note Added: 0122294