Inconsistency in ListCtrl::CheckItem between the generic and MSW implementations (Issue #23869)

Jorge Moraleda

unread,

Sep 12, 2023, 10:57:12 PM9/12/23

to wx-...@googlegroups.com, Subscribed

Method CheckItem in the generic implementation of ListCtrl fires either an EVT_LIST_ITEM_CHECKED or an EVT_LIST_ITEM_UNCHECKED event as appropriate https://github.com/wxWidgets/wxWidgets/blob/eb8a57760e61e72959593200d4df8d8b36dfb87a/src/generic/listctrl.cpp#L3912

But method CheckItem in the MSW implementation does not fire any events https://github.com/wxWidgets/wxWidgets/blob/eb8a57760e61e72959593200d4df8d8b36dfb87a/src/msw/listctrl.cpp#L1495

Either behavior is fine if it is well documented, but IMHO we should provide consistent behavior across platforms.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Maarten

unread,

Sep 13, 2023, 3:18:26 AM9/13/23

to wx-...@googlegroups.com, Subscribed

The MSW version generates the event here:
https://github.com/wxWidgets/wxWidgets/blob/eb8a57760e61e72959593200d4df8d8b36dfb87a/src/msw/listctrl.cpp#L2586

I checked in the listctrl example (List -> Enable Checkboxes, List -> Toggle the item checkbox state), and it works as expected.

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

VZ

unread,

Sep 13, 2023, 7:15:56 AM9/13/23

to wx-...@googlegroups.com, Subscribed

Perhaps something prevents this event from being generated in the OP's case, but we need a way to reproduce the problem -- could you please try to make a patch to the same showing it, as usual? TIA!

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

Jorge Moraleda

unread,

Sep 14, 2023, 1:51:47 AM9/14/23

to wx-...@googlegroups.com, Subscribed

Thank you @MaartenBent and @vadz . I am working on finding a minimal example that triggers the bug. I will report back.

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

Jorge Moraleda

unread,

Sep 14, 2023, 9:18:35 PM9/14/23

to wx-...@googlegroups.com, Subscribed

This is an example that shows the problem.

I apologize because it is wxPython and not wxWidgets. If you think the problem I am observing is coming from the wxPython layer, I will close this issue and open one against wxPython, but I did not want to begin by double posting since I am aware that many developers read both.

To show the problem: Double click on any row, which invokes on_item_activated. This is working as expected in all platforms, as shown by the appropriate log line. Notice that this method invokes CheckItem. On Linux GTK this fires a EVT_LIST_ITEM_CHECKED which triggers a a call to on_item_checked as expected and can be seen in the log, but on MSW, on_item_checked is never called.

I point out that to trigger the problem I seem to need LC_VIRTUAL

I look forward to hearing your thoughts. Thank you!

import wx

class TestVirtualList(wx.ListCtrl):
    def __init__(self, parent):
        super().__init__(parent, -1,
            style=wx.LC_REPORT|wx.LC_VIRTUAL|wx.LC_HRULES|wx.LC_VRULES)

        # Set of indexes that are checked if CheckBoxes are enabled
        self.checked = set()
        self.EnableCheckBoxes()
        self.InsertColumn(0, "Column 0", width=400)

        self.SetItemCount(1000000)

        self.Bind(wx.EVT_LIST_ITEM_CHECKED, self._on_item_checked)
        self.Bind(wx.EVT_LIST_ITEM_UNCHECKED, self._on_item_unchecked)
        self.Bind(wx.EVT_LIST_ITEM_ACTIVATED, self._on_item_activated)

    def _on_item_checked(self, event):
        print(f"_on_item_checked for {event.Index}")
        self.checked.add(event.Index)
        event.Skip()

    def _on_item_unchecked(self, event):
        print(f"_on_item_unchecked for {event.Index}")
        self.checked.discard(event.Index)
        event.Skip()

    def _on_item_activated(self, event):
        print(f"_on_item_activated for {event.Index}")
        self.currentItemcurrentItem = event.Index
        print(" ---> Invoking CheckItem")

        toggleCheckBox = not event.Index in self.checked
        self.CheckItem(event.Index, check=toggleCheckBox) # THIS DOES NOTHING ON MSW ???

    def OnGetItemIsChecked(self, item):
        return item in self.checked

    def OnGetItemText(self, item, col):
        return f"Item {item}, column {col}"

class TestFrame(wx.Frame):
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)

        sizer = wx.BoxSizer(wx.VERTICAL)

        self.list = TestVirtualList(self)
        sizer.Add(self.list, 1, wx.EXPAND)

        self.SetSizer(sizer)
        self.SetAutoLayout(True)

