From f1a3816cd9da0c3c7ceebe9735d37fb2d3158388 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 5 Nov 2023 23:50:51 +0100 Subject: [PATCH 1/4] Fix crash in wxAuiNotebook after dragging a page Fix using outdated click position in wxAuiTabCtrl mouse handlers, which resulted in an invalid wxEVT_AUINOTEBOOK_DRAG_MOTION event being generated and a crash while handling it. Ensure that we reset m_clickPt when resetting m_isDragging too so that we don't decide that we're dragging if the mouse enters the window with the left button already pressed after a previous drag. Closes #24027. --- src/aui/auibook.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/aui/auibook.cpp b/src/aui/auibook.cpp index d10384b71b..830798eff2 100644 --- a/src/aui/auibook.cpp +++ b/src/aui/auibook.cpp @@ -1099,6 +1099,7 @@ void wxAuiTabCtrl::OnCaptureLost(wxMouseCaptureLostEvent& WXUNUSED(event)) { if (m_isDragging) { + m_clickPt = wxDefaultPosition; m_isDragging = false; wxAuiNotebookEvent evt(wxEVT_AUINOTEBOOK_CANCEL_DRAG, m_windowId); @@ -1116,6 +1117,7 @@ void wxAuiTabCtrl::OnLeftUp(wxMouseEvent& evt) if (m_isDragging) { + m_clickPt = wxDefaultPosition; m_isDragging = false; wxAuiNotebookEvent e(wxEVT_AUINOTEBOOK_END_DRAG, m_windowId); From bd7b333a87fc85442e156881bcc5737c1c5182f7 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 6 Nov 2023 00:02:57 +0100 Subject: [PATCH 2/4] Ensure that we only pass valid indices to wxAuiTabCtrl::GetPage() Passing an invalid index to this function results in a crash, so ensure this doesn't happen -- at the very least this would have turned the crash fixed by the parent commit into "just" an assert failure. --- src/aui/auibook.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/aui/auibook.cpp b/src/aui/auibook.cpp index 830798eff2..fe607232fe 100644 --- a/src/aui/auibook.cpp +++ b/src/aui/auibook.cpp @@ -2607,6 +2607,8 @@ void wxAuiNotebook::OnTabDragMotion(wxAuiNotebookEvent& evt) if (dest_tabs->TabHitTest(pt.x, pt.y, &dest_location_tab)) { int src_idx = evt.GetSelection(); + wxCHECK_RET( src_idx != -1, "Invalid source tab?" ); + int dest_idx = dest_tabs->GetIdxFromWindow(dest_location_tab); // prevent jumpy drag From 0fd15e405f38ae7bb53197c1adf5f1e94c70a556 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 6 Nov 2023 00:05:50 +0100 Subject: [PATCH 3/4] Initialize wxAuiTabCtrl members in their declarations Ensure that all of them are initialized, including m_clickTab, which didn't seem to be always initialized before. No real changes (as m_clickTab doesn't seem to have been used before it was initialized). --- include/wx/aui/auibook.h | 10 +++++----- src/aui/auibook.cpp | 4 ---- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/include/wx/aui/auibook.h b/include/wx/aui/auibook.h index 9fb0739e15..5962a661c2 100644 --- a/include/wx/aui/auibook.h +++ b/include/wx/aui/auibook.h @@ -236,11 +236,11 @@ protected: protected: - wxPoint m_clickPt; - wxWindow* m_clickTab; - bool m_isDragging; - wxAuiTabContainerButton* m_hoverButton; - wxAuiTabContainerButton* m_pressedButton; + wxPoint m_clickPt = wxDefaultPosition; + wxWindow* m_clickTab = nullptr; + bool m_isDragging = false; + wxAuiTabContainerButton* m_hoverButton = nullptr; + wxAuiTabContainerButton* m_pressedButton = nullptr; void SetHoverTab(wxWindow* wnd); diff --git a/src/aui/auibook.cpp b/src/aui/auibook.cpp index fe607232fe..88b7300e56 100644 --- a/src/aui/auibook.cpp +++ b/src/aui/auibook.cpp @@ -1013,10 +1013,6 @@ wxAuiTabCtrl::wxAuiTabCtrl(wxWindow* parent, long style) : wxControl(parent, id, pos, size, style) { SetName(wxT("wxAuiTabCtrl")); - m_clickPt = wxDefaultPosition; - m_isDragging = false; - m_hoverButton = nullptr; - m_pressedButton = nullptr; } wxAuiTabCtrl::~wxAuiTabCtrl() From a0f798029545dbfc58d0c7651ee0e51830e57ff5 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 6 Nov 2023 00:11:52 +0100 Subject: [PATCH 4/4] Add wxAuiTabCtrl::DoEndDragging() No real changes, just refactor the code to reset all drag-related variables in a single function instead of doing it manually in several places. As this function also resets m_clickTab, which could previosuly remain a dangling pointer, change the code to get its value before calling it. --- include/wx/aui/auibook.h | 5 +++++ src/aui/auibook.cpp | 31 +++++++++++++++++++------------ 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/include/wx/aui/auibook.h b/include/wx/aui/auibook.h index 5962a661c2..8f79f8f576 100644 --- a/include/wx/aui/auibook.h +++ b/include/wx/aui/auibook.h @@ -239,11 +239,16 @@ protected: wxPoint m_clickPt = wxDefaultPosition; wxWindow* m_clickTab = nullptr; bool m_isDragging = false; + wxAuiTabContainerButton* m_hoverButton = nullptr; wxAuiTabContainerButton* m_pressedButton = nullptr; void SetHoverTab(wxWindow* wnd); +private: + // Reset dragging-related fields above to their initial values. + void DoEndDragging(); + #ifndef SWIG wxDECLARE_CLASS(wxAuiTabCtrl); wxDECLARE_EVENT_TABLE(); diff --git a/src/aui/auibook.cpp b/src/aui/auibook.cpp index 88b7300e56..035e763edb 100644 --- a/src/aui/auibook.cpp +++ b/src/aui/auibook.cpp @@ -1019,6 +1019,13 @@ wxAuiTabCtrl::~wxAuiTabCtrl() { } +void wxAuiTabCtrl::DoEndDragging() +{ + m_clickPt = wxDefaultPosition; + m_isDragging = false; + m_clickTab = nullptr; +} + void wxAuiTabCtrl::OnPaint(wxPaintEvent&) { wxPaintDC dc(this); @@ -1053,9 +1060,9 @@ void wxAuiTabCtrl::OnSize(wxSizeEvent& evt) void wxAuiTabCtrl::OnLeftDown(wxMouseEvent& evt) { CaptureMouse(); - m_clickPt = wxDefaultPosition; - m_isDragging = false; - m_clickTab = nullptr; + + // Reset any previous values first. + DoEndDragging(); m_pressedButton = nullptr; @@ -1095,11 +1102,12 @@ void wxAuiTabCtrl::OnCaptureLost(wxMouseCaptureLostEvent& WXUNUSED(event)) { if (m_isDragging) { - m_clickPt = wxDefaultPosition; - m_isDragging = false; + const auto clickTab = m_clickTab; + + DoEndDragging(); wxAuiNotebookEvent evt(wxEVT_AUINOTEBOOK_CANCEL_DRAG, m_windowId); - evt.SetSelection(GetIdxFromWindow(m_clickTab)); + evt.SetSelection(GetIdxFromWindow(clickTab)); evt.SetOldSelection(evt.GetSelection()); evt.SetEventObject(this); GetEventHandler()->ProcessEvent(evt); @@ -1113,11 +1121,12 @@ void wxAuiTabCtrl::OnLeftUp(wxMouseEvent& evt) if (m_isDragging) { - m_clickPt = wxDefaultPosition; - m_isDragging = false; + const auto clickTab = m_clickTab; + + DoEndDragging(); wxAuiNotebookEvent e(wxEVT_AUINOTEBOOK_END_DRAG, m_windowId); - e.SetSelection(GetIdxFromWindow(m_clickTab)); + e.SetSelection(GetIdxFromWindow(clickTab)); e.SetOldSelection(e.GetSelection()); e.SetEventObject(this); GetEventHandler()->ProcessEvent(e); @@ -1154,9 +1163,7 @@ void wxAuiTabCtrl::OnLeftUp(wxMouseEvent& evt) m_pressedButton = nullptr; } - m_clickPt = wxDefaultPosition; - m_isDragging = false; - m_clickTab = nullptr; + DoEndDragging(); } void wxAuiTabCtrl::OnMiddleUp(wxMouseEvent& evt)