From 5cdcfddc6165ef9ba1f38c670d57c74cbc006576 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 2 May 2020 00:47:43 +0200 Subject: [PATCH 1/2] Refactor event sending code in wxGrid to use even more functions Move the logic determining the return value of SendEvent() into its own function instead of repeating it twice. No real changes, this is a pure refactoring. --- include/wx/generic/grid.h | 8 ++++++-- src/generic/grid.cpp | 36 +++++++++++++++++------------------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/include/wx/generic/grid.h b/include/wx/generic/grid.h index 02ae6d2554..b6121f0083 100644 --- a/include/wx/generic/grid.h +++ b/include/wx/generic/grid.h @@ -97,6 +97,7 @@ class WXDLLIMPEXP_FWD_CORE wxGridCellAttr; class WXDLLIMPEXP_FWD_CORE wxGridCellAttrProviderData; class WXDLLIMPEXP_FWD_CORE wxGridColLabelWindow; class WXDLLIMPEXP_FWD_CORE wxGridCornerLabelWindow; +class WXDLLIMPEXP_FWD_CORE wxGridEvent; class WXDLLIMPEXP_FWD_CORE wxGridRowLabelWindow; class WXDLLIMPEXP_FWD_CORE wxGridWindow; class WXDLLIMPEXP_FWD_CORE wxGridTypeRegistry; @@ -2552,8 +2553,11 @@ protected: bool Redimension( wxGridTableMessage& ); - // generate the appropriate grid event and return -1 if it was vetoed, 1 if - // it was processed (but not vetoed) and 0 if it wasn't processed + // Send the given grid event and return -1 if it was vetoed, 1 if + // it was processed (but not vetoed) and 0 if it wasn't processed. + int DoSendEvent(wxGridEvent& gridEvt); + + // Generate an event of the given type and call DoSendEvent(). int SendEvent(wxEventType evtType, int row, int col, const wxMouseEvent& e); diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index adbf19cc61..d40c00549a 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -5293,16 +5293,27 @@ wxGrid::SendGridSizeEvent(wxEventType type, return GetEventHandler()->ProcessEvent(gridEvt); } -// Generate a grid event based on a mouse event and return: +// Process the event and return // -1 if the event was vetoed // +1 if the event was processed (but not vetoed) // 0 if the event wasn't handled +int wxGrid::DoSendEvent(wxGridEvent& gridEvt) +{ + const bool claimed = GetEventHandler()->ProcessEvent(gridEvt); + + // A Veto'd event may not be `claimed' so test this first + if ( !gridEvt.IsAllowed() ) + return -1; + + return claimed ? 1 : 0; +} + +// Generate a grid event based on a mouse event and call DoSendEvent() with it. int wxGrid::SendEvent(wxEventType type, int row, int col, const wxMouseEvent& mouseEv) { - bool claimed, vetoed; if ( type == wxEVT_GRID_LABEL_LEFT_CLICK || type == wxEVT_GRID_LABEL_LEFT_DCLICK || @@ -5324,8 +5335,8 @@ wxGrid::SendEvent(wxEventType type, pos.y, false, mouseEv); - claimed = GetEventHandler()->ProcessEvent(gridEvt); - vetoed = !gridEvt.IsAllowed(); + + return DoSendEvent(gridEvt); } else { @@ -5345,15 +5356,8 @@ wxGrid::SendEvent(wxEventType type, gridEvt.Veto(); } - claimed = GetEventHandler()->ProcessEvent(gridEvt); - vetoed = !gridEvt.IsAllowed(); + return DoSendEvent(gridEvt); } - - // A Veto'd event may not be `claimed' so test this first - if (vetoed) - return -1; - - return claimed ? 1 : 0; } // Generate a grid event of specified type, return value same as above @@ -5364,13 +5368,7 @@ wxGrid::SendEvent(wxEventType type, int row, int col, const wxString& s) wxGridEvent gridEvt( GetId(), type, this, row, col ); gridEvt.SetString(s); - const bool claimed = GetEventHandler()->ProcessEvent(gridEvt); - - // A Veto'd event may not be `claimed' so test this first - if ( !gridEvt.IsAllowed() ) - return -1; - - return claimed ? 1 : 0; + return DoSendEvent(gridEvt); } void wxGrid::Refresh(bool eraseb, const wxRect* rect) From e6e6dbe0778f035f3e87989400d14574c157c0cf Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 2 May 2020 00:53:59 +0200 Subject: [PATCH 2/2] Fix problems due to deleting grid cells in the event handlers Deleting last grid rows or column in a few event handlers could result in asserts/crashes in wxGrid code if the event handler also called event.Skip(), as wxGrid still tried to perform the default action using the deleted cell, when these events happened in the last row or column. It's not totally clear whether calling event.Skip() after performing an action modifying the grid should be allowed at all, but, in doubt, at least avoid crashing if it does happen, by considering the event as being handled (and even vetoed) if its handler deleted the cell in which it was generated. Closes #18731. --- include/wx/generic/grid.h | 6 ++++-- src/generic/grid.cpp | 10 +++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/include/wx/generic/grid.h b/include/wx/generic/grid.h index b6121f0083..414988125b 100644 --- a/include/wx/generic/grid.h +++ b/include/wx/generic/grid.h @@ -2553,8 +2553,10 @@ protected: bool Redimension( wxGridTableMessage& ); - // Send the given grid event and return -1 if it was vetoed, 1 if - // it was processed (but not vetoed) and 0 if it wasn't processed. + // Send the given grid event and return -1 if it was vetoed or, as a + // special exception, if an event for a particular cell resulted in this + // cell being deleted, 1 if it was processed (but not vetoed) and 0 if it + // wasn't processed. int DoSendEvent(wxGridEvent& gridEvt); // Generate an event of the given type and call DoSendEvent(). diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp index d40c00549a..94c9723e9d 100644 --- a/src/generic/grid.cpp +++ b/src/generic/grid.cpp @@ -5294,7 +5294,7 @@ wxGrid::SendGridSizeEvent(wxEventType type, } // Process the event and return -// -1 if the event was vetoed +// -1 if the event was vetoed or if event cell was deleted // +1 if the event was processed (but not vetoed) // 0 if the event wasn't handled int wxGrid::DoSendEvent(wxGridEvent& gridEvt) @@ -5305,6 +5305,14 @@ int wxGrid::DoSendEvent(wxGridEvent& gridEvt) if ( !gridEvt.IsAllowed() ) return -1; + // We also return -1 if the event cell was deleted, as this allows to have + // checks in several functions that generate an event and then proceed + // doing something by default with the selected cell: this shouldn't be + // done if the user-defined handler deleted this cell. + if ( gridEvt.GetRow() >= GetNumberRows() || + gridEvt.GetCol() >= GetNumberCols() ) + return -1; + return claimed ? 1 : 0; }