Add wxBookCtrlBase::DoSetSelectionAfterRemoval() and use it in wxSimplebook.

This fixes the wrong handling of the selection in wxSimplebook when the
currently selected page was deleted.

Also extend the unit tests to check for this bug.

Closes #15188.

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

diff --git a/include/wx/bookctrl.h b/include/wx/bookctrl.h
index d1065dbde5fd5c837bc9e5f403c1b58454df17e5..324fd94ca1509503fd3d23287052fda10c256458 100644 (file)
--- a/include/wx/bookctrl.h
+++ b/include/wx/bookctrl.h
@@ -256,6 +256,10 @@ protected:
     // false otherwise.
     bool DoSetSelectionAfterInsertion(size_t n, bool bSelect);
 
+    // Update the selection after removing the page at the given index,
+    // typically called from the derived class overridden DoRemovePage().
+    void DoSetSelectionAfterRemoval(size_t n);
+
     // set the selection to the given page, sending the events (which can
     // possibly prevent the page change from taking place) if SendEvent flag is
     // included
@@ -292,7 +296,11 @@ protected:
     // having nodes without any associated page)
     virtual bool AllowNullPage() const { return false; }
 
-    // remove the page and return a pointer to it
+    // Remove the page and return a pointer to it.
+    //
+    // It also needs to update the current selection if necessary, i.e. if the
+    // page being removed comes before the selected one and the helper method
+    // DoSetSelectionAfterRemoval() can be used for this.
     virtual wxWindow *DoRemovePage(size_t page) = 0;
 
     // our best size is the size which fits all our pages

diff --git a/include/wx/simplebook.h b/include/wx/simplebook.h
index 9b6474b0c4ce729b6ae3c914df570a9d2137ee45..795391466cae543da3ecc65e0d4e54c74cc49030 100644 (file)
--- a/include/wx/simplebook.h
+++ b/include/wx/simplebook.h
@@ -156,8 +156,15 @@ protected:
 
     virtual wxWindow *DoRemovePage(size_t page)
     {
-        m_pageTexts.erase(m_pageTexts.begin() + page);
-        return wxBookCtrlBase::DoRemovePage(page);
+        wxWindow* const win = wxBookCtrlBase::DoRemovePage(page);
+        if ( win )
+        {
+            m_pageTexts.erase(m_pageTexts.begin() + page);
+
+            DoSetSelectionAfterRemoval(page);
+        }
+
+        return win;
     }
 
     virtual void DoSize()

diff --git a/src/common/bookctrl.cpp b/src/common/bookctrl.cpp
index 9ec08bad1c9d98829693abcd4aab3e1e4d32c163..8d86c5343e6cf6e5baabfa14e3eceba1653d94bc 100644 (file)
--- a/src/common/bookctrl.cpp
+++ b/src/common/bookctrl.cpp
@@ -456,6 +456,26 @@ bool wxBookCtrlBase::DoSetSelectionAfterInsertion(size_t n, bool bSelect)
     return true;
 }
 
