Don't send SELECTED events for an already selected item in wxGTK wxListBox.

Bring wxGTK in line with wxMSW behaviour and avoid sending the
wxEVT_COMMAND_LISTBOX_SELECTED events when the user clicks on an already
selected item.

Refactor wxMSW code to extract the logic to avoid such events into a reusable
in other ports wxListBoxBase::DoChangeSingleSelection() function. Also add
wxListBox::GTKOnSelectionChanged() to wxGTK to avoid having to make the new
function public just so that it could be called by GTK callback and make the
previously existing CalcAndSendEvent() protected as well.

This fixes a unit test failure in ListBoxTestCase::ClickEvents() under wxGTK.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@65935 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

diff --git a/include/wx/gtk/listbox.h b/include/wx/gtk/listbox.h
index 45b330c515c834a7b86794fb6bd3d70b9a136344..707665eaadcf1e36e8f56699d7a25c64f82b58bb 100644 (file)
--- a/include/wx/gtk/listbox.h
+++ b/include/wx/gtk/listbox.h
@@ -100,6 +100,8 @@ public:
     void GTKDisableEvents();
     void GTKEnableEvents();
 
     void GTKDisableEvents();
     void GTKEnableEvents();
 

+    void GTKOnSelectionChanged();
+

 protected:
     virtual void DoClear();
     virtual void DoDeleteOneItem(unsigned int n);
 protected:
     virtual void DoClear();
     virtual void DoDeleteOneItem(unsigned int n);

diff --git a/include/wx/listbox.h b/include/wx/listbox.h
index d2332d5d132a011f3f51751712f46534229b3237..f023f6fa65d8b3a73c0e87604a04717d2d542f83 100644 (file)
--- a/include/wx/listbox.h
+++ b/include/wx/listbox.h
@@ -49,7 +49,7 @@ public:
 
     // multiple selection logic
     virtual bool IsSelected(int n) const = 0;
 
     // multiple selection logic
     virtual bool IsSelected(int n) const = 0;

-    virtual void SetSelection(int n) { DoSetSelection(n, true); }
+    virtual void SetSelection(int n);

     void SetSelection(int n, bool select) { DoSetSelection(n, select); }
     void Deselect(int n) { DoSetSelection(n, false); }
     void DeselectAll(int itemToLeaveSelected = -1);
     void SetSelection(int n, bool select) { DoSetSelection(n, select); }
     void Deselect(int n) { DoSetSelection(n, false); }
     void DeselectAll(int itemToLeaveSelected = -1);


@@ -96,11 +96,6 @@ public:
     int HitTest(int x, int y) const { return DoListHitTest(wxPoint(x, y)); }
 
 
     int HitTest(int x, int y) const { return DoListHitTest(wxPoint(x, y)); }
 
 

-    // For generating events in multiple and extended mode: compare the current
-    // selections with the previously recorded ones (in m_oldSelections) and
-    // send the appropriate event if they differ, otherwise just return false.
-    bool CalcAndSendEvent();
-

 protected:
     virtual void DoSetFirstItem(int n) = 0;
 
 protected:
     virtual void DoSetFirstItem(int n) = 0;
 


@@ -110,6 +105,17 @@ protected:
     virtual int DoListHitTest(const wxPoint& WXUNUSED(point)) const
         { return wxNOT_FOUND; }
 
     virtual int DoListHitTest(const wxPoint& WXUNUSED(point)) const
         { return wxNOT_FOUND; }
 

+    // Helper for the code generating events in single selection mode: updates
+    // m_oldSelections and return true if the selection really changed.
+    // Otherwise just returns false.
+    bool DoChangeSingleSelection(int item);
+
+    // Helper for generating events in multiple and extended mode: compare the
+    // current selections with the previously recorded ones (in
+    // m_oldSelections) and send the appropriate event if they differ,
+    // otherwise just return false.
+    bool CalcAndSendEvent();
+

     // Send a listbox (de)selection or double click event.
     //
     // Returns true if the event was processed.
     // Send a listbox (de)selection or double click event.
     //
     // Returns true if the event was processed.


