tdf#134709 writerfilter: consider gridAfter for table borders

When the table itself defines borders, those borders should
apply to the last cell in each row - even if some grids
are skipped with gridAfter. (i.e. it won't be a straight line
on the right side).

This commit depends on an earlier commit doing this for gridBefore.
Prior to this, gridAfter was never actually sent to DomainMapper,
so it was always at its initialized value of zero, and it was
only handled in the ooxml handlers.

So the bulk of this patch is setting up the foundational
parts of gridAfter - duplicating what was recently changed
in LO 7.1 for gridBefore.

Change-Id: Ieaf965fecf618eeaf41c7e8403b536c396138804
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/98692
Tested-by: Jenkins
Reviewed-by: Justin Luth <justin_luth@sil.org>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>

10 files changed, 58 insertions, 25 deletions

diff --git a/writerfilter/source/dmapper/DomainMapperTableHandler.cxx b/writerfilter/source/dmapper/DomainMapperTableHandler.cxx
index ac8c0f52eb71..ac667ddefd88 100644
--- a/writerfilter/source/dmapper/DomainMapperTableHandler.cxx
+++ b/writerfilter/source/dmapper/DomainMapperTableHandler.cxx
@@ -103,9 +103,10 @@ static void lcl_mergeBorder( PropertyIds nId, const PropertyMapPtr& pOrig, const
 }
 
 static void lcl_computeCellBorders( const PropertyMapPtr& pTableBorders, const PropertyMapPtr& pCellProps,
-        sal_uInt32 nCell, sal_uInt32 nFirstCell, sal_Int32 nRow, bool bIsEndCol, bool bIsEndRow, bool bMergedVertically )
+        sal_uInt32 nCell, sal_uInt32 nFirstCell, sal_uInt32 nLastCell, sal_Int32 nRow, bool bIsEndRow, bool bMergedVertically )
 {
     const bool bIsStartCol = nCell == nFirstCell;
+    const bool bIsEndCol = nCell == nLastCell;
     std::optional<PropertyMap::Property> pVerticalVal = pCellProps->getProperty(META_PROP_VERTICAL_BORDER);
     std::optional<PropertyMap::Property> pHorizontalVal = pCellProps->getProperty(META_PROP_HORIZONTAL_BORDER);
 
@@ -147,7 +148,7 @@ static void lcl_computeCellBorders( const PropertyMapPtr& pTableBorders, const P
     {
         if ( !bIsEndCol && nCell >= nFirstCell )
             pCellProps->Insert( PROP_RIGHT_BORDER, aVertProp, false );
-        if ( !bIsStartCol )
+        if ( !bIsStartCol && nCell <= nLastCell )
             pCellProps->Insert( PROP_LEFT_BORDER, aVertProp, false );
     }
 
@@ -752,7 +753,7 @@ 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)
+        // Merged cells (grid span) in a row will affect the actual column. (fake cells were added to handle gridBefore/After)
         sal_Int32 nCell = 0;
         pCellProperties[nRow].realloc( aRowOfCellsIterator->size() );
         beans::PropertyValues* pSingleCellProperties = pCellProperties[nRow].getArray();
@@ -930,7 +931,8 @@ CellPropertyValuesSeq_t DomainMapperTableHandler::endTableGetCellProperties(Tabl
                 }
 
                 const sal_uInt32 nFirstCell = m_rDMapper_Impl.getTableManager().getGridBefore(nRow);
-                lcl_computeCellBorders( rInfo.pTableBorders, *aCellIterator, nCell, nFirstCell, nRow, bIsEndCol, bIsEndRow, bMergedVertically );
+                const sal_uInt32 nLastCell = m_aCellProperties[nRow].size() - m_rDMapper_Impl.getTableManager().getGridAfter(nRow) - 1;
+                lcl_computeCellBorders( rInfo.pTableBorders, *aCellIterator, nCell, nFirstCell, nLastCell, nRow, bIsEndRow, bMergedVertically );
 
                 //now set the default left+right border distance TODO: there's an sprm containing the default distance!
                 aCellIterator->get()->Insert( PROP_LEFT_BORDER_DISTANCE,
diff --git a/writerfilter/source/dmapper/DomainMapperTableManager.cxx b/writerfilter/source/dmapper/DomainMapperTableManager.cxx
index a17a736504e4..cefe801145c5 100644
--- a/writerfilter/source/dmapper/DomainMapperTableManager.cxx
+++ b/writerfilter/source/dmapper/DomainMapperTableManager.cxx
@@ -40,7 +40,6 @@ DomainMapperTableManager::DomainMapperTableManager() :
     m_nRow(0),
     m_nCell(),
     m_nGridSpan(1),
-    m_nGridAfter(0),
     m_nHeaderRepeat(0),
     m_nTableWidth(0),
     m_bIsInShape(false),
@@ -356,7 +355,7 @@ bool DomainMapperTableManager::sprm(Sprm & rSprm)
                 setCurrentGridBefore( nIntValue );
                 break;
             case NS_ooxml::LN_CT_TrPrBase_gridAfter:
-                m_nGridAfter = nIntValue;
+                setCurrentGridAfter( nIntValue );
                 break;
             case NS_ooxml::LN_CT_TblPrBase_tblCaption:
                 // To-Do: Not yet preserved
@@ -811,7 +810,6 @@ void DomainMapperTableManager::endOfRowAction()
     getCurrentGrid()->clear();
     pCellWidths->clear();
 
-    m_nGridAfter = 0;
     m_bTableSizeTypeInserted = false;
 
 #ifdef DBG_UTIL
diff --git a/writerfilter/source/dmapper/DomainMapperTableManager.hxx b/writerfilter/source/dmapper/DomainMapperTableManager.hxx
index 14133b276359..1eb61b6df309 100644
--- a/writerfilter/source/dmapper/DomainMapperTableManager.hxx
+++ b/writerfilter/source/dmapper/DomainMapperTableManager.hxx
@@ -39,7 +39,6 @@ class DomainMapperTableManager : public TableManager
     sal_uInt32      m_nRow;
     ::std::vector< sal_uInt32 > m_nCell;
     sal_uInt32      m_nGridSpan;
-    sal_uInt32      m_nGridAfter; ///< number of grid columns in the parent table's table grid which shall be left after the last cell in the table row
     sal_Int32       m_nHeaderRepeat; //counter of repeated headers - if == -1 then the repeating stops
     sal_Int32       m_nTableWidth; //might be set directly or has to be calculated from the column positions
     /// Are we in a shape (text append stack is not empty) or in the body document?
diff --git a/writerfilter/source/dmapper/TableData.hxx b/writerfilter/source/dmapper/TableData.hxx
index a88c6a477430..a7b68dfd8130 100644
--- a/writerfilter/source/dmapper/TableData.hxx
+++ b/writerfilter/source/dmapper/TableData.hxx
@@ -121,18 +121,21 @@ class RowData final : public virtual SvRefBase
     mutable TablePropertyMapPtr mpProperties;
 
     sal_uInt32 m_nGridBefore; ///< number of grid columns in the parent table's table grid which must be skipped before the contents of this table row are added to the parent table
+    sal_uInt32 m_nGridAfter; ///< number of grid columns in the parent table's table grid which shall be left after the last cell in the table row
 
 public:
     typedef tools::SvRef<RowData> Pointer_t;
 
     RowData()
         : m_nGridBefore(0)
+        , m_nGridAfter(0)
     {
     }
 
     RowData(const RowData& rRowData)
     : SvRefBase(), mCells(rRowData.mCells), mpProperties(rRowData.mpProperties)
         , m_nGridBefore(rRowData.m_nGridBefore)
+        , m_nGridAfter(rRowData.m_nGridAfter)
     {
     }
 
@@ -240,6 +243,8 @@ public:
 
     sal_uInt32 getGridBefore() { return m_nGridBefore; }
     void setGridBefore(sal_uInt32 nSkipGrids) { m_nGridBefore = nSkipGrids; }
+    sal_uInt32 getGridAfter() { return m_nGridAfter; }
+    void setGridAfter(sal_uInt32 nSkipGrids) { m_nGridAfter = nSkipGrids; }
     sal_uInt32 getGridSpan(sal_uInt32 i) { return mCells[i]->getGridSpan(); }
     std::vector< sal_uInt32 > getGridSpans()
     {
diff --git a/writerfilter/source/dmapper/TableManager.cxx b/writerfilter/source/dmapper/TableManager.cxx
index 8b49fbe509e0..61a9965227eb 100644
--- a/writerfilter/source/dmapper/TableManager.cxx
+++ b/writerfilter/source/dmapper/TableManager.cxx
@@ -67,6 +67,20 @@ void TableManager::setCurrentGridBefore(sal_uInt32 nSkipGrids)
     mTableDataStack.top()->getCurrentRow()->setGridBefore(nSkipGrids);
 }
 
+sal_uInt32 TableManager::getGridAfter(sal_uInt32 nRow)
+{
+    assert(isInTable());
+    if (nRow >= mTableDataStack.top()->getRowCount())
+        return 0;
+    return mTableDataStack.top()->getRow(nRow)->getGridAfter();
+}
+
+void TableManager::setCurrentGridAfter(sal_uInt32 nSkipGrids)
+{
+    assert(isInTable());
+    mTableDataStack.top()->getCurrentRow()->setGridAfter(nSkipGrids);
+}
+
 std::vector<sal_uInt32> TableManager::getCurrentGridSpans()
 {
     return mTableDataStack.top()->getCurrentRow()->getGridSpans();
@@ -80,8 +94,11 @@ void TableManager::setCurrentGridSpan(sal_uInt32 nGridSpan, bool bFirstCell)
 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())
+    if (!pRow || nCell < pRow->getGridBefore()
+        || nCell >= pRow->getCellCount() - pRow->getGridAfter())
+    {
         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.
@@ -100,6 +117,7 @@ sal_uInt32 TableManager::findColumnCell(const sal_uInt32 nRow, const sal_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.
+    const sal_uInt32 nMaxCell = pRow->getCellCount() - pRow->getGridAfter() - 1;
     for (const auto& rSpan : pRow->getGridSpans())
     {
         nGrids += rSpan;
@@ -107,6 +125,8 @@ sal_uInt32 TableManager::findColumnCell(const sal_uInt32 nRow, const sal_uInt32
             return nCell;
 
         ++nCell;
+        if (nCell > nMaxCell)
+            break;
     }
     return SAL_MAX_UINT32; // must be in gridAfter or invalid column request
 }
@@ -444,6 +464,7 @@ void TableManager::startLevel()
             pTableData2->getCurrentRow()->setCurrentGridSpan(mpUnfinishedRow->getGridSpan(i));
         }
         pTableData2->getCurrentRow()->setGridBefore(mpUnfinishedRow->getGridBefore());
+        pTableData2->getCurrentRow()->setGridAfter(mpUnfinishedRow->getGridAfter());
         mpUnfinishedRow.clear();
     }
 
diff --git a/writerfilter/source/dmapper/TableManager.hxx b/writerfilter/source/dmapper/TableManager.hxx
index 234ba63f86bd..2dcf679e135f 100644
--- a/writerfilter/source/dmapper/TableManager.hxx
+++ b/writerfilter/source/dmapper/TableManager.hxx
@@ -501,6 +501,8 @@ public:
     sal_uInt32 getGridBefore(sal_uInt32 nRow);
     sal_uInt32 getCurrentGridBefore();
     void setCurrentGridBefore( sal_uInt32 nSkipGrids );
+    sal_uInt32 getGridAfter(sal_uInt32 nRow);
+    void setCurrentGridAfter( sal_uInt32 nSkipGrids );
     std::vector<sal_uInt32> getCurrentGridSpans();
     void setCurrentGridSpan( sal_uInt32 nGridSpan, bool bFirstCell = false );
     /// Given a zero-based row/cell, return the zero-based grid it belongs to, or SAL_MAX_UINT16 for invalid.
diff --git a/writerfilter/source/ooxml/OOXMLFastContextHandler.cxx b/writerfilter/source/ooxml/OOXMLFastContextHandler.cxx
index 38d81ec25a8e..4cdcecb49b12 100644
--- a/writerfilter/source/ooxml/OOXMLFastContextHandler.cxx
+++ b/writerfilter/source/ooxml/OOXMLFastContextHandler.cxx
@@ -1244,6 +1244,20 @@ void OOXMLFastContextHandlerValue::popBiDiEmbedLevel()
     OOXMLFactory::characters(this, u"\u202C"); // PDF (POP DIRECTIONAL FORMATTING)
 }
 
+void OOXMLFastContextHandlerValue::handleGridAfter()
+{
+    if (!getValue())
+        return;
+
+    if (OOXMLFastContextHandler* pTableRowProperties = getParent())
+    {
+        if (OOXMLFastContextHandler* pTableRow = pTableRowProperties->getParent())
+            // Save the value into the table row context, so it can be handled
+            // right before the end of the row.
+            pTableRow->setGridAfter(getValue());
+    }
+}
+
 /*
   class OOXMLFastContextHandlerTable
 */
@@ -1461,17 +1475,6 @@ void OOXMLFastContextHandlerTextTableRow::endRow()
     endParagraphGroup();
 }
 
