Improve selection and focus events generation in wxGenericLisCtrl

Avoid sending spurious wxEVT_LIST_ITEM_{FOCUSED, SELECTED, DESELECTED}
events and make the generic version consistent with the behaviour of the
native wxMSW one.

Also add/extend the tests and slightly improve the sample.

Closes https://github.com/wxWidgets/wxWidgets/pull/2044

include/wx/generic/private/listctrl.h

@@ -519,14 +519,26 @@ public:
 
     // all these functions only do something if the line is currently visible
 
+    // Make sure that _line_ is the only item highlighted in the control.
+    // _oldLine_ is the old focused item.
+    void HighlightOnly( size_t line, size_t oldLine = (size_t)-1 );
+
+    // In multiple selection mode, instead of sending one notification per item
+    // (which is too slow if a lot of items are selected) we send only one notification
+    // for all of them which is the wxMSW behaviour. Currently done for virtual
+    // list controls and for deselection only.
+    enum SendEvent { SendEvent_None, SendEvent_Normal };
+
     // change the line "selected" state, return true if it really changed
-    bool HighlightLine( size_t line, bool highlight = true);
+    bool HighlightLine( size_t line, bool highlight = true,
+                        SendEvent sendEvent = SendEvent_Normal );
 
     // as HighlightLine() but do it for the range of lines: this is incredibly
     // more efficient for virtual list controls!
     //
     // NB: unlike HighlightLine() this one does refresh the lines on screen
-    void HighlightLines( size_t lineFrom, size_t lineTo, bool on = true );
+    void HighlightLines( size_t lineFrom, size_t lineTo, bool on = true,
+                         SendEvent sendEvent = SendEvent_Normal );
 
     // toggle the line state and refresh it
     void ReverseHighlight( size_t line )
@@ -753,6 +765,16 @@ public:
         return m_hasFocus;
     }
 
+    void UpdateSelectionCount(bool selected)
+    {
+        wxASSERT_MSG( !IsVirtual(), "Can be called for non virtual lists only" );
+
+        if ( IsSingleSel() )
+            return;
+
+        selected ? ++m_selCount : --m_selCount;
+    }
+
 protected:
     // the array of all line objects for a non virtual list control (for the
     // virtual list control we only ever use m_lines[0])
@@ -803,11 +825,18 @@ protected:
            m_lineBeforeLastClicked,
            m_lineSelectSingleOnUp;
 
+    // Multiple selection extends from the anchor. Not used in single-selection mode.
+    size_t m_anchor;
+
     bool m_hasCheckBoxes;
 
 protected:
     wxWindow *GetMainWindowOfCompositeControl() wxOVERRIDE { return GetParent(); }
 
+    // the total count of items selected in a non virtual list control with
+    // multiple selections (always 0 otherwise)
+    size_t m_selCount;
+
     // the total count of items in a virtual list control
     size_t m_countVirt;
 
@@ -858,6 +887,24 @@ private:
     // initialize the current item if needed
     void UpdateCurrent();
 
+    // change the current (== focused) item, without sending any event
+    // return true if m_current really changed.
+    bool ChangeCurrentWithoutEvent(size_t current);
+
+    // Trying to activate the current item from keyboard is only possible
+    // if it is actually selected. We don't send wxEVT_LIST_ITEM_ACTIVATED
+    // event if it is not, and wxEVT_LIST_KEY_DOWN event should carry -1
+    // in this case, as the wxMSW implementation does.
+    bool ShouldSendEventForCurrent() const
+    {
+        return HasCurrent() && IsHighlighted(m_current);
+    }
+
+    // For multiple selection mode.
+    // Change the selected range from [anchor, oldCurrent] to [anchor, newCurrent]
+    // without generating unnecessary wxEVT_LIST_ITEM_{DE}SELECTED events.
+    void ExtendSelection(size_t oldCurrent, size_t newCurrent);
+
     // delete all items but don't refresh: called from dtor
     void DoDeleteAllItems();
 

samples/listctrl/listtest.cpp

