fix bug in generic wxTreeCtrl: calling SelectItem(true) on an already selected item shouldn't toggle its selection (see #10830)

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

src/generic/treectlg.cpp

@@ -2146,15 +2146,16 @@ void wxGenericTreeCtrl::DoSelectItem(const wxTreeItemId& itemId,
 
 void wxGenericTreeCtrl::SelectItem(const wxTreeItemId& itemId, bool select)
 {
+    wxGenericTreeItem * const item = (wxGenericTreeItem*) itemId.m_pItem;
+    wxCHECK_RET( item, wxT("SelectItem(): invalid tree item") );
+
     if ( select )
     {
-        DoSelectItem(itemId, !HasFlag(wxTR_MULTIPLE));
+        if ( !item->IsSelected() )
+            DoSelectItem(itemId, !HasFlag(wxTR_MULTIPLE));
     }
     else // deselect
     {
-        wxGenericTreeItem *item = (wxGenericTreeItem*) itemId.m_pItem;
-        wxCHECK_RET( item, wxT("SelectItem(): invalid tree item") );
-
         wxTreeEvent event(wxEVT_COMMAND_TREE_SEL_CHANGING, this, item);
         if ( GetEventHandler()->ProcessEvent( event ) && !event.IsAllowed() )
             return;

tests/controls/treectrltest.cpp

@@ -38,17 +38,31 @@ public:
 private:
     CPPUNIT_TEST_SUITE( TreeCtrlTestCase );
         CPPUNIT_TEST( HasChildren );
+        CPPUNIT_TEST( SelectItemSingle );
+        CPPUNIT_TEST( PseudoTest_MultiSelect );
+        CPPUNIT_TEST( SelectItemMulti );
         CPPUNIT_TEST( PseudoTest_SetHiddenRoot );
         CPPUNIT_TEST( HasChildren );
     CPPUNIT_TEST_SUITE_END();
 
     void HasChildren();
+    void SelectItemSingle();
+    void SelectItemMulti();
+    void PseudoTest_MultiSelect() { ms_multiSelect = true; }
     void PseudoTest_SetHiddenRoot() { ms_hiddenRoot = true; }
 
+    static bool ms_multiSelect;
     static bool ms_hiddenRoot;
 
+    // the tree control itself
     wxTreeCtrl *m_tree;
 
+    // and some of its items
+    wxTreeItemId m_root,
+                 m_child1,
+                 m_child2,
+                 m_grandchild;
+
     DECLARE_NO_COPY_CLASS(TreeCtrlTestCase)
 };
 
@@ -62,19 +76,34 @@ CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( TreeCtrlTestCase, "TreeCtrlTestCase" );
 // test initialization
 // ----------------------------------------------------------------------------
 
+bool TreeCtrlTestCase::ms_multiSelect = false;
 bool TreeCtrlTestCase::ms_hiddenRoot = false;
 
 void TreeCtrlTestCase::setUp()
 {
     m_tree = new wxTreeCtrl(wxTheApp->GetTopWindow());
+
+    if ( ms_multiSelect )
+        m_tree->ToggleWindowStyle(wxTR_MULTIPLE);
+
     if ( ms_hiddenRoot )
         m_tree->ToggleWindowStyle(wxTR_HIDE_ROOT); // actually set it
+
+    m_root = m_tree->AddRoot("root");
+    m_child1 = m_tree->AppendItem(m_root, "child1");
+    m_child2 = m_tree->AppendItem(m_root, "child2");
+    m_grandchild = m_tree->AppendItem(m_child1, "grandchild");
 }
 
 void TreeCtrlTestCase::tearDown()
 {
     delete m_tree;
     m_tree = NULL;
+
+    m_root =
+    m_child1 =
+    m_child2 =
+    m_grandchild = wxTreeItemId();
 }
 
 // ----------------------------------------------------------------------------
@@ -83,14 +112,73 @@ void TreeCtrlTestCase::tearDown()
 
 void TreeCtrlTestCase::HasChildren()
 {
-    const wxTreeItemId root = m_tree->AddRoot("root");
+    CPPUNIT_ASSERT( m_tree->HasChildren(m_root) );
-    const wxTreeItemId child1 = m_tree->AppendItem(root, "child1");
+    CPPUNIT_ASSERT( m_tree->HasChildren(m_child1) );
-    const wxTreeItemId child2 = m_tree->AppendItem(root, "child2");
+    CPPUNIT_ASSERT( !m_tree->HasChildren(m_child2) );
-    const wxTreeItemId grandchild = m_tree->AppendItem(child1, "grandchild");
+    CPPUNIT_ASSERT( !m_tree->HasChildren(m_grandchild) );
-
+}
-    CPPUNIT_ASSERT( m_tree->HasChildren(root) );
+
-    CPPUNIT_ASSERT( m_tree->HasChildren(child1) );
+void TreeCtrlTestCase::SelectItemSingle()
-    CPPUNIT_ASSERT( !m_tree->HasChildren(child2) );
+{
-    CPPUNIT_ASSERT( !m_tree->HasChildren(grandchild) );
+    // this test should be only ran in single-selection control
+    CPPUNIT_ASSERT( !m_tree->HasFlag(wxTR_MULTIPLE) );
+
+    // initially nothing is selected
+    CPPUNIT_ASSERT( !m_tree->IsSelected(m_child1) );
+
+    // selecting an item should make it selected
+    m_tree->SelectItem(m_child1);
+    CPPUNIT_ASSERT( m_tree->IsSelected(m_child1) );
+
+    // selecting it again shouldn't change anything
+    m_tree->SelectItem(m_child1);
+    CPPUNIT_ASSERT( m_tree->IsSelected(m_child1) );
+
+    // selecting another item should switch the selection to it
+    m_tree->SelectItem(m_child2);
+    CPPUNIT_ASSERT( !m_tree->IsSelected(m_child1) );
+    CPPUNIT_ASSERT( m_tree->IsSelected(m_child2) );
+
+    // selecting it again still shouldn't change anything
+    m_tree->SelectItem(m_child2);
+    CPPUNIT_ASSERT( !m_tree->IsSelected(m_child1) );
+    CPPUNIT_ASSERT( m_tree->IsSelected(m_child2) );
+
+    // deselecting an item should remove the selection entirely
+    m_tree->UnselectItem(m_child2);
+    CPPUNIT_ASSERT( !m_tree->IsSelected(m_child1) );
+    CPPUNIT_ASSERT( !m_tree->IsSelected(m_child2) );
+}
+
+void TreeCtrlTestCase::SelectItemMulti()
+{
+    // this test should be only ran in multi-selection control
+    CPPUNIT_ASSERT( m_tree->HasFlag(wxTR_MULTIPLE) );
+
+    // initially nothing is selected
+    CPPUNIT_ASSERT( !m_tree->IsSelected(m_child1) );
+
+    // selecting an item should make it selected
+    m_tree->SelectItem(m_child1);
+    CPPUNIT_ASSERT( m_tree->IsSelected(m_child1) );
+
+    // selecting it again shouldn't change anything
+    m_tree->SelectItem(m_child1);
+    CPPUNIT_ASSERT( m_tree->IsSelected(m_child1) );
+
+    // selecting another item shouldn't deselect the previously selected one
+    m_tree->SelectItem(m_child2);
+    CPPUNIT_ASSERT( m_tree->IsSelected(m_child1) );
+    CPPUNIT_ASSERT( m_tree->IsSelected(m_child2) );
+
+    // selecting it again still shouldn't change anything
+    m_tree->SelectItem(m_child2);
+    CPPUNIT_ASSERT( m_tree->IsSelected(m_child1) );
+    CPPUNIT_ASSERT( m_tree->IsSelected(m_child2) );
+
+    // deselecting one of the items should leave the others selected
+    m_tree->UnselectItem(m_child1);
+    CPPUNIT_ASSERT( !m_tree->IsSelected(m_child1) );
+    CPPUNIT_ASSERT( m_tree->IsSelected(m_child2) );
 }