Reuse wxBookCtrlBase::DoSetSelection() in wxTreebook too

Avoid duplicating base class DoSetSelection() implementation in
wxTreebook, just extend it slightly by using DoGetNonNullPage() to allow
using a (sub-)page if the page associated to the selected item is null
and reuse it.

Also get rid of wxTreebook::m_actualSelection, it seems completely
unnecessary to bother keeping and updating it when we can just find it
whenever we need (which actually seems to only have been the case in the
now removed DoSetSelection() implementation anyhow).

As a side effect of this, wxTreebook pages should now be sizer correctly
when switching to them as DoSetSelection() in the base class does call
SetSize() on the page before showing it, unlike the previously used
version in wxTreebook, which omitted this call for some reason.

There should be no other user-visible changes.

Closes #4379.

include/wx/bookctrl.h

@@ -292,6 +292,12 @@ protected:
     // having nodes without any associated page)
     virtual bool AllowNullPage() const { return false; }
 
+    // For classes that allow null pages, we also need a way to find the
+    // closest non-NULL page corresponding to the given index, e.g. the first
+    // leaf item in wxTreebook tree and this method must be overridden to
+    // return it if AllowNullPage() is overridden.
+    virtual wxWindow *DoGetNonNullPage(size_t page) { return m_pages[page]; }
+
     // Remove the page and return a pointer to it.
     //
     // It also needs to update the current selection if necessary, i.e. if the

include/wx/treebook.h

@@ -38,7 +38,6 @@ public:
     // Default ctor doesn't create the control, use Create() afterwards
     wxTreebook()
     {
-        Init();
     }
 
     // This ctor creates the tree book control
@@ -49,8 +48,6 @@ public:
                long style = wxBK_DEFAULT,
                const wxString& name = wxEmptyString)
     {
-        Init();
-
         (void)Create(parent, id, pos, size, style, name);
     }
 
@@ -145,22 +142,16 @@ protected:
 
     // This subclass of wxBookCtrlBase accepts NULL page pointers (empty pages)
     virtual bool AllowNullPage() const wxOVERRIDE { return true; }
+    virtual wxWindow *DoGetNonNullPage(size_t page) wxOVERRIDE;
 
     // event handlers
     void OnTreeSelectionChange(wxTreeEvent& event);
     void OnTreeNodeExpandedCollapsed(wxTreeEvent& event);
 
-    // array of page ids and page windows
+    // array of tree item ids corresponding to the page indices
     wxVector<wxTreeItemId> m_treeIds;
 
-    // in the situation when m_selection page is not wxNOT_FOUND but page is
-    // NULL this is the first (sub)child that has a non-NULL page
-    int m_actualSelection;
-
 private:
-    // common part of all constructors
-    void Init();
-
     // The real implementations of page insertion functions
     // ------------------------------------------------------
     // All DoInsert/Add(Sub)Page functions add the page into :
@@ -182,12 +173,11 @@ private:
                          bool bSelect = false,
                          int imageId = NO_IMAGE);
 
-    // Sets selection in the tree control and updates the page being shown.
+    // Overridden methods used by the base class DoSetSelection()
-    int DoSetSelection(size_t pos, int flags = 0) wxOVERRIDE;
+    // implementation.
-
+    void UpdateSelectedPage(size_t newsel) wxOVERRIDE;
-    // Returns currently shown page. In a case when selected the node
+    wxBookCtrlEvent* CreatePageChangingEvent() const wxOVERRIDE;
-    // has empty (NULL) page finds first (sub)child with not-empty page.
+    void MakeChangedEvent(wxBookCtrlEvent &event) wxOVERRIDE;
-    wxTreebookPage *DoGetCurrentPage() const;
 
     // Does the selection update. Called from page insertion functions
     // to update selection if the selected page was pushed by the newly inserted

src/common/bookctrl.cpp

