tdf#123090 Handle removed column with gridSpan.

This is a combination of 3 commits.
(cherry picked from commit 48ef20f2039d1a300a4324072e9b712c9994b406)
(cherry picked from commit 00e89430a2f8cd1f9ec702a7583a1e4c886a2b46)
(cherry picked from commit 1f0206d940cd8f7fb627a59cfe4165c0bfebaf46)

Change-Id: Ic6fa6f335623e2114fc8bea76dc54833284d2a02
Reviewed-on: https://gerrit.libreoffice.org/68154
Tested-by: Jenkins
Reviewed-by: Andras Timar <andras.timar@collabora.com>

5 files changed, 91 insertions, 19 deletions

diff --git a/oox/source/drawingml/table/tableproperties.cxx b/oox/source/drawingml/table/tableproperties.cxx
index 5f1495166b39..128668f012c4 100644
--- a/oox/source/drawingml/table/tableproperties.cxx
+++ b/oox/source/drawingml/table/tableproperties.cxx
@@ -273,33 +273,76 @@ const TableStyle& TableProperties::getUsedTableStyle( const ::oox::core::XmlFilt
     return *pTableStyle;
 }
 
-void TableProperties::pushToPropSet( const ::oox::core::XmlFilterBase& rFilterBase,
-    const Reference < XPropertySet >& xPropSet, const TextListStylePtr& pMasterTextListStyle )
+void TableProperties::pushToPropSet(const ::oox::core::XmlFilterBase& rFilterBase,
+                                    const Reference<XPropertySet>& xPropSet,
+                                    const TextListStylePtr& pMasterTextListStyle)
 {
-    uno::Reference< XColumnRowRange > xColumnRowRange(
-         xPropSet->getPropertyValue("Model"), uno::UNO_QUERY_THROW );
+    uno::Reference<XColumnRowRange> xColumnRowRange(xPropSet->getPropertyValue("Model"),
+                                                    uno::UNO_QUERY_THROW);
 
-    CreateTableColumns( xColumnRowRange->getColumns(), mvTableGrid );
-    CreateTableRows( xColumnRowRange->getRows(), mvTableRows );
+    CreateTableColumns(xColumnRowRange->getColumns(), mvTableGrid);
+    CreateTableRows(xColumnRowRange->getRows(), mvTableRows);
 
     TableStyle* pTableStyleToDelete = nullptr;
-    const TableStyle& rTableStyle( getUsedTableStyle( rFilterBase, pTableStyleToDelete ) );
+    const TableStyle& rTableStyle(getUsedTableStyle(rFilterBase, pTableStyleToDelete));
     sal_Int32 nRow = 0;
-    for (auto & tableRow : mvTableRows)
+
+    for (auto& tableRow : mvTableRows)
     {
         sal_Int32 nColumn = 0;
-        for (auto & tableCell : tableRow.getTableCells())
+        sal_Int32 nColumnSize = tableRow.getTableCells().size();
+        sal_Int32 nRemovedColumn = 0; //
+
+        for (sal_Int32 nColIndex = 0; nColIndex < nColumnSize; nColIndex++)
         {
-            TableCell& rTableCell(tableCell);
-            if ( !rTableCell.getvMerge() && !rTableCell.gethMerge() )
+            TableCell& rTableCell(tableRow.getTableCells().at(nColIndex));
+
+            if (!rTableCell.getvMerge() && !rTableCell.gethMerge())
             {
-                uno::Reference< XTable > xTable( xColumnRowRange, uno::UNO_QUERY_THROW );
-                if ( ( rTableCell.getRowSpan() > 1 ) || ( rTableCell.getGridSpan() > 1 ) )
-                    MergeCells( xTable, nColumn, nRow, rTableCell.getGridSpan(), rTableCell.getRowSpan() );
+                uno::Reference<XTable> xTable(xColumnRowRange, uno::UNO_QUERY_THROW);
+                bool bMerged = false;
 
-                Reference< XCellRange > xCellRange( xTable, UNO_QUERY_THROW );
-                rTableCell.pushToXCell( rFilterBase, pMasterTextListStyle, xCellRange->getCellByPosition( nColumn, nRow ), *this, rTableStyle,
-                    nColumn, tableRow.getTableCells().size()-1, nRow, mvTableRows.size()-1 );
+                if ((rTableCell.getRowSpan() > 1) || (rTableCell.getGridSpan() > 1))
+                {
+                    MergeCells(xTable, nColumn, nRow, rTableCell.getGridSpan(),
+                               rTableCell.getRowSpan());
+
+                    if (rTableCell.getGridSpan() > 1)
+                    {
+                        nRemovedColumn = (rTableCell.getGridSpan() - 1);
+                        // MergeCells removes columns. Our loop does not know about those
+                        // removed columns and we skip handling those removed columns.
+                        nColIndex += nRemovedColumn;
+                        // It will adjust new column number after push current column's
+                        // props with pushToXCell.
+                        bMerged = true;
+                    }
+                }
+
+                Reference<XCellRange> xCellRange(xTable, UNO_QUERY_THROW);
+                Reference<XCell> xCell;
+
+                if (nRemovedColumn)
+                {
+                    try
+                    {
+                        xCell = xCellRange->getCellByPosition(nColumn, nRow);
+                    }
+                    // Exception can come from TableModel::getCellByPosition when a column
+                    // is removed while merging columns. So adjust again here.
+                    catch (Exception&)
+                    {
+                        xCell = xCellRange->getCellByPosition(nColumn - nRemovedColumn, nRow);
+                    }
+                }
+                else
+                    xCell = xCellRange->getCellByPosition(nColumn, nRow);
+
+                rTableCell.pushToXCell(rFilterBase, pMasterTextListStyle, xCell, *this, rTableStyle,
+                                       nColumn, tableRow.getTableCells().size() - 1, nRow,
+                                       mvTableRows.size() - 1);
+                if (bMerged)
+                    nColumn += nRemovedColumn;
             }
             ++nColumn;
         }
diff --git a/sd/qa/unit/data/pptx/tdf123090.pptx b/sd/qa/unit/data/pptx/tdf123090.pptx
new file mode 100644
index 000000000000..c219f52de0a2
--- /dev/null
+++ b/sd/qa/unit/data/pptx/tdf123090.pptx
diff --git a/sd/qa/unit/import-tests.cxx b/sd/qa/unit/import-tests.cxx
index 2ef5faebe78b..2f506414f920 100644
--- a/sd/qa/unit/import-tests.cxx
+++ b/sd/qa/unit/import-tests.cxx
@@ -188,6 +188,7 @@ public:
     void testPatternImport();
     void testPptCrop();
     void testTdf119015();
+    void testTdf123090();
     void testTdf120028();
     void testTdf120028b();
 
@@ -272,6 +273,7 @@ public:
     CPPUNIT_TEST(testTdf116266);
     CPPUNIT_TEST(testPptCrop);
     CPPUNIT_TEST(testTdf119015);
+    CPPUNIT_TEST(testTdf123090);
     CPPUNIT_TEST(testTdf120028);
     CPPUNIT_TEST(testTdf120028b);
 
@@ -2554,13 +2556,40 @@ void SdImportTest::testTdf119015()
     uno::Reference<table::XTable> xTable(pTableObj->getTable());
 
     // Test that we actually have three cells: this threw css.lang.IndexOutOfBoundsException
-    uno::Reference<text::XTextRange> xTextRange(xTable->getCellByPosition(2, 0),
+    uno::Reference<text::XTextRange> xTextRange(xTable->getCellByPosition(1, 0),
                                                 uno::UNO_QUERY_THROW);
     CPPUNIT_ASSERT_EQUAL(OUString("A3"), xTextRange->getString());
 
     xDocShRef->DoClose();
 }
 
+void SdImportTest::testTdf123090()
+{
+    ::sd::DrawDocShellRef xDocShRef
+        = loadURL(m_directories.getURLFromSrc("/sd/qa/unit/data/pptx/tdf123090.pptx"), PPTX);
+
+    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());
+
+    // Test that we actually have two cells: this threw css.lang.IndexOutOfBoundsException
+    uno::Reference<text::XTextRange> xTextRange(xTable->getCellByPosition(1, 0),
+                                                uno::UNO_QUERY_THROW);
+    CPPUNIT_ASSERT_EQUAL(OUString("aaa"), xTextRange->getString());
+
+    sal_Int32 nWidth;
+    const OUString sWidth("Width");
+    uno::Reference< css::table::XTableColumns > xColumns( xTable->getColumns(), uno::UNO_QUERY_THROW);
+    uno::Reference< beans::XPropertySet > xRefColumn( xColumns->getByIndex(1), uno::UNO_QUERY_THROW );
+    xRefColumn->getPropertyValue( sWidth ) >>= nWidth;
+    CPPUNIT_ASSERT_EQUAL( sal_Int32(9136), nWidth);
+
+    xDocShRef->DoClose();
+}
+
 void SdImportTest::testTdf120028()
 {
     // Check that the table shape has 4 columns.
diff --git a/svx/source/table/cellcursor.cxx b/svx/source/table/cellcursor.cxx
index bdd7156ef669..d77dbb3ffc59 100644
--- a/svx/source/table/cellcursor.cxx
+++ b/svx/source/table/cellcursor.cxx
@@ -251,6 +251,7 @@ void SAL_CALL CellCursor::merge(  )
     try
     {
         mxTable->merge( aStart.mnCol, aStart.mnRow, aEnd.mnCol - aStart.mnCol + 1, aEnd.mnRow - aStart.mnRow + 1 );
+        mxTable->optimize();
         mxTable->setModified(true);
     }
     catch( Exception& )
diff --git a/svx/source/table/tablecontroller.cxx b/svx/source/table/tablecontroller.cxx
index 6d7a5ec37b55..284493ac78fd 100644
--- a/svx/source/table/tablecontroller.cxx
+++ b/svx/source/table/tablecontroller.cxx
@@ -1821,7 +1821,6 @@ void SvxTableController::MergeRange( sal_Int32 nFirstCol, sal_Int32 nFirstRow, s
             }
 
             xRange->merge();
-            mxTable->optimize();
             mbHasJustMerged = true;
             setSelectedCells( maCursorFirstPos, maCursorFirstPos );