Amebis/wxWidgets
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix bug in workaround for event handlers deleting wxGrid cells

Browse Source 
The changes of e6e6dbe077 (Fix problems due to deleting grid cells in
the event handlers, 2020-05-02) fixed the bug (see #18731), but
introduced another one in place: if wxEVT_GRID_CELL_CHANGED handler
deleted any cells, wxGrid code could mistakenly restore the "old" cell
value (actually it restored the value for a different cell, as the
current cell coordinates necessarily changed too in this case).

This bug became more visible after the addition of activatable editors
as the new code asserts, instead of trying to restore the old value, if
wxEVT_GRID_CELL_CHANGED is vetoed, which also resulted in asserts if the
handler deleted the current cell instead.

Fix this by separating the cases of event being really vetoed and of the
event handler deleting the cell for which the event was generated and
handle the latter separately from the former where it matters.

This commit is best viewed ignoring whitespace changes.
This commit is contained in:
Vadim Zeitlin
2020-06-30 19:04:52 +02:00
parent f877106663
commit 8445d1993e
2 changed files with 66 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
include/wx/generic/grid.h
View File

@@ -2789,9 +2789,10 @@ protected:
enum EventResult
{
Event_Vetoed = -1, // Also returned when cell was deleted.
Event_Vetoed = -1,
Event_Unhandled,
Event_Handled
Event_Handled,
Event_CellDeleted // Event handler deleted the cell.
};
// Send the given grid event and returns one of the event handling results

93
src/generic/grid.cpp
View File

@@ -5292,13 +5292,13 @@ wxGrid::EventResult wxGrid::DoSendEvent(wxGridEvent& gridEvt)
if ( !gridEvt.IsAllowed() )
return Event_Vetoed;
// 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.
// Detect the special case in which 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 Event_Vetoed;
return Event_CellDeleted;
return claimed ? Event_Handled : Event_Unhandled;
}
@@ -6044,10 +6044,18 @@ void wxGrid::DoGridProcessTab(wxKeyboardState& kbdState)
bool wxGrid::SetCurrentCell( const wxGridCellCoords& coords )
{
if ( SendEvent(wxEVT_GRID_SELECT_CELL, coords) == Event_Vetoed )
switch ( SendEvent(wxEVT_GRID_SELECT_CELL, coords) )
{
// the event has been vetoed - do nothing
return false;
case Event_Vetoed:
case Event_CellDeleted:
// We shouldn't do anything if the event was vetoed and can't do
// anything if the cell doesn't exist any longer.
return false;
case Event_Unhandled:
case Event_Handled:
// But it doesn't matter here if the event was skipped or not.
break;
}
wxGridWindow *currentGridWindow = CellToGridWindow(coords);
@@ -7156,8 +7164,19 @@ void wxGrid::EnableCellEditControl( bool enable )
bool wxGrid::DoEnableCellEditControl(const wxGridActivationSource& actSource)
{
if ( SendEvent(wxEVT_GRID_EDITOR_SHOWN) == Event_Vetoed )
return false;
switch ( SendEvent(wxEVT_GRID_EDITOR_SHOWN) )
{
case Event_Vetoed:
case Event_CellDeleted:
// We shouldn't do anything if the event was vetoed and can't do
// anything if the cell doesn't exist any longer.
return false;
case Event_Unhandled:
case Event_Handled:
// But it doesn't matter here if the event was skipped or not.
break;
}
if ( !DoShowCellEditControl(actSource) )
{
@@ -7240,19 +7259,26 @@ bool wxGrid::DoShowCellEditControl(const wxGridActivationSource& actSource)
// This is somewhat similar to what DoSaveEditControlValue() does.
// but we don't allow vetoing CHANGED event here as this code is
// new and shouldn't have to support this obsolete usage.
if ( SendEvent(wxEVT_GRID_CELL_CHANGING, res.GetNewValue()) != Event_Vetoed )
switch ( SendEvent(wxEVT_GRID_CELL_CHANGING, res.GetNewValue()) )
{
const wxString& oldval = GetCellValue(m_currentCellCoords);
case Event_Vetoed:
case Event_CellDeleted:
break;
editor->DoActivate(row, col, this);
case Event_Unhandled:
case Event_Handled:
const wxString& oldval = GetCellValue(m_currentCellCoords);
// Show the new cell value.
RefreshBlock(m_currentCellCoords, m_currentCellCoords);
editor->DoActivate(row, col, this);
if ( SendEvent(wxEVT_GRID_CELL_CHANGED, oldval) == Event_Vetoed )
{
wxFAIL_MSG( "Vetoing wxEVT_GRID_CELL_CHANGED is ignored" );
}
// Show the new cell value.
RefreshBlock(m_currentCellCoords, m_currentCellCoords);
if ( SendEvent(wxEVT_GRID_CELL_CHANGED, oldval) == Event_Vetoed )
{
wxFAIL_MSG( "Vetoing wxEVT_GRID_CELL_CHANGED is ignored" );
}
break;
}
wxFALLTHROUGH;
@@ -7486,21 +7512,28 @@ void wxGrid::DoSaveEditControlValue()
wxGridCellEditorPtr editor = GetCurrentCellEditorPtr();
wxString newval;
bool changed = editor->EndEdit(row, col, this, oldval, &newval);
if ( !editor->EndEdit(row, col, this, oldval, &newval) )
return;
if ( changed && SendEvent(wxEVT_GRID_CELL_CHANGING, newval) != Event_Vetoed )
switch ( SendEvent(wxEVT_GRID_CELL_CHANGING, newval) )
{
editor->ApplyEdit(row, col, this);
case Event_Vetoed:
case Event_CellDeleted:
break;
// for compatibility reasons dating back to wx 2.8 when this event
// was called wxEVT_GRID_CELL_CHANGE and wxEVT_GRID_CELL_CHANGING
// didn't exist we allow vetoing this one too
if ( SendEvent(wxEVT_GRID_CELL_CHANGED, oldval) == Event_Vetoed )
{
// Event has been vetoed, set the data back.
SetCellValue(m_currentCellCoords, oldval);
case Event_Unhandled:
case Event_Handled:
editor->ApplyEdit(row, col, this);
// for compatibility reasons dating back to wx 2.8 when this event
// was called wxEVT_GRID_CELL_CHANGE and wxEVT_GRID_CELL_CHANGING
// didn't exist we allow vetoing this one too
if ( SendEvent(wxEVT_GRID_CELL_CHANGED, oldval) == Event_Vetoed )
{
// Event has been vetoed, set the data back.
SetCellValue(m_currentCellCoords, oldval);
}
}
}
}
void wxGrid::OnHideEditor(wxCommandEvent& WXUNUSED(event))