app = wx.App()
TEST_FRAME = TestFrame(None, title="CheckItem test")
TEST_FRAME.Show()

app.MainLoop()

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

VZ

unread,

Sep 15, 2023, 8:00:44 AM9/15/23

to wx-...@googlegroups.com, Subscribed

I think CheckItem() shouldn't generate any events, according to the general rule that changing the state programmatically doesn't do it (with a lot of exceptions, unfortunately, e.g. wxTextCtrl::SetValue()), i.e. the fix would be to stop doing this in wxListMainWindow::CheckItem() and only generate this event when it's called from wxListMainWindow::OnMouse() (BTW, it seems like you can't check an item from keyboard in the generic version).

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

Maarten

unread,

Sep 15, 2023, 12:29:07 PM9/15/23

to wx-...@googlegroups.com, Subscribed

Thanks for the example. I don't use python, but I notice you use wxLC_VIRTUAL, and with that I am able to reproduce it in the c++ example.

When calling CheckItem() in wxLC_REPORT mode, the (msw) wxListCtrl automatically generates check/uncheck events. There is no difference between a user clicking the checkbox, or setting it with ListView_SetCheckState.

So I think in this case we should make the listctrl in wxLC_VIRTUAL mode behave the same.

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

Jorge Moraleda

unread,

Sep 15, 2023, 12:54:27 PM9/15/23

to wx-...@googlegroups.com, Subscribed

I notice you use wxLC_VIRTUAL, and with that I am able to reproduce it in the c++ example.

Yes. In my experiments I concluded the problem occurs only with wxLC_VIRTUAL.

If I understand correctly the logic of a list in virtual mode the only effect of CheckItem is to generate the EVT_LIST_ITEM_CHECKED or EVT_LIST_ITEM_UNCHECKED event so IMHO CheckItem should generate the events and be one of the exceptions to the general rule that changing the state programmatically doesn't generate events. Because otherwise CheckItem would be a useless noop with a list in virtual mode.

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

Maarten

unread,

Sep 15, 2023, 2:53:52 PM9/15/23

to wx-...@googlegroups.com, Subscribed

While checking the listctrl example in virtual mode, I also noticed another issue. It keeps generating wxEVT_LIST_CACHE_HINT events. This seems to be a regression of #23608. Restoring MSWDisableComposited(); fixes it.

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

VZ

unread,

Sep 16, 2023, 9:38:51 AM9/16/23

to wx-...@googlegroups.com, Subscribed

If I understand correctly the logic of a list in virtual mode the only effect of CheckItem is to generate the EVT_LIST_ITEM_CHECKED or EVT_LIST_ITEM_UNCHECKED event

I'm not so sure about it, in principle the correct way to check an item in virtual mode is to change the value returned from OnGetItemIsChecked() for this item and then refresh it, just as for the item text.

so IMHO CheckItem in MSW should generate the events and be one of the exceptions to the general rule that changing the state programmatically doesn't generate events. Because otherwise CheckItem would be a useless noop with a list in virtual mode. (CheckItem in the generic control, in virtual mode, does generate the events and already works as I think it should).

But the argument that it already does generate these events is a strong one... If we do it like this, we really must document this behaviour and, ideally, also update the test suite to check for it.

BTW, it seems like you can't check an item from keyboard in the generic version

I just tested this, and I can reproduce it: In my example, in MSW, pressing space triggers on_item_activated event but on GTK it does not. What should we do about this? Fix it as part of this issue? Open a separate issue?

Probably open a separate issue, thanks.

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

Jorge Moraleda

unread,

Sep 16, 2023, 2:26:07 PM9/16/23

to wx-...@googlegroups.com, Subscribed

If I understand correctly the logic of a list in virtual mode the only effect of CheckItem is to generate the EVT_LIST_ITEM_CHECKED or EVT_LIST_ITEM_UNCHECKED event

I'm not so sure about it, in principle the correct way to check an item in virtual mode is to change the value returned from OnGetItemIsChecked() for this item and then refresh it, just as for the item text.

Agreed. In virtual mode the base does not know how to change the value that OnGetItemIsChecked() so, the way I see it our two choices are that either CheckItem() in the base class does nothing, or it fires EVT_LIST_ITEM_CHECKED or EVT_LIST_ITEM_UNCHECKED so the callback can change the list model so that OnGetItemIsChecked() returns the correct value.

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

VZ

unread,

Sep 30, 2023, 5:15:07 PM9/30/23

to wx-...@googlegroups.com, Subscribed

Closed #23869 as completed via 07df143.

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

Reply all

Reply to author

Forward