tdf#129452 writerfilter: use column, not cell when comparing rows

A natural mistake - especially when there are no existing
functions - is to assume that cell number defines column
number, and so it is valid to compare between rows.

This patch provides the functions, and switches this bug's
earlier patch to properly use columns instead of cells.

This commit depends on two prior patches in bug 129452:
  Change-Id: Ie305477f0e3468a4a923095d76f520d97fe99ffe
  Change-Id: Ibfdac336bbb1f7303c7e585a85c94be37ad6f916

I hope that this implementation covers all the bases.
This code is dreadful to understand,
as witnessed by comments from those much smarter than I.

P.S. I also cancelled a vertical merge if the cell
is not there (in a gridAfter situation).
If a row is shorter than the previous ones,
then a vertically merged cell should not be considered
to be able to span that gap. If the cells below are still
merged, it would be considered a new merged cell.

Change-Id: I63158ac73b1bf86d9f75dd3c1299d1b1a3f08064
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/97434
Tested-by: Justin Luth <justin_luth@sil.org>
Tested-by: László Németh <nemeth@numbertext.org>
Reviewed-by: Justin Luth <justin_luth@sil.org>
Reviewed-by: László Németh <nemeth@numbertext.org>

3 files changed, 56 insertions, 4 deletions

diff --git a/writerfilter/source/dmapper/DomainMapperTableHandler.cxx b/writerfilter/source/dmapper/DomainMapperTableHandler.cxx
index 7da44a475efd..901fdfc0c115 100644
--- a/writerfilter/source/dmapper/DomainMapperTableHandler.cxx
+++ b/writerfilter/source/dmapper/DomainMapperTableHandler.cxx
@@ -758,6 +758,9 @@ CellPropertyValuesSeq_t DomainMapperTableHandler::endTableGetCellProperties(Tabl
             }
         }
 
+        // Note that this is intentionally called "cell" and not "column".
+        // Don't make the mistake that all cell x's will be in the same column.
+        // Merged cells (grid span) in a row will affect the actual column. (fake cells were added to handle gridBefore)
         sal_Int32 nCell = 0;
         pCellProperties[nRow].realloc( aRowOfCellsIterator->size() );
         beans::PropertyValues* pSingleCellProperties = pCellProperties[nRow].getArray();
