winforms icon indicating copy to clipboard operation
winforms copied to clipboard

Updating ListView.ListViewItem.ListViewSubItem Text or Name values updates them all, why? is this normal?

Open creizlein opened this issue 11 months ago • 7 comments

.NET version

Tested and Checked on .NET Runtime 7.0 and 8.0

Did it work in .NET Framework?

Not tested/verified

Did it work in any of the earlier releases of .NET Core or .NET 5+?

n/a.

Issue description

Talking about WinForms, ListView Control, ListViewItem and ListViewSubItems

when you have a ListViewItem , lets say, with 10 ListViewSubItems, on a Details view, and you change the text, it triggers a update text on ALL of the subitems.

If you check on the Text Setter, it reads as follows:

ListViewSubItem class

        public string Text {
            get => text ?? string.Empty;
            set {
                text = value;
                _owner?.UpdateSubItems(-1);
            }
        }

_owner being the ListViewItem, so if we go there, we find this:

internal void UpdateSubItems(int index) => UpdateSubItems(index, SubItemCount);
internal void UpdateSubItems(int index, int oldCount) {
    // trim down, not modified, just formated.
    if (_listView is not null && _listView.IsHandleCreated) {
        int subItemCount = SubItemCount;
        int itemIndex = Index;

        if (index != -1) {
            _listView.SetItemText(itemIndex, index, _subItems[index].Text);
        } else {
            for (int i = 0; i < subItemCount; i++) {
                _listView.SetItemText(itemIndex, i, _subItems[i].Text);
            }
        }

        for (int i = subItemCount; i < oldCount; i++) {
            _listView.SetItemText(itemIndex, i, string.Empty);
        }
    }
}

the ListView itself is then the one in charge of triggering the PInvoke.SendMessage with each of the subitems actual text.

So my question is, why does this happen? and even worst, why does changing the NAME of the item also triggers an update on the text?

Seems a little bit weird and overwhelming, but then again I am just checking and making sure this is not a problem..

I have found some laggy and performance issues and found myself that the ListView was killing the UI Thread with message loops because I am updating a text on a subitem on column 20 , of course, it then updates all 30 of them which are all the columns I have.

Steps to reproduce

Create a WinForms app with a ListView on it, add a couple of ListViewItems with several subitems, and try to change the text of Nth subitem only and check how many of them get triggered to update.

creizlein avatar Feb 28 '24 03:02 creizlein

I believe the problem comes from the fact that the ListViewSubItem calls UpdateSubItems(-1) , with -1 of course, but then the ListViewItem itself does not correct that and calls the ListView to only update that specific item with the index of itself.

creizlein avatar Feb 28 '24 03:02 creizlein

@creizlein I'd suggest you look into the code history and see. Generally small mistakes like this happen in a large code base. I imagine the team would be happy to review a PR if you submitted one.

elachlan avatar Feb 28 '24 04:02 elachlan

Maybe something like:

[Localizable(true)]
[AllowNull]
public string Text
{
    get => text ?? string.Empty;
    set
    {
        text = value;
        _owner?.UpdateSubItems(_owner.SubItems.IndexOf(this));
    }
}

[Localizable(true)]
[AllowNull]
public string Name
{
    get => name ?? string.Empty;
    set
    {
        name = value;
        _owner?.UpdateSubItems(_owner.SubItems.IndexOf(this));
    }
}

elachlan avatar Feb 28 '24 04:02 elachlan

Idk I am up to the task, but I will try to find all the possibilities and submit something. I still don't understand why it updates the TEXT when you change the NAME, I believe it has something to do for when there is no text at all, it takes the name and put it as text, but if that's the case then it should IF it at least.

Also I an wondering why there is no check on if the name/text is changed first before sending the messages, like if (name!=value) just return. I will put together a PR for it.

Was first wondering if it was on purpose at all, maybe there is a solid reason behind updating them all.

creizlein avatar Feb 28 '24 04:02 creizlein

I'd just try it and see. Same with the name update triggering the change. I figure its probably got something to do with the designer. There aren't much references to the Name though.

elachlan avatar Feb 28 '24 04:02 elachlan

I believe the name is used for Text in Designer and the code doesn’t check if it’s running under designer. But I also don’t know why anyone would change Name at runtime.Sent from my iPhoneI apologize for any typos Siri might have made.(503) 803-6077On Feb 27, 2024, at 6:31 PM, Lachlan Ennis @.***> wrote: I'd just try it and see. Same with the name update triggering the change. I figure its probably got something to do with the designer. There aren't much references to the Name though.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: @.***>

paul1956 avatar Feb 28 '24 07:02 paul1956

@paul1956 's theory makes a lot of sense. As @elachlan said, we would be happy to review a PR should you submit one. Given that this would be an actual behavior change, we would need to put it behind a quirk so that it doesn't impact existing applications.

merriemcgaw avatar Feb 28 '24 21:02 merriemcgaw

@merriemcgaw, would you accept a PR without the quirk if the behavior is only changed at Text and not Name?

elachlan avatar May 12 '24 23:05 elachlan

@elachlan I think we would actually, thank you!

merriemcgaw avatar May 17 '24 22:05 merriemcgaw

@creizlein I have a pr up if you would like to review it. We can't seem to reproduce your performance issue, but it should remove one of the loops.

elachlan avatar May 20 '24 10:05 elachlan

Sorry I am late to the party, i was some days out of town and was also watching @merriemcgaw presentation today, I would love to give this a try, let me setup my dev platform so i can run this last code and provide some feedback hopefully this week.

Btw, great presentation @merriemcgaw , kudos !

creizlein avatar May 22 '24 07:05 creizlein