ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

tdf#136956 reorder undo actions in removeColumns

Browse Source 
and removeRows.  Inside the removeColumns and removeRows, undo actions
are added first, and then cell spans are updated to reflect the removed
columns or rows. Once undo the cell spans they become immediately
invalid because the rows or columns are already removed, hence cause
Impress to crash.

Change-Id: I9d8641bdad43026eca03cbeaaa3a5907b516304f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/112355
Tested-by: Jenkins
Reviewed-by: Mark Hung <marklh9@gmail.com>
This commit is contained in:
Mark Hung
2021-03-16 15:23:58 +01:00
parent fd9b1fbcae
commit f3f7cc53ef
2 changed files with 70 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sd/qa/unit/misc-tests.cxx
+35
View File
@@ -27,6 +27,8 @@
#include <com/sun/star/graphic/XGraphic.hpp>
#include <com/sun/star/container/XIndexAccess.hpp>
#include <com/sun/star/frame/XLoadable.hpp>
#include <com/sun/star/table/XTable.hpp>
#include <com/sun/star/table/XMergeableCellRange.hpp>
#include <vcl/scheduler.hxx>
#include <osl/thread.hxx>
@@ -82,6 +84,7 @@ public:
void testTdf130988();
void testTdf131033();
void testTdf129898LayerDrawnInSlideshow();
void testTdf136956();
CPPUNIT_TEST_SUITE(SdMiscTest);
CPPUNIT_TEST(testTdf96206);
@@ -103,6 +106,7 @@ public:
CPPUNIT_TEST(testTdf130988);
CPPUNIT_TEST(testTdf131033);
CPPUNIT_TEST(testTdf129898LayerDrawnInSlideshow);
CPPUNIT_TEST(testTdf136956);
CPPUNIT_TEST_SUITE_END();
virtual void registerNamespaces(xmlXPathContextPtr& pXmlXPathCtx) override
@@ -874,6 +878,37 @@ void SdMiscTest::testTdf129898LayerDrawnInSlideshow()
xDocShRef->DoClose();
}
void SdMiscTest::testTdf136956()
{
::sd::DrawDocShellRef xDocShRef = loadURL(m_directories.getURLFromSrc(u"sd/qa/unit/data/odp/cellspan.odp"), ODP);
const SdrPage *pPage = GetPage( 1, xDocShRef );
sdr::table::SdrTableObj *pTableObj = dynamic_cast<sdr::table::SdrTableObj*>(pPage->GetObj(0));
CPPUNIT_ASSERT( pTableObj );
uno::Reference< table::XTable > xTable(pTableObj->getTable(), uno::UNO_SET_THROW);
uno::Reference< css::table::XMergeableCellRange > xRange(
xTable->createCursorByRange( xTable->getCellRangeByPosition( 0, 0, 3, 2 ) ), uno::UNO_QUERY_THROW );
// 4x3 Table before merge.
CPPUNIT_ASSERT_EQUAL(sal_Int32(4), xTable->getColumnCount());
CPPUNIT_ASSERT_EQUAL(sal_Int32(3), xTable->getRowCount());
xRange->merge();
// 1x1 Table after merge.
CPPUNIT_ASSERT_EQUAL(sal_Int32(1), xTable->getColumnCount());
CPPUNIT_ASSERT_EQUAL(sal_Int32(1), xTable->getRowCount());
xDocShRef->GetUndoManager()->Undo();
// 4x3 Table after undo. Undo crashed before.
CPPUNIT_ASSERT_EQUAL(sal_Int32(4), xTable->getColumnCount());
CPPUNIT_ASSERT_EQUAL(sal_Int32(3), xTable->getRowCount());
xDocShRef->DoClose();
}
CPPUNIT_TEST_SUITE_REGISTRATION(SdMiscTest);
CPPUNIT_PLUGIN_IMPLEMENT();
svx/source/table/tablemodel.cxx
+35 -26
View File
@@ -694,24 +694,6 @@ void TableModel::removeColumns( sal_Int32 nIndex, sal_Int32 nCount )
{
rModel.BegUndo( SvxResId(STR_UNDO_COL_DELETE) );
rModel.AddUndo( rModel.GetSdrUndoFactory().CreateUndoGeoObject(*mpTableObj) );
TableModelRef xThis( this );
ColumnVector aRemovedCols( nCount );
sal_Int32 nOffset;
for( nOffset = 0; nOffset < nCount; ++nOffset )
{
aRemovedCols[nOffset] = maColumns[nIndex+nOffset];
}
CellVector aRemovedCells( nCount * nRows );
CellVector::iterator aCellIter( aRemovedCells.begin() );
for( sal_Int32 nRow = 0; nRow < nRows; ++nRow )
{
for( nOffset = 0; nOffset < nCount; ++nOffset )
(*aCellIter++) = getCell( nIndex + nOffset, nRow );
}
rModel.AddUndo( std::make_unique<RemoveColUndo>( xThis, nIndex, aRemovedCols, aRemovedCells ) );
}
// only rows before and inside the removed rows are considered
@@ -758,6 +740,29 @@ void TableModel::removeColumns( sal_Int32 nIndex, sal_Int32 nCount )
}
}
// We must not add RemoveColUndo before we make cell spans correct, otherwise we
// get invalid cell span after undo.
if( bUndo )
{
TableModelRef xThis( this );
ColumnVector aRemovedCols( nCount );
sal_Int32 nOffset;
for( nOffset = 0; nOffset < nCount; ++nOffset )
{
aRemovedCols[nOffset] = maColumns[nIndex+nOffset];
}
CellVector aRemovedCells( nCount * nRows );
CellVector::iterator aCellIter( aRemovedCells.begin() );
for( sal_Int32 nRow = 0; nRow < nRows; ++nRow )
{
for( nOffset = 0; nOffset < nCount; ++nOffset )
(*aCellIter++) = getCell( nIndex + nOffset, nRow );
}
rModel.AddUndo( std::make_unique<RemoveColUndo>( xThis, nIndex, aRemovedCols, aRemovedCells ) );
}
// now remove the columns
remove_range<ColumnVector,ColumnVector::iterator>( maColumns, nIndex, nCount );
while( nRows-- )
@@ -862,14 +867,6 @@ void TableModel::removeRows( sal_Int32 nIndex, sal_Int32 nCount )
{
rModel.BegUndo( SvxResId(STR_UNDO_ROW_DELETE) );
rModel.AddUndo( rModel.GetSdrUndoFactory().CreateUndoGeoObject(*mpTableObj) );
TableModelRef xThis( this );
RowVector aRemovedRows( nCount );
for( sal_Int32 nOffset = 0; nOffset < nCount; ++nOffset )
aRemovedRows[nOffset] = maRows[nIndex+nOffset];
rModel.AddUndo( std::make_unique<RemoveRowUndo>( xThis, nIndex, aRemovedRows ) );
}
// only rows before and inside the removed rows are considered
@@ -916,6 +913,18 @@ void TableModel::removeRows( sal_Int32 nIndex, sal_Int32 nCount )
}
}
if( bUndo )
{
TableModelRef xThis( this );
RowVector aRemovedRows( nCount );
for( sal_Int32 nOffset = 0; nOffset < nCount; ++nOffset )
aRemovedRows[nOffset] = maRows[nIndex+nOffset];
// We must not RemoveRowUndo before we make cell spans correct, otherwise we
// get invalid cell span after undo.
rModel.AddUndo( std::make_unique<RemoveRowUndo>( xThis, nIndex, aRemovedRows ) );
}
// now remove the rows
remove_range<RowVector,RowVector::iterator>( maRows, nIndex, nCount );