@@ -493,9 +493,9 @@ int wxBookCtrlBase::DoSetSelection(size_t n, int flags)
         if ( allowed )
         {
             if ( oldSel != wxNOT_FOUND )
-                DoShowPage(m_pages[oldSel], false);
+                DoShowPage(DoGetNonNullPage(oldSel), false);
 
-            wxWindow *page = m_pages[n];
+            wxWindow* const page = DoGetNonNullPage(n);
             page->SetSize(GetPageRect());
             DoShowPage(page, true);
 
@@ -510,6 +510,15 @@ int wxBookCtrlBase::DoSetSelection(size_t n, int flags)
                 (void)GetEventHandler()->ProcessEvent(*event);
             }
         }
+        else
+        {
+            // Selection in the control might have already had changed.
+            if ( oldSel != wxNOT_FOUND )
+            {
+                m_selection = oldSel;
+                UpdateSelectedPage(oldSel);
+            }
+        }
 
         delete event;
     }

src/generic/treebkg.cpp

@@ -66,12 +66,6 @@ wxEND_EVENT_TABLE()
 // wxTreebook creation
 // ----------------------------------------------------------------------------
 
-void wxTreebook::Init()
-{
-    m_selection =
-    m_actualSelection = wxNOT_FOUND;
-}
-
 bool
 wxTreebook::Create(wxWindow *parent,
                    wxWindowID id,
@@ -314,8 +308,6 @@ bool wxTreebook::DeleteAllPages()
 {
     wxBookCtrlBase::DeleteAllPages();
     m_treeIds.clear();
-    m_selection =
-    m_actualSelection = wxNOT_FOUND;
 
     wxTreeCtrl *tree = GetTreeCtrl();
     tree->DeleteChildren(tree->GetRootItem());
@@ -347,13 +339,6 @@ void wxTreebook::DoInternalAddPage(size_t newPos,
         {
             // selection has been moved one unit toward the end
             ++m_selection;
-            if ( m_actualSelection != wxNOT_FOUND )
-                ++m_actualSelection;
-        }
-        else if ( m_actualSelection != wxNOT_FOUND &&
-                    newPos <= (size_t)m_actualSelection )
-        {
-            DoSetSelection(m_selection);
         }
     }
 }
@@ -377,10 +362,6 @@ void wxTreebook::DoInternalRemovePageRange(size_t pagePos, size_t subCount)
         {
             // selection is far after the deleted page, so just update the index and move on
             m_selection -= 1 + subCount;
-            if ( m_actualSelection != wxNOT_FOUND)
-            {
-                m_actualSelection -= subCount + 1;
-            }
         }
         else if ( (size_t)m_selection >= pagePos )
         {
@@ -391,7 +372,6 @@ void wxTreebook::DoInternalRemovePageRange(size_t pagePos, size_t subCount)
             wxTreeItemId nodeId = tree->GetNextSibling(pageId);
 
             m_selection = wxNOT_FOUND;
-            m_actualSelection = wxNOT_FOUND;
 
             if ( nodeId.IsOk() )
             {
@@ -413,17 +393,6 @@ void wxTreebook::DoInternalRemovePageRange(size_t pagePos, size_t subCount)
                 }
             }
         }
-        else if ( m_actualSelection != wxNOT_FOUND &&
-                    (size_t)m_actualSelection >= pagePos )
-        {
-            // nothing to do -- selection is before the deleted node, but
-            // actually shown page (the first (sub)child with page != NULL) is
-            // already deleted
-            m_actualSelection = m_selection;
-
-            // send event as documented
-            DoSetSelection(m_selection, SetSelection_SendEvent);
-        }
         //else: nothing to do -- selection is before the deleted node
     }
     else
@@ -558,91 +527,41 @@ bool wxTreebook::SetPageImage(size_t n, int imageId)
     return true;
 }
 
