Recent

Author Topic: 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect  (Read 27486 times)

d7_2_laz

  • Hero Member
  • *****
  • Posts: 506
I thought i was ready ...migrating to 2.2 / Windows ran very smooth so far, but ...
i saw that my app's custom listview (when procssing WM_CONTEXTMENU) pushed the wrong context menu when having more than one item selected.
Then i tried to look at the MouseDown event and it's the same:

Selecting two items in the listview and check:
Delphi 7 / Lazarus 2.0.12      SelCount is 2; "Seleted" is NOT nil
2.2 RC1:                              SelCount is 2; "Seleted" is NIL

That makes me very nervous, as i rely on the property at various places.

Saw by diff that within CommCtrls.pp regarding that area there had been at least some changes
TListItem class
2.0.12:      property Selected: Boolean index Ord(lisSelected) read GetState write SetState;
2.2 RC1     property Selected: Boolean index lisSelected read GetState write SetState;
Similar with Focused etc.

What's the purpose of this change, and how to get back the old behaviour? Referring to SelCount would not be a desired workaround.

Imo, if i select two items, then "Selected" should not be nil (but point to the first item selected, as it did before)

I retried with some Shwllview app around looking at OnContextPopup: the same.
« Last Edit: October 06, 2021, 12:29:08 pm by d7_2_laz »
Lazarus 3.2  FPC 3.2.2 Win10 64bit

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9754
  • Debugger - SynEdit - and more
    • wiki
I don't think that change is related....

LCL: In TListItem, use TListItemState type directly.
SVN 64948@trunk

It is only the declaration, no actual code (other than no longer typecasting the ord int value to the state).

I don't know why the selected was changed. (or if it was intend)

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9754
  • Debugger - SynEdit - and more
    • wiki
If the release wiki page does not mention it, then please report as a bug

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 4458
  • I like bugs.
@d7_2_laz, if you can bisect the guilty revision it would help.
As Martin mentioned the change you found is not a likely candidate.
Mostly Lazarus trunk and FPC 3.2 on Manjaro Linux 64-bit.

d7_2_laz

  • Hero Member
  • *****
  • Posts: 506
I did not see much more than this cast too, but i don't think it's the guiltiy. It was simply an indicator, oha, something had changed in this context, and i believe there is a good reason for .... which one i don't know.

About the changed behaviour itself, personally i would vote: it's a bug, because 1) by contents: when selecting multiple objects, then it's not obvious why the Selected object should be nil;.  2) due to impact on the programmers, many statements or if-branches will fail now, as i already did encounter.

Referring of the documentation, i couldn't find any indication in the Release Notes.

Simply as a kind of additional info (pls. don't misunderstand) i was interested to see, what the newer Delphi docs are saying:
http://docwiki.embarcadero.com/Libraries/Sydney/en/Vcl.ComCtrls.TCustomListView.Selected
"Read Selected to access the properties of the first selected item in the list. If SelCount is 0 ...., Selected is nil"
That's the same as the previous behaviour of the VCL too.

I'd like to wait until one of guys from the VCL might be seeing it and maybe tell an opinion about.
Lazarus 3.2  FPC 3.2.2 Win10 64bit

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9754
  • Debugger - SynEdit - and more
    • wiki
Well, I don't do much on the VCL.

But its certainly worth a bug report. That will also increase the chances that the right person will notice it.

Or try also the mail list. Not everyone is on the forum.

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 4458
  • I like bugs.
I'd like to wait until one of guys from the VCL might be seeing it and maybe tell an opinion about.
"Guys from the VCL" hang around Delphi forums. This is FPC/Lazarus forum.
Please bisect the guilty revision.
 https://wiki.lazarus.freepascal.org/How_do_I_create_a_bug_report#Regression_caused_by_a_certain_revision
This is clearly your "itch to scratch" so to speak.
Mostly Lazarus trunk and FPC 3.2 on Manjaro Linux 64-bit.

d7_2_laz

  • Hero Member
  • *****
  • Posts: 506
I won't add any bug reports if i am not sure. My topic's title did end with a question mark.

In fact it turns out to be more complicated than i assumed at the beginning.
** I tried to build an easy testcase from the scratch (simple listview / custom listview / Shell listview as picked up from within the IDE), but that did NOT show up the issue.
** My own app (not using the shell controls) as well as a shell control demo from within this forum (i tried it for cross check), are showing up the issue.

So there is some layer between, and i still could not identify which specific change destroys the correct value of "Selected" when using both apps with RC1. - The impact is: one won't be able to call the right context menu.

Instead of more many words, for everyone who is interested in and wants to see a test case)
Pickup the zip file from Reply #32 within:
https://forum.lazarus.freepascal.org/index.php/topic,31625.msg397071.html#msg397071
and do:
-  Set ShellListView ("SLV") to multiselect true, and add a event for OnContextPopup.
-  Within this event, place a simple check like:
        if SLV.Selected = nil then
             ShowMessage(' Not Ok!!!  Selected is NIL, SelCount is:  '  + inttostr(SLV.SelCount))
        else  ShowMessage('OK, Selected is not  NIL;  SelCount is:   '  + inttostr(SLV.SelCount));;