@@ -564,6 +564,15 @@ void MyFrame::InitWithReportItems()
     itemCol.SetAlign(wxLIST_FORMAT_RIGHT);
     m_listCtrl->InsertColumn(2, itemCol);
 
+    if ( m_numListItems <= 0 )
+    {
+        m_listCtrl->SetColumnWidth( 0, 100 );
+        m_listCtrl->SetColumnWidth( 1, wxLIST_AUTOSIZE );
+        m_listCtrl->SetColumnWidth( 2, wxLIST_AUTOSIZE_USEHEADER );
+
+        return;
+    }
+
     // to speed up inserting we hide the control temporarily
     m_listCtrl->Hide();
 
@@ -584,25 +593,38 @@ void MyFrame::InitWithReportItems()
     item.SetTextColour(*wxRED);
     m_listCtrl->SetItem( item );
 
-    item.m_itemId = 2;
-    item.SetTextColour(*wxGREEN);
-    m_listCtrl->SetItem( item );
-    item.m_itemId = 4;
-    item.SetTextColour(*wxLIGHT_GREY);
-    item.SetFont(*wxITALIC_FONT);
-    item.SetBackgroundColour(*wxRED);
-    m_listCtrl->SetItem( item );
+    if ( m_numListItems > 2 )
+    {
+        item.m_itemId = 2;
+        item.SetTextColour(*wxGREEN);
+        m_listCtrl->SetItem( item );
+    }
+
+    if ( m_numListItems > 4 )
+    {
+        item.m_itemId = 4;
+        item.SetTextColour(*wxLIGHT_GREY);
+        item.SetFont(*wxITALIC_FONT);
+        item.SetBackgroundColour(*wxRED);
+        m_listCtrl->SetItem( item );
+    }
 
     m_listCtrl->SetTextColour(*wxBLUE);
 
-    // Set images in columns
-    m_listCtrl->SetItemColumnImage(1, 1, 0);
+    if ( m_numListItems > 1 )
+    {
+        // Set images in columns
+        m_listCtrl->SetItemColumnImage(1, 1, 0);
+    }
 
-    wxListItem info;
-    info.SetImage(0);
-    info.SetId(3);
-    info.SetColumn(2);
-    m_listCtrl->SetItem(info);
+    if ( m_numListItems > 3 )
+    {
+        wxListItem info;
+        info.SetImage(0);
+        info.SetId(3);
+        info.SetColumn(2);
+        m_listCtrl->SetItem(info);
+    }
 
     // test SetItemFont too
     m_listCtrl->SetItemFont(0, *wxITALIC_FONT);
@@ -1073,6 +1095,11 @@ void MyListCtrl::OnColClick(wxListEvent& event)
 {
     int col = event.GetColumn();
 
+    if ( col == -1 )
+    {
+        return; // clicked outside any column.
+    }
+
     // set or unset image
     static bool x = false;
     x = !x;
@@ -1409,8 +1436,6 @@ void MyListCtrl::OnListKeyDown(wxListEvent& event)
             wxFALLTHROUGH;
 
         default:
-            LogEvent(event, "OnListKeyDown");
-
             event.Skip();
     }
 }

tests/controls/listbasetest.cpp

@@ -26,6 +26,7 @@
 #include "wx/uiaction.h"
 #include "wx/imaglist.h"
 #include "wx/artprov.h"
+#include "wx/stopwatch.h"
 
 void ListBaseTestCase::ColumnsOrder()
 {
@@ -177,6 +178,123 @@ void ListBaseTestCase::ChangeMode()
     CPPUNIT_ASSERT_EQUAL( "First", list->GetItemText(0) );
 }
 