+void wxBookCtrlBase::DoSetSelectionAfterRemoval(size_t n)
+{
+    if ( m_selection >= (int)n )
+    {
+        // ensure that the selection is valid
+        int sel;
+        if ( GetPageCount() == 0 )
+            sel = wxNOT_FOUND;
+        else
+            sel = m_selection ? m_selection - 1 : 0;
+
+        // if deleting current page we shouldn't try to hide it
+        m_selection = m_selection == (int)n ? wxNOT_FOUND
+                                            : m_selection - 1;
+
+        if ( sel != wxNOT_FOUND && sel != m_selection )
+            SetSelection(sel);
+    }
+}
+
 int wxBookCtrlBase::DoSetSelection(size_t n, int flags)
 {
     wxCHECK_MSG( n < GetPageCount(), wxNOT_FOUND,

diff --git a/src/generic/choicbkg.cpp b/src/generic/choicbkg.cpp
index c8b2c5acf5537fe3ddcdab750f01226eb4f7be66..5c0ac7a514c18a95e9fe4f2296ef52d6e48aea67 100644 (file)
--- a/src/generic/choicbkg.cpp
+++ b/src/generic/choicbkg.cpp
@@ -209,22 +209,7 @@ wxWindow *wxChoicebook::DoRemovePage(size_t page)
     {
         GetChoiceCtrl()->Delete(page);
 
-        if ( m_selection >= (int)page )
-        {
-            // ensure that the selection is valid
-            int sel;
-            if ( GetPageCount() == 0 )
-                sel = wxNOT_FOUND;
-            else
-                sel = m_selection ? m_selection - 1 : 0;
-
-            // if deleting current page we shouldn't try to hide it
-            m_selection = m_selection == (int)page ? wxNOT_FOUND
-                                                   : m_selection - 1;
-
-            if ( sel != wxNOT_FOUND && sel != m_selection )
-                SetSelection(sel);
-        }
+        DoSetSelectionAfterRemoval(page);
     }
 
     return win;

diff --git a/src/generic/listbkg.cpp b/src/generic/listbkg.cpp
index ccdc712780881d788976c9245b0b95a9187a6702..ba736367c766f1ae5032f55cd6a66b8c0bacbfc5 100644 (file)
--- a/src/generic/listbkg.cpp
+++ b/src/generic/listbkg.cpp
@@ -348,28 +348,13 @@ wxListbook::InsertPage(size_t n,
 
 wxWindow *wxListbook::DoRemovePage(size_t page)
 {
-    const size_t page_count = GetPageCount();
     wxWindow *win = wxBookCtrlBase::DoRemovePage(page);
 
     if ( win )
     {
         GetListView()->DeleteItem(page);
 
-        if (m_selection >= (int)page)
-        {
-            // force new sel valid if possible
-            int sel = m_selection - 1;
-            if (page_count == 1)
-                sel = wxNOT_FOUND;
-            else if ((page_count == 2) || (sel == wxNOT_FOUND))
-                sel = 0;
-
-            // force sel invalid if deleting current page - don't try to hide it
-            m_selection = (m_selection == (int)page) ? wxNOT_FOUND : m_selection - 1;
-
-            if ((sel != wxNOT_FOUND) && (sel != m_selection))
-                SetSelection(sel);
-        }
+        DoSetSelectionAfterRemoval(page);
 
         GetListView()->Arrange();
         UpdateSize();

diff --git a/src/generic/toolbkg.cpp b/src/generic/toolbkg.cpp
index 1fe2ad781d4b0964a55ac2f39b80ee7211684daa..f324ea96db78409a4561b72130a336c0b7b9340c 100644 (file)
--- a/src/generic/toolbkg.cpp
+++ b/src/generic/toolbkg.cpp
@@ -333,28 +333,13 @@ bool wxToolbook::InsertPage(size_t n,
 
 wxWindow *wxToolbook::DoRemovePage(size_t page)
 {
-    const size_t page_count = GetPageCount();
     wxWindow *win = wxBookCtrlBase::DoRemovePage(page);
 
     if ( win )
     {
         GetToolBar()->DeleteTool(page + 1);
 
-        if (m_selection >= (int)page)
-        {
-            // force new sel valid if possible
-            int sel = m_selection - 1;
-            if (page_count == 1)
-                sel = wxNOT_FOUND;
-            else if ((page_count == 2) || (sel == wxNOT_FOUND))
-                sel = 0;
-
-            // force sel invalid if deleting current page - don't try to hide it
-            m_selection = (m_selection == (int)page) ? wxNOT_FOUND : m_selection - 1;
-
-            if ((sel != wxNOT_FOUND) && (sel != m_selection))
-                SetSelection(sel);
-        }
+        DoSetSelectionAfterRemoval(page);
     }
 
     return win;

diff --git a/tests/controls/bookctrlbasetest.cpp b/tests/controls/bookctrlbasetest.cpp
index c5065607e286997cd0dcc05bc688dab3ab5618fc..acd787f6fad5e90a0ab6d5e5ae80569d724f498e 100644 (file)
--- a/tests/controls/bookctrlbasetest.cpp
+++ b/tests/controls/bookctrlbasetest.cpp
@@ -95,19 +95,25 @@ void BookCtrlBaseTestCase::PageManagement()
     CPPUNIT_ASSERT_EQUAL(0, base->GetSelection());
     CPPUNIT_ASSERT_EQUAL(4, base->GetPageCount());
 
+    // Change the selection to verify that deleting a page before the currently
+    // selected one correctly updates the selection.
+    base->SetSelection(2);
+    CPPUNIT_ASSERT_EQUAL(2, base->GetSelection());
+
     base->DeletePage(1);
 
     CPPUNIT_ASSERT_EQUAL(3, base->GetPageCount());
+    CPPUNIT_ASSERT_EQUAL(1, base->GetSelection());
 
     base->RemovePage(0);
 
     CPPUNIT_ASSERT_EQUAL(2, base->GetPageCount());
+    CPPUNIT_ASSERT_EQUAL(0, base->GetSelection());
 
     base->DeleteAllPages();
 
     CPPUNIT_ASSERT_EQUAL(0, base->GetPageCount());
-
-    AddPanels();
+    CPPUNIT_ASSERT_EQUAL(-1, base->GetSelection());
 }
 
 void BookCtrlBaseTestCase::ChangeEvents()