Merge branch 'listctrl-check-events'

Make events sent by wxListCtrl::CheckItem() consistent across all
platforms.

See #27154.

docs/changes.txt

@@ -169,6 +169,10 @@ Changes in behaviour not resulting in compilation errors
   in progress, if any. Call SetIMEInteraction(wxSTC_IME_WINDOWED) to restore
   the previous behaviour if necessary.
 
+- Generic wxListCtrl (used in wxGTK and wxOSX) CheckItem() now only sends
+  wxEVT_LIST_ITEM_[UN]CHECKED events when the state really changed, for
+  consistency with wxMSW.
+
 
 Changes in behaviour which may result in build errors
 -----------------------------------------------------

interface/wx/listctrl.h

@@ -1433,6 +1433,11 @@ public:
         @c EVT_LIST_ITEM_CHECKED and @c EVT_LIST_ITEM_UNCHECKED events. See
         OnGetItemIsChecked() for information on how to update the checkbox state.
 
+        For a non-virtual control, @c EVT_LIST_ITEM_CHECKED or @c
+        EVT_LIST_ITEM_UNCHECKED event is generated only if the state of the
+        item actually changes (i.e. nothing happens if @a check is the same as
+        the value returned by IsItemChecked()).
+
         @param item Item (zero-based) index.
         @param check If @true, check the item, otherwise uncheck.
 

src/generic/listctrl.cpp

@@ -4071,6 +4071,12 @@ void wxListMainWindow::CheckItem(long item, bool state)
     if ( !IsVirtual() )
     {
         wxListLineData* line = GetLine((size_t)item);
+
+        // Don't send any events if nothing changes, for consistency with
+        // wxMSW, where the native control doesn't send them in this case.
+        if ( line->IsChecked() == state )
+            return;
+
         line->Check(state);
 
         RefreshLine(item);

src/qt/listctrl.cpp

@@ -271,16 +271,8 @@ public:
 
         if ( role == Qt::CheckStateRole && col == 0 )
         {
-            m_rows[row].m_checked =
-                static_cast<Qt::CheckState>(value.toUInt()) == Qt::Checked;
-
-            wxListEvent event;
-            InitListEvent(event,
-                          m_listCtrl,
-                          m_rows[row].m_checked ? wxEVT_LIST_ITEM_CHECKED
-                                                : wxEVT_LIST_ITEM_UNCHECKED,
-                          index);
-            m_listCtrl->HandleWindowEvent(event);
+            CheckItem(row,
+                      static_cast<Qt::CheckState>(value.toUInt()) == Qt::Checked);
             return true;
         }
 
@@ -735,14 +727,19 @@ public:
 
     bool IsItemChecked(long item) const
     {
-        wxCHECK_MSG(item >= 0 && item <= wxSsize(m_rows), false, "Invalid row");
+        wxCHECK_MSG(item >= 0 && item < wxSsize(m_rows), false, "Invalid row");
 
         return m_rows[item].m_checked;
     }
 
     void CheckItem(long item, bool check)
     {
-        wxCHECK_RET(item >= 0 && item <= wxSsize(m_rows), "Invalid row");
+        wxCHECK_RET(item >= 0 && item < wxSsize(m_rows), "Invalid row");
+
+        // Don't send any events if nothing changes, for consistency with
+        // wxMSW, where the native control doesn't send them in this case.
+        if ( m_rows[item].m_checked == check )
+            return;
 
         m_rows[item].m_checked = check;
 
@@ -750,6 +747,14 @@ public:
         roles.push_back(Qt::CheckStateRole);
         const QModelIndex modelIndex = index(item, 0);
         dataChanged(modelIndex, modelIndex, roles);
+
+        wxListEvent event;
+        InitListEvent(event,
+                      m_listCtrl,
+                      check ? wxEVT_LIST_ITEM_CHECKED
+                            : wxEVT_LIST_ITEM_UNCHECKED,
+                      modelIndex);
+        m_listCtrl->HandleWindowEvent(event);
     }
 
     virtual bool IsVirtual() const

tests/controls/listctrltest.cpp

@@ -154,6 +154,117 @@ TEST_CASE_METHOD(ListCtrlTestCase, "ListCtrl::ColumnCount", "[listctrl]")
     CHECK(m_list->GetColumnCount() == 0);
 }
 
