Make wxListCtrl::CheckItem() event generation consistent

Previously, wxMSW native version only sent wxEVT_LIST_ITEM_[UN]CHECKED
if the item state has really changed, while the generic version, used in
wxGTK and wxOSX, did it unconditionally and wxQt native version never
did it at all.

Make all ports behave as wxMSW, which seems like the most useful
behaviour and add a unit test checking that this behaves as expected.

Note that virtual controls still always send the events in all ports, as
they work as notifications for the model in their case, and a new test
checks that this is the case in all ports (except wxQt which doesn't
support them).

docs/changes.txt

@@ -160,6 +160,10 @@ Changes in behaviour not resulting in compilation errors
   button that was pressed. Check the button ID and call Skip() if you want the
   the default handling to take place.
 
+- 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;
         }
 
@@ -744,12 +736,25 @@ public:
     {
         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;
 
         QVector<int> roles;
         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]")