@@ -905,10 +908,20 @@ CellPropertyValuesSeq_t DomainMapperTableHandler::endTableGetCellProperties(Tabl
                 // tdf#129452 Checking if current cell is vertically merged with all the other cells below to the bottom.
                 // This must be done in order to apply the bottom border of the table to the first cell in a vertical merge.
                 bool bMergedVertically = bool(m_aCellProperties[nRow][nCell]->getProperty(PROP_VERTICAL_MERGE));
-
-                for (size_t i = nRow + 1; bMergedVertically && i < m_aCellProperties.size(); i++)
-                    if ( m_aCellProperties[i].size() > sal::static_int_cast<std::size_t>(nCell) )
-                        bMergedVertically = bool(m_aCellProperties[i][nCell]->getProperty(PROP_VERTICAL_MERGE));
+                if ( bMergedVertically )
+                {
+                    const sal_uInt32 nColumn = m_rDMapper_Impl.getTableManager().findColumn(nRow, nCell);
+                    for (size_t i = nRow + 1; bMergedVertically && i < m_aCellProperties.size(); i++)
+                    {
+                        const sal_uInt32 nColumnCell = m_rDMapper_Impl.getTableManager().findColumnCell(i, nColumn);
+                        if ( m_aCellProperties[i].size() > sal::static_int_cast<std::size_t>(nColumnCell) )
+                        {
+                            bMergedVertically = bool(m_aCellProperties[i][nColumnCell]->getProperty(PROP_VERTICAL_MERGE));
+                        }
+                        else
+                            bMergedVertically = false;
+                    }
+                }
 
                 lcl_computeCellBorders( rInfo.pTableBorders, *aCellIterator, nCell, nRow, bIsEndCol, bIsEndRow, bMergedVertically );
 
@@ -1231,6 +1244,7 @@ void DomainMapperTableHandler::endTable(unsigned int nestedTableLevel, bool bTab
                 TableParagraphVectorPtr pTableParagraphs = m_rDMapper_Impl.getTableManager().getCurrentParagraphs();
                 for (size_t nRow = 0; nRow < m_aTableRanges.size(); ++nRow)
                 {
+                    // Note that this is "cell" since you must not treat it as "column".
                     for (size_t nCell = 0; nCell < m_aTableRanges[nRow].size(); ++nCell)
                     {
                         auto rStartPara = m_aTableRanges[nRow][nCell][0];
diff --git a/writerfilter/source/dmapper/TableManager.cxx b/writerfilter/source/dmapper/TableManager.cxx
index 582b4d4705a6..2649ee1120e2 100644
--- a/writerfilter/source/dmapper/TableManager.cxx
+++ b/writerfilter/source/dmapper/TableManager.cxx
@@ -69,6 +69,40 @@ void TableManager::setCurrentGridSpan(sal_uInt32 nGridSpan)
     mTableDataStack.top()->getCurrentRow()->setCurrentGridSpan(nGridSpan);
 }
 
+sal_uInt32 TableManager::findColumn(const sal_uInt32 nRow, const sal_uInt32 nCell)
+{
+    RowData::Pointer_t pRow = mTableDataStack.top()->getRow(nRow);
+    if (!pRow || nCell < pRow->getGridBefore() || nCell >= pRow->getCellCount())
+        return SAL_MAX_UINT32;
+
+    // The gridSpans provide a one-based index, so add up all the spans of the PREVIOUS columns,
+    // and that result will provide the first possible zero-based number for the desired column.
+    sal_uInt32 nColumn = 0;
+    for (sal_uInt32 n = 0; n < nCell; ++n)
+        nColumn += pRow->getGridSpan(n);
+    return nColumn;
+}
+
+sal_uInt32 TableManager::findColumnCell(const sal_uInt32 nRow, const sal_uInt32 nCol)
+{
+    RowData::Pointer_t pRow = mTableDataStack.top()->getRow(nRow);
+    if (!pRow || nCol < pRow->getGridBefore())
+        return SAL_MAX_UINT32;
+
+    sal_uInt32 nCell = 0;
+    sal_uInt32 nGrids = 0;
+    // The gridSpans give us a one-based index, but requested column is zero-based - so keep that in mind.
+    for (const auto& rSpan : pRow->getGridSpans())
+    {
+        nGrids += rSpan;
+        if (nCol < nGrids)
+            return nCell;
+
+        ++nCell;
+    }
+    return SAL_MAX_UINT32; // must be in gridAfter or invalid column request
+}
+
 void TableManager::endOfRowAction() {}
 
 void TableManager::endOfCellAction() {}
diff --git a/writerfilter/source/dmapper/TableManager.hxx b/writerfilter/source/dmapper/TableManager.hxx
index d1c6ee431bc8..1cc7caaf958e 100644
--- a/writerfilter/source/dmapper/TableManager.hxx
+++ b/writerfilter/source/dmapper/TableManager.hxx
@@ -504,6 +504,10 @@ public:
     void setCurrentGridBefore( sal_uInt32 nSkipGrids );
     std::vector<sal_uInt32> getCurrentGridSpans();
     void setCurrentGridSpan( sal_uInt32 nGridSpan );
+    /// Given a zero-based row/cell, return the zero-based grid it belongs to, or SAL_MAX_UINT16 for invalid.
+    sal_uInt32 findColumn( const sal_uInt32 nRow, const sal_uInt32 nCell );
+    /// Given a zero-based row/col, return the zero-based cell describing that grid, or SAL_MAX_UINT16 for invalid.
+    sal_uInt32 findColumnCell( const sal_uInt32 nRow, const sal_uInt32 nCol );
 
     void setTableStartsAtCellStart(bool bTableStartsAtCellStart);
     void setCellLastParaAfterAutospacing(bool bIsAfterAutospacing);