-int wxTreebook::DoSetSelection(size_t pagePos, int flags)
+void wxTreebook::UpdateSelectedPage(size_t newsel)
 {
-    wxCHECK_MSG( IS_VALID_PAGE(pagePos), wxNOT_FOUND,
+    GetTreeCtrl()->SelectItem(DoInternalGetPage(newsel));
-                 wxT("invalid page index in wxListbook::DoSetSelection()") );
-    wxASSERT_MSG( GetPageCount() == DoInternalGetPageCount(),
-                  wxT("wxTreebook logic error: m_treeIds and m_pages not in sync!"));
-
-    wxBookCtrlEvent event(wxEVT_TREEBOOK_PAGE_CHANGING, m_windowId);
-    const int oldSel = m_selection;
-    wxTreeCtrl *tree = GetTreeCtrl();
-    bool allowed = false;
-
-    if (flags & SetSelection_SendEvent)
-    {
-        event.SetEventObject(this);
-        event.SetSelection(pagePos);
-        event.SetOldSelection(m_selection);
-
-        // don't send the event if the old and new pages are the same; do send it
-        // otherwise and be prepared for it to be vetoed
-        allowed = (int)pagePos == m_selection ||
-                  !GetEventHandler()->ProcessEvent(event) ||
-                  event.IsAllowed();
-    }
-
-    if ( !(flags & SetSelection_SendEvent) || allowed )
-    {
-        // hide the previously shown page
-        wxTreebookPage * const oldPage = DoGetCurrentPage();
-        if ( oldPage )
-            oldPage->Hide();
-
-        // then show the new one
-        m_selection = pagePos;
-        wxTreebookPage *page = wxBookCtrlBase::GetPage(m_selection);
-        if ( !page )
-        {
-            // find the next page suitable to be shown: the first (grand)child
-            // of this one with a non-NULL associated page
-            wxTreeItemId childId = m_treeIds[pagePos];
-            int actualPagePos = pagePos;
-            while ( !page && childId.IsOk() )
-            {
-                wxTreeItemIdValue cookie;
-                childId = tree->GetFirstChild( childId, cookie );
-                if ( childId.IsOk() )
-                {
-                    page = wxBookCtrlBase::GetPage(++actualPagePos);
-                }
-            }
-
-            m_actualSelection = page ? actualPagePos : m_selection;
-        }
-
-        if ( page )
-            page->Show();
-
-        tree->SelectItem(DoInternalGetPage(pagePos));
-
-        if (flags & SetSelection_SendEvent)
-        {
-            // notify about the (now completed) page change
-            event.SetEventType(wxEVT_TREEBOOK_PAGE_CHANGED);
-            (void)GetEventHandler()->ProcessEvent(event);
-        }
-    }
-    else if ( (flags & SetSelection_SendEvent) && !allowed) // page change vetoed
-    {
-        // tree selection might have already had changed
-        if ( oldSel != wxNOT_FOUND )
-            tree->SelectItem(DoInternalGetPage(oldSel));
-    }
-
-    return oldSel;
 }
 
-wxTreebookPage *wxTreebook::DoGetCurrentPage() const
+wxBookCtrlEvent* wxTreebook::CreatePageChangingEvent() const
 {
-    if ( m_selection == wxNOT_FOUND )
+    return new wxBookCtrlEvent(wxEVT_TREEBOOK_PAGE_CHANGING, m_windowId);
-        return NULL;
+}
 
-    wxTreebookPage *page = wxBookCtrlBase::GetPage(m_selection);
+void wxTreebook::MakeChangedEvent(wxBookCtrlEvent &event)
-    if ( !page && m_actualSelection != wxNOT_FOUND )
+{
+    event.SetEventType(wxEVT_TREEBOOK_PAGE_CHANGED);
+}
+
+wxWindow *wxTreebook::DoGetNonNullPage(size_t n)
+{
+    wxWindow* page = wxBookCtrlBase::GetPage(n);
+
+    if ( !page )
     {
-        page = wxBookCtrlBase::GetPage(m_actualSelection);
+        // Find the next suitable page, i.e. the first (grand)child
+        // of this one with a non-NULL associated page
+        wxTreeCtrl* const tree = GetTreeCtrl();
+        for ( wxTreeItemId childId = m_treeIds[n]; childId.IsOk(); )
+        {
+            wxTreeItemIdValue cookie;
+            childId = tree->GetFirstChild( childId, cookie );
+            if ( childId.IsOk() )
+            {
+                page = wxBookCtrlBase::GetPage(++n);
+                if ( page )
+                    break;
+            }
+        }
     }
 
     return page;