@@ -118,6 +124,9 @@ protected:
     // Array storing the indices of all selected items that we already notified
     // the user code about for multi selection list boxes.
     //
     // Array storing the indices of all selected items that we already notified
     // the user code about for multi selection list boxes.
     //

+    // For single selection list boxes, we reuse this array to store the single
+    // currently selected item, this is used by DoChangeSingleSelection().
+    //

     // TODO-OPT: wxSelectionStore would be more efficient for big list boxes.
     wxArrayInt m_oldSelections;
 
     // TODO-OPT: wxSelectionStore would be more efficient for big list boxes.
     wxArrayInt m_oldSelections;
 

diff --git a/src/common/lboxcmn.cpp b/src/common/lboxcmn.cpp
index 8787130451b1d615ddcc6cf04a15ff376dddfc09..5d7abb469f11e32bd9025f214c63b95addf6be0e 100644 (file)
--- a/src/common/lboxcmn.cpp
+++ b/src/common/lboxcmn.cpp
@@ -59,6 +59,14 @@ bool wxListBoxBase::SetStringSelection(const wxString& s, bool select)
     return true;
 }
 
     return true;
 }
 

+void wxListBoxBase::SetSelection(int n)
+{
+    if ( !HasMultipleSelection() )
+        DoChangeSingleSelection(n);
+
+    DoSetSelection(n, true);
+}
+

 void wxListBoxBase::DeselectAll(int itemToLeaveSelected)
 {
     if ( HasMultipleSelection() )
 void wxListBoxBase::DeselectAll(int itemToLeaveSelected)
 {
     if ( HasMultipleSelection() )


@@ -114,6 +122,24 @@ bool wxListBoxBase::SendEvent(wxEventType evtType, int item, bool selected)
     return HandleWindowEvent(event);
 }
 
     return HandleWindowEvent(event);
 }
 

