sd/qa/unit/misc-tests.cxx       |   35 ++++++++++++++++++++++
 svx/source/table/tablemodel.cxx |   61 ++++++++++++++++++++++------------------
 2 files changed, 70 insertions(+), 26 deletions(-)

New commits:
commit eeeb3a422e25c22201e1325154f1913df3942f2a
Author:     Mark Hung <mark...@gmail.com>
AuthorDate: Thu Mar 11 23:44:06 2021 +0800
Commit:     Michael Stahl <michael.st...@allotropia.de>
CommitDate: Wed Mar 17 11:00:44 2021 +0100

    tdf#136956 reorder undo actions in removeColumns
    
    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 <mark...@gmail.com>
    (cherry picked from commit f3f7cc53efda828af8897fa45fa2a8f18cf3b48b)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/112526
    Reviewed-by: Michael Stahl <michael.st...@allotropia.de>

diff --git a/sd/qa/unit/misc-tests.cxx b/sd/qa/unit/misc-tests.cxx
index 52c3b55513be..75818805295a 100644
--- a/sd/qa/unit/misc-tests.cxx
+++ b/sd/qa/unit/misc-tests.cxx
@@ -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>
@@ -83,6 +85,7 @@ public:
     void testTdf130988();
     void testTdf131033();
     void testTdf129898LayerDrawnInSlideshow();
+    void testTdf136956();
 
     CPPUNIT_TEST_SUITE(SdMiscTest);
     CPPUNIT_TEST(testTdf96206);
@@ -104,6 +107,7 @@ public:
     CPPUNIT_TEST(testTdf130988);
     CPPUNIT_TEST(testTdf131033);
     CPPUNIT_TEST(testTdf129898LayerDrawnInSlideshow);
+    CPPUNIT_TEST(testTdf136956);
     CPPUNIT_TEST_SUITE_END();
 
 virtual void registerNamespaces(xmlXPathContextPtr& pXmlXPathCtx) override
@@ -875,6 +879,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();
diff --git a/svx/source/table/tablemodel.cxx b/svx/source/table/tablemodel.cxx
index d7d04191163c..993535a52ec7 100644
--- a/svx/source/table/tablemodel.cxx
+++ b/svx/source/table/tablemodel.cxx
@@ -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 );
 
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to