From fac4822ab3e12e54f684b7c4a5763731454d65e7 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 16 Apr 2022 23:01:57 +0100 Subject: [PATCH] Make wxAuiNotebook::FindPage() work correctly Make this function virtual in the base class so that it could be overridden to do the right thing in wxAuiNotebook, instead of just always returning NULL as before and add a unit test checking that it works. Explain that wxBookCtrlBase::m_pages may not be used in the derived classes, but that in this case they must override all the methods using it. Finally, "soft-deprecate" wxAuiNotebook::GetPageIndex(), which is identical to FindPage() now. This fixes infinite recursion when handling wxEVT_HELP in wxAuiNotebook in wxUniv too, see #22309. Closes #15932. --- include/wx/aui/auibook.h | 6 +++++- include/wx/bookctrl.h | 8 +++++--- interface/wx/aui/auibook.h | 3 +++ src/aui/auibook.cpp | 7 ++----- tests/controls/auitest.cpp | 14 ++++++++++++++ 5 files changed, 29 insertions(+), 9 deletions(-) diff --git a/include/wx/aui/auibook.h b/include/wx/aui/auibook.h index 8e4c1b08d7..1fb144ad6c 100644 --- a/include/wx/aui/auibook.h +++ b/include/wx/aui/auibook.h @@ -292,7 +292,11 @@ public: virtual size_t GetPageCount() const wxOVERRIDE; virtual wxWindow* GetPage(size_t pageIdx) const wxOVERRIDE; - int GetPageIndex(wxWindow* pageWnd) const; + virtual int FindPage(const wxWindow* page) const wxOVERRIDE; + + // This is wxAUI-specific equivalent of FindPage(), prefer to use the other + // function. + int GetPageIndex(wxWindow* pageWnd) const { return FindPage(pageWnd); } bool SetPageText(size_t page, const wxString& text) wxOVERRIDE; wxString GetPageText(size_t pageIdx) const wxOVERRIDE; diff --git a/include/wx/bookctrl.h b/include/wx/bookctrl.h index 9e5bb9fe0b..33bf6145cd 100644 --- a/include/wx/bookctrl.h +++ b/include/wx/bookctrl.h @@ -213,7 +213,7 @@ public: } // return the index of the given page or wxNOT_FOUND - int FindPage(const wxWindow* page) const; + virtual int FindPage(const wxWindow* page) const; // hit test: returns which page is hit and, optionally, where (icon, label) virtual int HitTest(const wxPoint& WXUNUSED(pt), @@ -304,7 +304,7 @@ protected: // leaf item in wxTreebook tree and this method must be overridden to // return it if AllowNullPage() is overridden. Note that it can still // return null if there are no valid pages after this one. - virtual wxWindow *TryGetNonNullPage(size_t page) { return m_pages[page]; } + virtual wxWindow *TryGetNonNullPage(size_t page) { return GetPage(page); } // Remove the page and return a pointer to it. // @@ -338,7 +338,9 @@ protected: #endif // wxUSE_HELP - // the array of all pages of this control + // the array of all pages of this control: this is used in most, but not + // all derived classes, notable wxAuiNotebook doesn't store its page here + // and so must override all virtual methods of this class using m_pages wxVector m_pages; // get the page area diff --git a/interface/wx/aui/auibook.h b/interface/wx/aui/auibook.h index 91c63d571a..670520604a 100644 --- a/interface/wx/aui/auibook.h +++ b/interface/wx/aui/auibook.h @@ -238,6 +238,9 @@ public: /** Returns the page index for the specified window. If the window is not found in the notebook, wxNOT_FOUND is returned. + + This is AUI-specific equivalent to wxBookCtrl::FindPage() and it is + recommended to use that generic method instead of this one. */ int GetPageIndex(wxWindow* page_wnd) const; diff --git a/src/aui/auibook.cpp b/src/aui/auibook.cpp index e3fcea5bd2..fb39238409 100644 --- a/src/aui/auibook.cpp +++ b/src/aui/auibook.cpp @@ -2163,15 +2163,12 @@ bool wxAuiNotebook::RemovePage(size_t page_idx) return true; } -// GetPageIndex() returns the index of the page, or -1 if the -// page could not be located in the notebook -int wxAuiNotebook::GetPageIndex(wxWindow* page_wnd) const +int wxAuiNotebook::FindPage(const wxWindow* page) const { - return m_tabs.GetIdxFromWindow(page_wnd); + return m_tabs.GetIdxFromWindow(page); } - // SetPageText() changes the tab caption of the specified page bool wxAuiNotebook::SetPageText(size_t page_idx, const wxString& text) { diff --git a/tests/controls/auitest.cpp b/tests/controls/auitest.cpp index ef1a38321a..7813f7d3aa 100644 --- a/tests/controls/auitest.cpp +++ b/tests/controls/auitest.cpp @@ -151,4 +151,18 @@ TEST_CASE_METHOD(AuiNotebookTestCase, "wxAuiNotebook::RTTI", "[aui][rtti]") CHECK( wxDynamicCast(nb, wxBookCtrlBase) == book ); } +TEST_CASE_METHOD(AuiNotebookTestCase, "wxAuiNotebook::FindPage", "[aui]") +{ + wxPanel *p1 = new wxPanel(nb); + wxPanel *p2 = new wxPanel(nb); + wxPanel *p3 = new wxPanel(nb); + REQUIRE( nb->AddPage(p1, "Page 1") ); + REQUIRE( nb->AddPage(p2, "Page 2") ); + + CHECK( nb->FindPage(NULL) == wxNOT_FOUND ); + CHECK( nb->FindPage(p1) == 0 ); + CHECK( nb->FindPage(p2) == 1 ); + CHECK( nb->FindPage(p3) == wxNOT_FOUND ); +} + #endif