+bool wxListBoxBase::DoChangeSingleSelection(int item)
+{
+    // As we don't use m_oldSelections in single selection mode, we store the
+    // last item that we notified the user about in it in this case because we
+    // need to remember it to be able to filter out the dummy selection changes
+    // that we get when the user clicks on an already selected item.
+    if ( !m_oldSelections.empty() && *m_oldSelections.begin() == item )
+    {
+        // Same item as the last time.
+        return false;
+    }
+
+    m_oldSelections.clear();
+    m_oldSelections.push_back(item);
+
+    return true;
+}
+

 bool wxListBoxBase::CalcAndSendEvent()
 {
     wxArrayInt selections;
 bool wxListBoxBase::CalcAndSendEvent()
 {
     wxArrayInt selections;

diff --git a/src/gtk/listbox.cpp b/src/gtk/listbox.cpp
index 5260617949456317ad10edd84c9a2891eed65280..19d4beb5a6e0bf9109890f2b8f6134365876545b 100644 (file)
--- a/src/gtk/listbox.cpp
+++ b/src/gtk/listbox.cpp
@@ -119,49 +119,9 @@ gtk_listitem_changed_callback(GtkTreeSelection * WXUNUSED(selection),
 {
     if (g_blockEventsOnDrag) return;
 
 {
     if (g_blockEventsOnDrag) return;
 

-    if (listbox->HasFlag(wxLB_MULTIPLE | wxLB_EXTENDED))
-    {
-        listbox->CalcAndSendEvent();
-    }
-    else // single selection
-    {
-        wxCommandEvent event(wxEVT_COMMAND_LISTBOX_SELECTED, listbox->GetId() );
-        event.SetEventObject( listbox );
-
-        int index = listbox->GetSelection();
-        if (index == wxNOT_FOUND)
-        {
-            // indicate that this is a deselection
-            event.SetExtraLong( 0 );
-            event.SetInt( -1 );
-
-            listbox->HandleWindowEvent( event );
-
-            return;
-        }
-        else
-        {
-            GtkTreeEntry* entry = listbox->GTKGetEntry( index );
-
-            // indicate that this is a selection
-            event.SetExtraLong( 1 );
-
-            event.SetInt( index );
-            event.SetString(wxConvUTF8.cMB2WX(gtk_tree_entry_get_label(entry)));
-
-            if ( listbox->HasClientObjectData() )
-                event.SetClientObject(
-                    (wxClientData*) gtk_tree_entry_get_userdata(entry)
-                                 );
-            else if ( listbox->HasClientUntypedData() )
-                event.SetClientData( gtk_tree_entry_get_userdata(entry) );
-
-            listbox->HandleWindowEvent( event );
-
-            g_object_unref (entry);
-        }
-    }
+    listbox->GTKOnSelectionChanged();

 }
 }

+

 }
 
 //-----------------------------------------------------------------------------
 }
 
 //-----------------------------------------------------------------------------


@@ -761,6 +721,55 @@ int wxListBox::FindString( const wxString &item, bool bCase ) const
 // selection
 // ----------------------------------------------------------------------------
 
 // selection
 // ----------------------------------------------------------------------------
 

+void wxListBox::GTKOnSelectionChanged()
+{
+    if ( HasFlag(wxLB_MULTIPLE | wxLB_EXTENDED) )
+    {
+        CalcAndSendEvent();
+    }
+    else // single selection
+    {
+        const int index = GetSelection();
+        if ( !DoChangeSingleSelection(index) )
+            return;
+
+        wxCommandEvent event(wxEVT_COMMAND_LISTBOX_SELECTED, GetId() );
+        event.SetEventObject( this );
+
+        if (index == wxNOT_FOUND)
+        {
+            // indicate that this is a deselection
+            event.SetExtraLong( 0 );
+            event.SetInt( -1 );
+
+            HandleWindowEvent( event );
+
+            return;
+        }
+        else
+        {
+            GtkTreeEntry* entry = GTKGetEntry( index );
+
+            // indicate that this is a selection
+            event.SetExtraLong( 1 );
+
+            event.SetInt( index );
+            event.SetString(wxConvUTF8.cMB2WX(gtk_tree_entry_get_label(entry)));
+
+            if ( HasClientObjectData() )
+                event.SetClientObject(
+                    (wxClientData*) gtk_tree_entry_get_userdata(entry)
+                                 );
+            else if ( HasClientUntypedData() )
+                event.SetClientData( gtk_tree_entry_get_userdata(entry) );
+
+            HandleWindowEvent( event );
+
+            g_object_unref (entry);
+        }
+    }
+}
+

 int wxListBox::GetSelection() const
 {
     wxCHECK_MSG( m_treeview != NULL, wxNOT_FOUND, wxT("invalid listbox"));
 int wxListBox::GetSelection() const
 {
     wxCHECK_MSG( m_treeview != NULL, wxNOT_FOUND, wxT("invalid listbox"));

diff --git a/src/msw/listbox.cpp b/src/msw/listbox.cpp
index 53d60ce89bc54ed8f04e3c967ccb71a9d84f29dc..52fe4d0c4d5b1d3475418eac61f5693bf8e8fa79 100644 (file)
--- a/src/msw/listbox.cpp
+++ b/src/msw/listbox.cpp
@@ -689,20 +689,10 @@ bool wxListBox::MSWCommand(WXUINT param, WXWORD WXUNUSED(id))
     if ( n == wxNOT_FOUND )
        return false;
 
     if ( n == wxNOT_FOUND )
        return false;
 

-    // As we don't use m_oldSelections in single selection mode, we store the
-    // last item that we notified the user about in it in this case because we
-    // need to remember it to be able to filter out the dummy LBN_SELCHANGE
-    // messages that we get when the user clicks on an already selected item.

     if ( param == LBN_SELCHANGE )
     {
     if ( param == LBN_SELCHANGE )
     {

-        if ( !m_oldSelections.empty() && *m_oldSelections.begin() == n )
-        {
-            // Same item as the last time.
+        if ( !DoChangeSingleSelection(n) )

             return false;
             return false;

-        }
-
-        m_oldSelections.clear();
-        m_oldSelections.push_back(n);

     }
 
     // Do generate an event otherwise.
     }
 
     // Do generate an event otherwise.