-void OOXMLFastContextHandlerTextTableRow::handleGridAfter(const OOXMLValue::Pointer_t& rValue)
-{
-    if (OOXMLFastContextHandler* pTableRowProperties = getParent())
-    {
-        if (OOXMLFastContextHandler* pTableRow = pTableRowProperties->getParent())
-            // Save the value into the table row context, so it can be handled
-            // right before the end of the row.
-            pTableRow->setGridAfter(rValue);
-    }
-}
-
 namespace {
 OOXMLValue::Pointer_t fakeNoBorder()
 {
diff --git a/writerfilter/source/ooxml/OOXMLFastContextHandler.hxx b/writerfilter/source/ooxml/OOXMLFastContextHandler.hxx
index 0d797226d101..1dd93f82de5c 100644
--- a/writerfilter/source/ooxml/OOXMLFastContextHandler.hxx
+++ b/writerfilter/source/ooxml/OOXMLFastContextHandler.hxx
@@ -345,6 +345,8 @@ public:
     virtual void pushBiDiEmbedLevel() override;
     virtual void popBiDiEmbedLevel() override;
 
+    void handleGridAfter();
+
 private:
     OOXMLValue::Pointer_t mpValue;
 };
@@ -419,7 +421,6 @@ public:
     static void startRow();
     void endRow();
     void handleGridBefore( const OOXMLValue::Pointer_t& val );
-    void handleGridAfter(const OOXMLValue::Pointer_t& rValue);
 };
 
 class OOXMLFastContextHandlerTextTable : public OOXMLFastContextHandler