+#ifdef __WXGTK__
+    #define wxGTK_TIMED_YIELD(t) \
+        if ( !IsRunningUnderXVFB() ) \
+            for ( wxStopWatch sw; sw.Time() < t; ) wxYield()
+#else // !__WXGTK__
+    #define wxGTK_TIMED_YIELD(t)
+#endif // __WXGTK__
+
+void ListBaseTestCase::MultiSelect()
+{
+#if wxUSE_UIACTIONSIMULATOR
+
+#ifndef __WXMSW__
+    // FIXME: This test fails on Travis CI although works fine on
+    //        development machine, no idea why though!
+    if ( IsAutomaticTest() )
+        return;
+#endif // !__WXMSW__
+
+    wxListCtrl* const list = GetList();
+
+    EventCounter focused(list, wxEVT_LIST_ITEM_FOCUSED);
+    EventCounter selected(list, wxEVT_LIST_ITEM_SELECTED);
+    EventCounter deselected(list, wxEVT_LIST_ITEM_DESELECTED);
+
+    list->InsertColumn(0, "Header");
+
+    for ( int i = 0; i < 10; ++i )
+        list->InsertItem(i, wxString::Format("Item %d", i));
+
+    wxUIActionSimulator sim;
+
+    wxRect pos;
+    list->GetItemRect(2, pos); // Choose the third item as anchor
+
+    // We move in slightly so we are not on the edge
+    wxPoint point = list->ClientToScreen(pos.GetPosition()) + wxPoint(10, 10);
+
+    sim.MouseMove(point);
+    wxYield();
+
+    sim.MouseClick(); // select the anchor
+    wxYield();
+
+    wxGTK_TIMED_YIELD(50);
+
+    list->GetItemRect(5, pos);
+    point = list->ClientToScreen(pos.GetPosition()) + wxPoint(10, 10);
+
+    sim.MouseMove(point);
+    wxYield();
+
+    sim.KeyDown(WXK_SHIFT);
+    sim.MouseClick();
+    sim.KeyUp(WXK_SHIFT);
+    wxYield();
+
+    wxGTK_TIMED_YIELD(10);
+
+    // when the first item was selected the focus changes to it, but not
+    // on subsequent clicks
+    CPPUNIT_ASSERT_EQUAL(4, list->GetSelectedItemCount()); // item 2 to 5 (inclusive) are selected
+    CPPUNIT_ASSERT_EQUAL(2, focused.GetCount()); // count the focus which was on the anchor
+    CPPUNIT_ASSERT_EQUAL(4, selected.GetCount());
+    CPPUNIT_ASSERT_EQUAL(0, deselected.GetCount());
+
+    focused.Clear();
+    selected.Clear();
+    deselected.Clear();
+
+    sim.Char(WXK_END, wxMOD_SHIFT); // extend the selection to the last item
+    wxYield();
+
+    wxGTK_TIMED_YIELD(10);
+
+    CPPUNIT_ASSERT_EQUAL(8, list->GetSelectedItemCount()); // item 2 to 9 (inclusive) are selected
+    CPPUNIT_ASSERT_EQUAL(1, focused.GetCount()); // focus is on the last item
+    CPPUNIT_ASSERT_EQUAL(4, selected.GetCount()); // only newly selected items got the event
+    CPPUNIT_ASSERT_EQUAL(0, deselected.GetCount());
+
+    focused.Clear();
+    selected.Clear();
+    deselected.Clear();
+
+    sim.Char(WXK_HOME, wxMOD_SHIFT); // select from anchor to the first item
+    wxYield();
+
+    wxGTK_TIMED_YIELD(10);
+
+    CPPUNIT_ASSERT_EQUAL(3, list->GetSelectedItemCount()); // item 0 to 2 (inclusive) are selected
+    CPPUNIT_ASSERT_EQUAL(1, focused.GetCount()); // focus is on item 0
+    CPPUNIT_ASSERT_EQUAL(2, selected.GetCount()); // events are only generated for item 0 and 1
+    CPPUNIT_ASSERT_EQUAL(7, deselected.GetCount()); // item 2 (exclusive) to 9 are deselected
+
+    focused.Clear();
+    selected.Clear();
+    deselected.Clear();
+
+    list->EnsureVisible(0);
+    wxYield();
+
+    list->GetItemRect(2, pos);
+    point = list->ClientToScreen(pos.GetPosition()) + wxPoint(10, 10);
+
+    sim.MouseMove(point);
+    wxYield();
+
+    sim.MouseClick();
+    wxYield();
+
+    CPPUNIT_ASSERT_EQUAL(1, list->GetSelectedItemCount()); // anchor is the only selected item
+    CPPUNIT_ASSERT_EQUAL(1, focused.GetCount()); // because the focus changed from item 0 to anchor
+    CPPUNIT_ASSERT_EQUAL(0, selected.GetCount()); // anchor is already in selection state
+    CPPUNIT_ASSERT_EQUAL(2, deselected.GetCount()); // items 0 and 1 are deselected
+#endif // wxUSE_UIACTIONSIMULATOR
+}
+
 void ListBaseTestCase::ItemClick()
 {
 #if wxUSE_UIACTIONSIMULATOR
@@ -236,16 +354,9 @@ void ListBaseTestCase::ItemClick()
 
     // when the first item was selected the focus changes to it, but not
     // on subsequent clicks
-
-    // FIXME: This test fail under wxGTK & wxOSX because we get 3 FOCUSED events and
-    //        2 SELECTED ones instead of the one of each we expect for some
-    //        reason, this needs to be debugged as it may indicate a bug in the
-    //        generic wxListCtrl implementation.
-#ifndef _WX_GENERIC_LISTCTRL_H_
     CPPUNIT_ASSERT_EQUAL(1, focused.GetCount());
     CPPUNIT_ASSERT_EQUAL(1, selected.GetCount());
     CPPUNIT_ASSERT_EQUAL(1, deselected.GetCount());
-#endif
     CPPUNIT_ASSERT_EQUAL(1, activated.GetCount());
     CPPUNIT_ASSERT_EQUAL(1, rclick.GetCount());
 #endif // wxUSE_UIACTIONSIMULATOR

tests/controls/listbasetest.h

@@ -26,6 +26,7 @@ protected:
         CPPUNIT_TEST( ChangeMode ); \
         WXUISIM_TEST( ItemClick ); \
         WXUISIM_TEST( KeyDown ); \
+        WXUISIM_TEST( MultiSelect ); \
         CPPUNIT_TEST( DeleteItems ); \
         CPPUNIT_TEST( InsertItem ); \
         CPPUNIT_TEST( Find ); \
@@ -40,6 +41,7 @@ protected:
     void ItemRect();
     void ItemText();
     void ChangeMode();
+    void MultiSelect();
     void ItemClick();
     void KeyDown();
     void DeleteItems();

tests/controls/listviewtest.cpp

@@ -20,6 +20,7 @@
 
 #include "wx/listctrl.h"
 #include "listbasetest.h"
+#include "testableframe.h"
 
 class ListViewTestCase : public ListBaseTestCase, public CppUnit::TestCase
 {
@@ -61,7 +62,8 @@ void ListViewTestCase::setUp()
 
 void ListViewTestCase::tearDown()
 {
-    wxDELETE(m_list);
+    DeleteTestWindow(m_list);
+    m_list = NULL;
 }
 
 void ListViewTestCase::Selection()
@@ -104,6 +106,8 @@ void ListViewTestCase::Selection()
 
 void ListViewTestCase::Focus()
 {
+    EventCounter focused(m_list, wxEVT_LIST_ITEM_FOCUSED);
+
     m_list->InsertColumn(0, "Column 0");
 
     m_list->InsertItem(0, "Item 0");
@@ -111,10 +115,12 @@ void ListViewTestCase::Focus()
     m_list->InsertItem(2, "Item 2");
     m_list->InsertItem(3, "Item 3");
 
+    CPPUNIT_ASSERT_EQUAL(0, focused.GetCount());
     CPPUNIT_ASSERT_EQUAL(-1, m_list->GetFocusedItem());
 
     m_list->Focus(0);
 
+    CPPUNIT_ASSERT_EQUAL(1, focused.GetCount());
     CPPUNIT_ASSERT_EQUAL(0, m_list->GetFocusedItem());
 }