-  Compile and test (Windows) with 2.012. And then compile and test with 2.2 RC1
-  For testing, select at least two items in the Listview; then do right mouse click
  ---> See what the message box is telling you
___OR SIMPLY___  pickup the zip file herein attached, which already contains those steps described.
(Remark: with new ShellCtrls.pas, sthe event "STVGetImageIndex" is no longer needed)
Lazarus 3.2  FPC 3.2.2 Win10 64bit

wp

  • Hero Member
  • *****
  • Posts: 11830
I can confirm the described behaviour with your test application on Laz 2.2.RC1 and on Laz trunk. However, I see that you are using the virtual listview mode here. When I use an umodified TShellListView which is in normal data mode I cannot reproduce the error (see attached project). So, are you sure that the observed behaviour is not due to virtual mode?

d7_2_laz

  • Hero Member
  • *****
  • Posts: 506
Hi wp.
YEP!
As mentioned, had no prob too with an easy custom listviwe (using items.Add), but my app is using "of course" the virtual mode as well. Note:  encountered no issues here at all upto RC1.
That had been one of the candidates i thought of when saying "additonal layers" that possibly might have interfered.

Do you think this could be the culprit?
Lazarus 3.2  FPC 3.2.2 Win10 64bit

wp

  • Hero Member
  • *****
  • Posts: 11830
Yes it is. By means of bisecting the svn revisions and inspecting the svn commit notes I could relatively easily find r60703 as the cause of the issue: up to r60702 the behaviour was correct, starting with r60703 the error appeared.

The commit is dated March 16 2019 and posted by Juha; the commit notes are: "LCL: Improved TListView selection handling. Issue #34877, patch from Yuriy Sydorov."

Please post a bug report and assign it to Juha. To simplify his work maybe you should try to simplify the demo: a multi-select ListView with owner data should show the issue as well.

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 9754
  • Debugger - SynEdit - and more
    • wiki
Small observation, if I use ctrl click to select the 2nd item, then it works ok. (at least Lazarus trunk)

d7_2_laz

  • Hero Member
  • *****
  • Posts: 506
Thank you all for your attention!
wp:   i'd appreciare to create a simplified test project especially around the listview in virtual mode.
It indeed does showup the issue easily -> is attached here.

I could create a user in mantis and enter (and assign) it there, but i don't intend to install additional development subsystems (ie. trunk, version control systems) on my host), at least not these days.
If that would be ok, i'd go ahead. Maybe i elaborate the teset case a bit so that WndProc -> WM_CONTEXTMENU could be checked also. Would the latter be needed?

Martin_fr and Jamie: Oh and oops. Really! Can be seen in the test case too.
- Pressing Shift and doing the selection now, the Selected object goes lost..
- Pressing Ctrl and doing the selection now, the Selected object is preserved.
Might there, for to preserve Selection, somewhere a combination of SelCount and TShiftState is inspected,
where siimply ssCtrl in included but ssShift not?
Lazarus 3.2  FPC 3.2.2 Win10 64bit

wp

  • Hero Member
  • *****
  • Posts: 11830
I wonder if this demo is indicative of a bug in TCustomListView. You inherit a new class from TCustomListView in which you override, among others, the MouseDown and MouseUp procedures. But you do not call inherited. So, all that is implemented by TCustomListView and its ancestors is not executed.

Please modify the demo such that it does not use a subclassed TCustomListView

d7_2_laz

  • Hero Member
  • *****
  • Posts: 506
A overriden TCustomListView is my special target, so it's an essential part of the test case which i won't miss.
With or without "inherited" didn't make a difference, i previously had checked that.

Here a slighty improved version (with inherited and a Check on WM_CONTEXTMENU):
Lazarus 3.2  FPC 3.2.2 Win10 64bit

 

TinyPortal © 2005-2018