diff --git a/writerfilter/source/ooxml/factoryimpl_ns.py b/writerfilter/source/ooxml/factoryimpl_ns.py
index c36e0a896d32..52ccf36fdc17 100644
--- a/writerfilter/source/ooxml/factoryimpl_ns.py
+++ b/writerfilter/source/ooxml/factoryimpl_ns.py
@@ -440,8 +440,8 @@ def factoryChooseAction(actionNode):
         ret.append("    %sif (OOXMLFastContextHandlerTextTableRow* pTextTableRow = dynamic_cast<OOXMLFastContextHandlerTextTableRow*>(pHandler))" % extra_space)
         ret.append("    %s    pTextTableRow->%s();" % (extra_space, actionNode.getAttribute("action")))
     elif actionNode.getAttribute("action") == "handleGridAfter":
-        ret.append("    %sif (OOXMLFastContextHandlerTextTableRow* pTextTableRow = dynamic_cast<OOXMLFastContextHandlerTextTableRow*>(pHandler))" % extra_space)
-        ret.append("    %s    pTextTableRow->%s();" % (extra_space, actionNode.getAttribute("action")))
+        ret.append("    %sif (OOXMLFastContextHandlerValue* pValueHandler = dynamic_cast<OOXMLFastContextHandlerValue*>(pHandler))" % extra_space)
+        ret.append("    %s    pValueHandler->%s();" % (extra_space, actionNode.getAttribute("action")))
     # tdf#111550
     elif actionNode.getAttribute("action") in ("start_P_Tbl"):
         ret.append("    %sif (OOXMLFastContextHandlerTextTable* pTextTable = dynamic_cast<OOXMLFastContextHandlerTextTable*>(pHandler))" % extra_space)