+TEST_CASE_METHOD(ListCtrlTestCase, "ListCtrl::CheckItem", "[listctrl]")
+{
+    if ( !m_list->EnableCheckBoxes() )
+        return;
+
+    m_list->InsertColumn(0, "Column 0");
+    m_list->InsertItem(0, "Item 0");
+
+    EventCounter checked(m_list, wxEVT_LIST_ITEM_CHECKED);
+    EventCounter unchecked(m_list, wxEVT_LIST_ITEM_UNCHECKED);
+
+    CHECK( !m_list->IsItemChecked(0) );
+
+    // Unchecking an unchecked item doesn't do anything.
+    m_list->CheckItem(0, false);
+    CHECK( !m_list->IsItemChecked(0) );
+    CHECK( checked.GetCount() == 0 );
+    CHECK( unchecked.GetCount() == 0 );
+
+    m_list->CheckItem(0, true);
+    CHECK( m_list->IsItemChecked(0) );
+    CHECK( checked.GetCount() == 1 );
+    CHECK( unchecked.GetCount() == 0 );
+    checked.Clear();
+
+    // And neither does checking an already checked one.
+    m_list->CheckItem(0, true);
+    CHECK( m_list->IsItemChecked(0) );
+    CHECK( checked.GetCount() == 0 );
+    CHECK( unchecked.GetCount() == 0 );
+
+    m_list->CheckItem(0, false);
+    CHECK( !m_list->IsItemChecked(0) );
+    CHECK( checked.GetCount() == 0 );
+    CHECK( unchecked.GetCount() == 1 );
+}
+
+// wxQt doesn't support virtual list controls with checkboxes currently.
+#ifndef __WXQT__
+
+TEST_CASE_METHOD(ListCtrlTestCase, "ListCtrl::CheckItemVirtual", "[listctrl]")
+{
+    // Virtual list control storing the checked state of its single item
+    // itself, as a real application would do.
+    //
+    // Note that we can't use EventCounter here because it doesn't skip the
+    // command events, so our handlers wouldn't be called if we did, and we
+    // count the events ourselves instead.
+    class CheckableVirtualListCtrl : public wxListCtrl
+    {
+    public:
+        CheckableVirtualListCtrl()
+            : wxListCtrl(wxTheApp->GetTopWindow(), wxID_ANY,
+                         wxDefaultPosition, wxDefaultSize,
+                         wxLC_REPORT | wxLC_VIRTUAL)
+        {
+            Bind(wxEVT_LIST_ITEM_CHECKED, [this](wxListEvent&)
+                {
+                    m_checked = true;
+                    m_checkedCount++;
+                });
+            Bind(wxEVT_LIST_ITEM_UNCHECKED, [this](wxListEvent&)
+                {
+                    m_checked = false;
+                    m_uncheckedCount++;
+                });
+        }
+
+        wxString OnGetItemText(long, long) const override { return "Item"; }
+        bool OnGetItemIsChecked(long) const override { return m_checked; }
+
+        int m_checkedCount = 0;
+        int m_uncheckedCount = 0;
+
+    private:
+        bool m_checked = false;
+    };
+
+    delete m_list;
+    auto* const list = new CheckableVirtualListCtrl();
+    m_list = list;
+
+    if ( !m_list->EnableCheckBoxes() )
+        return;
+
+    m_list->InsertColumn(0, "Column 0");
+    m_list->SetItemCount(1);
+
+    CHECK( !m_list->IsItemChecked(0) );
+
+    m_list->CheckItem(0, true);
+    CHECK( m_list->IsItemChecked(0) );
+    CHECK( list->m_checkedCount == 1 );
+    CHECK( list->m_uncheckedCount == 0 );
+
+    // Unlike for non-virtual controls, the events are always generated for
+    // the virtual ones, even if the state doesn't change, as it's not stored
+    // by the control itself.
+    m_list->CheckItem(0, true);
+    CHECK( m_list->IsItemChecked(0) );
+    CHECK( list->m_checkedCount == 2 );
+    CHECK( list->m_uncheckedCount == 0 );
+
+    m_list->CheckItem(0, false);
+    CHECK( !m_list->IsItemChecked(0) );
+    CHECK( list->m_checkedCount == 2 );
+    CHECK( list->m_uncheckedCount == 1 );
+}
+
+#endif // __WXQT__
+
 #if wxUSE_UIACTIONSIMULATOR
 
 TEST_CASE_METHOD(ListCtrlTestCase, "ListCtrl::ColumnDrag", "[listctrl]")