Fix assert with setting current cell in wxGrid::Redimension()

Avoid calling wxGrid::SetCurrentCell(0, 0) when the grid has no columns
or rows, as it doesn't have any cells then and doing this logically
fails the precondition assert in GetColPos().

Also refactor all 6 different snippets calling SetCurrentCell() in
Redimension() into a single function to simplify the code and make it
more maintainable.

Add a unit test verifying that this works as intended.

Closes https://github.com/wxWidgets/wxWidgets/pull/1546
This commit is contained in:
Ilya Sinitsyn
2019-09-13 00:06:37 +07:00
committed by Vadim Zeitlin
parent 41b444e011
commit dda6aa6bdc
3 changed files with 78 additions and 46 deletions

View File

@@ -2309,6 +2309,10 @@ private:
// common part of Clip{Horz,Vert}GridLines // common part of Clip{Horz,Vert}GridLines
void DoClipGridLines(bool& var, bool clip); void DoClipGridLines(bool& var, bool clip);
// Redimension() helper: update m_currentCellCoords if necessary after a
// grid size change
void UpdateCurrentCellOnRedim();
// update the sorting indicator shown in the specified column (whose index // update the sorting indicator shown in the specified column (whose index
// must be valid) // must be valid)
// //

View File

@@ -2915,13 +2915,7 @@ bool wxGrid::Redimension( wxGridTableMessage& msg )
} }
} }
if ( m_currentCellCoords == wxGridNoCellCoords ) UpdateCurrentCellOnRedim();
{
// if we have just inserted cols into an empty grid the current
// cell will be undefined...
//
SetCurrentCell( 0, 0 );
}
if ( m_selection ) if ( m_selection )
m_selection->UpdateRows( pos, numRows ); m_selection->UpdateRows( pos, numRows );
@@ -2960,13 +2954,7 @@ bool wxGrid::Redimension( wxGridTableMessage& msg )
} }
} }
if ( m_currentCellCoords == wxGridNoCellCoords ) UpdateCurrentCellOnRedim();
{
// if we have just inserted cols into an empty grid the current
// cell will be undefined...
//
SetCurrentCell( 0, 0 );
}
if ( !GetBatchCount() ) if ( !GetBatchCount() )
{ {
@@ -2996,15 +2984,7 @@ bool wxGrid::Redimension( wxGridTableMessage& msg )
} }
} }
if ( !m_numRows ) UpdateCurrentCellOnRedim();
{
m_currentCellCoords = wxGridNoCellCoords;
}
else
{
if ( m_currentCellCoords.GetRow() >= m_numRows )
m_currentCellCoords.Set( 0, 0 );
}
if ( m_selection ) if ( m_selection )
m_selection->UpdateRows( pos, -((int)numRows) ); m_selection->UpdateRows( pos, -((int)numRows) );
@@ -3080,13 +3060,7 @@ bool wxGrid::Redimension( wxGridTableMessage& msg )
} }
} }
if ( m_currentCellCoords == wxGridNoCellCoords ) UpdateCurrentCellOnRedim();
{
// if we have just inserted cols into an empty grid the current
// cell will be undefined...
//
SetCurrentCell( 0, 0 );
}
if ( m_selection ) if ( m_selection )
m_selection->UpdateCols( pos, numCols ); m_selection->UpdateCols( pos, numCols );
@@ -3144,13 +3118,8 @@ bool wxGrid::Redimension( wxGridTableMessage& msg )
if ( m_useNativeHeader ) if ( m_useNativeHeader )
GetGridColHeader()->SetColumnCount(m_numCols); GetGridColHeader()->SetColumnCount(m_numCols);
if ( m_currentCellCoords == wxGridNoCellCoords ) UpdateCurrentCellOnRedim();
{
// if we have just inserted cols into an empty grid the current
// cell will be undefined...
//
SetCurrentCell( 0, 0 );
}
if ( !GetBatchCount() ) if ( !GetBatchCount() )
{ {
CalcDimensions(); CalcDimensions();
@@ -3199,15 +3168,7 @@ bool wxGrid::Redimension( wxGridTableMessage& msg )
} }
} }
if ( !m_numCols ) UpdateCurrentCellOnRedim();
{
m_currentCellCoords = wxGridNoCellCoords;
}
else
{
if ( m_currentCellCoords.GetCol() >= m_numCols )
m_currentCellCoords.Set( 0, 0 );
}
if ( m_selection ) if ( m_selection )
m_selection->UpdateCols( pos, -((int)numCols) ); m_selection->UpdateCols( pos, -((int)numCols) );
@@ -3622,6 +3583,41 @@ void wxGrid::ProcessRowLabelMouseEvent( wxMouseEvent& event, wxGridRowLabelWindo
} }
} }
void wxGrid::UpdateCurrentCellOnRedim()
{
if (m_currentCellCoords == wxGridNoCellCoords)
{
// We didn't have any valid selection before, which can only happen
// if the grid was empty.
// Check if this is still the case and ensure we do have valid
// selection if the grid is not empty any more.
if (m_numCols > 0 && m_numRows > 0)
{
SetCurrentCell(0, 0);
}
}
else
{
if (m_numCols == 0 || m_numRows == 0)
{
// We have to reset the selection, as it must either use validate
// coordinates otherwise, but there are no valid coordinates for
// the grid cells any more now that it is empty.
m_currentCellCoords = wxGridNoCellCoords;
}
else
{
int col = m_currentCellCoords.GetCol();
int row = m_currentCellCoords.GetRow();
if (col >= m_numCols)
col = m_numCols - 1;
if (row >= m_numRows)
row = m_numRows - 1;
SetCurrentCell(col, row);
}
}
}
void wxGrid::UpdateColumnSortingIndicator(int col) void wxGrid::UpdateColumnSortingIndicator(int col)
{ {
wxCHECK_RET( col != wxNOT_FOUND, "invalid column index" ); wxCHECK_RET( col != wxNOT_FOUND, "invalid column index" );

View File

@@ -58,6 +58,7 @@ private:
CPPUNIT_TEST( Cursor ); CPPUNIT_TEST( Cursor );
CPPUNIT_TEST( Selection ); CPPUNIT_TEST( Selection );
CPPUNIT_TEST( AddRowCol ); CPPUNIT_TEST( AddRowCol );
CPPUNIT_TEST( DeleteAndAddRowCol );
CPPUNIT_TEST( ColumnOrder ); CPPUNIT_TEST( ColumnOrder );
CPPUNIT_TEST( ColumnVisibility ); CPPUNIT_TEST( ColumnVisibility );
CPPUNIT_TEST( LineFormatting ); CPPUNIT_TEST( LineFormatting );
@@ -75,6 +76,7 @@ private:
CPPUNIT_TEST( ColumnOrder ); CPPUNIT_TEST( ColumnOrder );
WXUISIM_TEST( ResizeScrolledHeader ); WXUISIM_TEST( ResizeScrolledHeader );
WXUISIM_TEST( ColumnMinWidth ); WXUISIM_TEST( ColumnMinWidth );
CPPUNIT_TEST( DeleteAndAddRowCol );
CPPUNIT_TEST( PseudoTest_NativeLabels ); CPPUNIT_TEST( PseudoTest_NativeLabels );
NONGTK_TEST( LabelClick ); NONGTK_TEST( LabelClick );
NONGTK_TEST( SortClick ); NONGTK_TEST( SortClick );
@@ -93,6 +95,7 @@ private:
void Cursor(); void Cursor();
void Selection(); void Selection();
void AddRowCol(); void AddRowCol();
void DeleteAndAddRowCol();
void ColumnOrder(); void ColumnOrder();
void ColumnVisibility(); void ColumnVisibility();
void LineFormatting(); void LineFormatting();
@@ -540,6 +543,35 @@ void GridTestCase::AddRowCol()
CPPUNIT_ASSERT_EQUAL(7, m_grid->GetNumberCols()); CPPUNIT_ASSERT_EQUAL(7, m_grid->GetNumberCols());
} }
void GridTestCase::DeleteAndAddRowCol()
{
CPPUNIT_ASSERT_EQUAL(10, m_grid->GetNumberRows());
CPPUNIT_ASSERT_EQUAL(2, m_grid->GetNumberCols());
m_grid->DeleteRows(0, 10);
m_grid->DeleteCols(0, 2);
CPPUNIT_ASSERT_EQUAL(0, m_grid->GetNumberRows());
CPPUNIT_ASSERT_EQUAL(0, m_grid->GetNumberCols());
m_grid->AppendRows(5);
m_grid->AppendCols(3);
CPPUNIT_ASSERT_EQUAL(5, m_grid->GetNumberRows());
CPPUNIT_ASSERT_EQUAL(3, m_grid->GetNumberCols());
// The order of functions calls can be important
m_grid->DeleteCols(0, 3);
m_grid->DeleteRows(0, 5);
CPPUNIT_ASSERT_EQUAL(0, m_grid->GetNumberRows());
CPPUNIT_ASSERT_EQUAL(0, m_grid->GetNumberCols());
// Different functions calls order
m_grid->AppendCols(3);
m_grid->AppendRows(5);
}
void GridTestCase::ColumnOrder() void GridTestCase::ColumnOrder()
{ {
m_grid->AppendCols(2); m_grid->AppendCols(2);