diff --git a/writerfilter/source/ooxml/model.xml b/writerfilter/source/ooxml/model.xml
index 4ae5870386b6..4f0ba4a7c432 100644
--- a/writerfilter/source/ooxml/model.xml
+++ b/writerfilter/source/ooxml/model.xml
@@ -18416,6 +18416,7 @@
       <element name="cnfStyle" tokenid="ooxml:CT_TrPrBase_cnfStyle"/>
       <element name="divId" tokenid="ooxml:CT_TrPrBase_divId"/>
       <element name="gridBefore" tokenid="ooxml:CT_TrPrBase_gridBefore"/>
+      <element name="gridAfter" tokenid="ooxml:CT_TrPrBase_gridAfter"/>
       <element name="wBefore" tokenid="ooxml:CT_TrPrBase_wBefore"/>
       <element name="wAfter" tokenid="ooxml:CT_TrPrBase_wAfter"/>
       <element name="cantSplit" tokenid="ooxml:CT_TrPrBase_cantSplit"/>
@@ -18425,8 +18426,9 @@
       <element name="jc" tokenid="ooxml:CT_TrPrBase_jc"/>
       <element name="hidden" tokenid="ooxml:CT_TrPrBase_hidden"/>
     </resource>
-    <resource name="CT_TrPrBaseGridAfter" resource="TextTableRow">
-      <attribute name="val" tokenid="ooxml:CT_TrPrBase_gridAfter" action="handleGridAfter"/>
+    <resource name="CT_TrPrBaseGridAfter" resource="Value">
+      <attribute name="val" tokenid="ooxml:CT_TrPrBase_gridAfter" action="setValue"/>
+      <action name="end" action="handleGridAfter"/>
     </resource>
     <resource name="CT_TrPr" resource="Properties">
       <element name="ins" tokenid="ooxml:CT_TrPr_ins"/>