Fix adding pages without associated window to wxTreebook

wxTreebook is supposed to allow not specifying any valid window for the
top-level pages, but this didn't work any longer, probably since the
changes of 02a92e23f3 (see #4379), as a
possibly null page was dereferenced without checking, resulting in a
crash.

Fix this by adding a missing check.

Also rename DoGetNonNullPage() to TryGetNonNullPage() to make it more
clear that this function can return null and add a unit test checking
that calling AddPage(NULL) really works (or at least doesn't crash).

See https://github.com/wxWidgets/wxWidgets/pull/921
This commit is contained in:
Vadim Zeitlin 2018-09-18 00:33:59 +02:00
parent 39e19a8f8c
commit b88d5e08ce
5 changed files with 32 additions and 8 deletions

View File

@ -293,8 +293,9 @@ protected:
// For classes that allow null pages, we also need a way to find the // 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 // 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 // leaf item in wxTreebook tree and this method must be overridden to
// return it if AllowNullPage() is overridden. // return it if AllowNullPage() is overridden. Note that it can still
virtual wxWindow *DoGetNonNullPage(size_t page) { return m_pages[page]; } // return null if there are no valid pages after this one.
virtual wxWindow *TryGetNonNullPage(size_t page) { return m_pages[page]; }
// Remove the page and return a pointer to it. // Remove the page and return a pointer to it.
// //

View File

@ -142,7 +142,7 @@ protected:
// This subclass of wxBookCtrlBase accepts NULL page pointers (empty pages) // This subclass of wxBookCtrlBase accepts NULL page pointers (empty pages)
virtual bool AllowNullPage() const wxOVERRIDE { return true; } virtual bool AllowNullPage() const wxOVERRIDE { return true; }
virtual wxWindow *DoGetNonNullPage(size_t page) wxOVERRIDE; virtual wxWindow *TryGetNonNullPage(size_t page) wxOVERRIDE;
// event handlers // event handlers
void OnTreeSelectionChange(wxTreeEvent& event); void OnTreeSelectionChange(wxTreeEvent& event);

View File

@ -493,11 +493,18 @@ int wxBookCtrlBase::DoSetSelection(size_t n, int flags)
if ( allowed ) if ( allowed )
{ {
if ( oldSel != wxNOT_FOUND ) if ( oldSel != wxNOT_FOUND )
DoShowPage(DoGetNonNullPage(oldSel), false); {
if ( wxWindow* const oldPage = TryGetNonNullPage(oldSel) )
{
DoShowPage(oldPage, false);
}
}
wxWindow* const page = DoGetNonNullPage(n); if ( wxWindow* const page = TryGetNonNullPage(n) )
page->SetSize(GetPageRect()); {
DoShowPage(page, true); page->SetSize(GetPageRect());
DoShowPage(page, true);
}
// change selection now to ignore the selection change event // change selection now to ignore the selection change event
m_selection = n; m_selection = n;

View File

@ -542,7 +542,7 @@ void wxTreebook::MakeChangedEvent(wxBookCtrlEvent &event)
event.SetEventType(wxEVT_TREEBOOK_PAGE_CHANGED); event.SetEventType(wxEVT_TREEBOOK_PAGE_CHANGED);
} }
wxWindow *wxTreebook::DoGetNonNullPage(size_t n) wxWindow *wxTreebook::TryGetNonNullPage(size_t n)
{ {
wxWindow* page = wxBookCtrlBase::GetPage(n); wxWindow* page = wxBookCtrlBase::GetPage(n);

View File

@ -43,11 +43,13 @@ private:
wxBOOK_CTRL_BASE_TESTS(); wxBOOK_CTRL_BASE_TESTS();
CPPUNIT_TEST( Image ); CPPUNIT_TEST( Image );
CPPUNIT_TEST( SubPages ); CPPUNIT_TEST( SubPages );
CPPUNIT_TEST( ContainerPage );
CPPUNIT_TEST( Expand ); CPPUNIT_TEST( Expand );
CPPUNIT_TEST( Delete ); CPPUNIT_TEST( Delete );
CPPUNIT_TEST_SUITE_END(); CPPUNIT_TEST_SUITE_END();
void SubPages(); void SubPages();
void ContainerPage();
void Expand(); void Expand();
void Delete(); void Delete();
@ -92,6 +94,20 @@ void TreebookTestCase::SubPages()
CPPUNIT_ASSERT_EQUAL(3, m_treebook->GetPageParent(5)); CPPUNIT_ASSERT_EQUAL(3, m_treebook->GetPageParent(5));
} }
void TreebookTestCase::ContainerPage()
{
// Get rid of the pages added in setUp().
m_treebook->DeleteAllPages();
CHECK( m_treebook->GetPageCount() == 0 );
// Adding a page without the associated window should be allowed.
REQUIRE_NOTHROW( m_treebook->AddPage(NULL, "Container page") );
CHECK( m_treebook->GetPageParent(0) == -1 );
m_treebook->AddSubPage(new wxPanel(m_treebook), "Child page");
CHECK( m_treebook->GetPageParent(1) == 0 );
}
void TreebookTestCase::Expand() void TreebookTestCase::Expand()
{ {
wxPanel* subpanel1 = new wxPanel(m_treebook); wxPanel* subpanel1 = new wxPanel(m_treebook);