[Libreoffice-commits] core.git: Branch 'distro/collabora/cp-6.2' - sw/qa writerfilter/source

László Németh (via logerrit) logerrit at kemper.freedesktop.org
Tue Jun 23 17:46:45 UTC 2020


 sw/qa/extras/ooxmlexport/data/tdf116194.docx             |binary
 sw/qa/extras/ooxmlexport/ooxmlexport10.cxx               |    9 +++
 writerfilter/source/dmapper/DomainMapperTableManager.cxx |   37 ++++++++++-----
 writerfilter/source/dmapper/DomainMapperTableManager.hxx |    3 -
 writerfilter/source/dmapper/TableData.hxx                |   11 +++-
 writerfilter/source/dmapper/TableManager.cxx             |   20 ++++++++
 writerfilter/source/ooxml/OOXMLFastContextHandler.cxx    |    4 +
 writerfilter/source/ooxml/model.xml                      |    5 +-
 8 files changed, 74 insertions(+), 15 deletions(-)

New commits:
commit 489453ad71a710fb5896003f815d78db9c09c598
Author:     László Németh <nemeth at numbertext.org>
AuthorDate: Mon Dec 2 19:02:43 2019 +0100
Commit:     Andras Timar <andras.timar at collabora.com>
CommitDate: Tue Jun 23 19:46:13 2020 +0200

    tdf#116194 DOCX import: fix missing tables with w:gridBefore
    
    Regression from the commit cf33af732ed0d3d553bb74636e3b14c55d44c153
    "handle w:gridBefore by faking cells (fdo#38414)"
    
    This patch replaces the previous fix with a better solution,
    fixing tdf#38414 on the proposed DomainMapper level. (Note:
    to reject the old fix completely, its follow-up commit w:gridAfter
    will be handled in a similar way.)
    
    Now the related regressions, tdf#111679, tdf#120512 and the complex
    forms of tdf#116194, tdf120256 and tdf#122608 are fixed, too.
    
    Reviewed-on: https://gerrit.libreoffice.org/84263
    Reviewed-by: László Németh <nemeth at numbertext.org>
    Tested-by: László Németh <nemeth at numbertext.org>
    (cherry picked from commit da1f71edfc72928b07a569b98e2766a8a7de9d2a)
    Reviewed-on: https://gerrit.libreoffice.org/84711
    Tested-by: Jenkins
    
    Change-Id: Id25f5fb4d9021c87ee8c82782b2038e6fb255673
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/96951
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
    Reviewed-by: Andras Timar <andras.timar at collabora.com>

diff --git a/sw/qa/extras/ooxmlexport/data/tdf116194.docx b/sw/qa/extras/ooxmlexport/data/tdf116194.docx
new file mode 100644
index 000000000000..feec3ee9870f
Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/tdf116194.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx
index 79375ec5f0ac..a3a9ccf63778 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx
@@ -571,6 +571,15 @@ DECLARE_OOXMLEXPORT_TEST(testGridBefore, "gridbefore.docx")
     CPPUNIT_ASSERT( leftA3.toInt32() > leftB2.toInt32());
 }
 
+DECLARE_OOXMLEXPORT_TEST(testTdf116194, "tdf116194.docx")
+{
+    // The problem was that the importer lost consecutive tables with w:gridBefore
+    xmlDocPtr pXmlDoc = parseExport();
+    if (!pXmlDoc)
+        return;
+    assertXPath(pXmlDoc, "/w:document/w:body/w:tbl", 2);
+}
+
 DECLARE_OOXMLEXPORT_TEST(testMsoBrightnessContrast, "msobrightnesscontrast.docx")
 {
     uno::Reference<text::XTextDocument> textDocument(mxComponent, uno::UNO_QUERY);
diff --git a/writerfilter/source/dmapper/DomainMapperTableManager.cxx b/writerfilter/source/dmapper/DomainMapperTableManager.cxx
index 4eaa51823c24..407d3c36c17c 100644
--- a/writerfilter/source/dmapper/DomainMapperTableManager.cxx
+++ b/writerfilter/source/dmapper/DomainMapperTableManager.cxx
@@ -47,7 +47,7 @@ DomainMapperTableManager::DomainMapperTableManager() :
     m_nRow(0),
     m_nCell(),
     m_nGridSpan(1),
-    m_nGridBefore(0),
+    m_aGridBefore(),
     m_nGridAfter(0),
     m_nHeaderRepeat(0),
     m_nTableWidth(0),
@@ -347,7 +347,7 @@ bool DomainMapperTableManager::sprm(Sprm & rSprm)
                 }
                 break;
             case NS_ooxml::LN_CT_TrPrBase_gridBefore:
-                m_nGridBefore = nIntValue;
+                m_aGridBefore.back( ) = nIntValue;
                 break;
             case NS_ooxml::LN_CT_TrPrBase_gridAfter:
                 m_nGridAfter = nIntValue;
@@ -387,6 +387,11 @@ std::shared_ptr< vector<sal_Int32> > const & DomainMapperTableManager::getCurren
     return m_aTableGrid.back( );
 }
 
+sal_uInt32 DomainMapperTableManager::getCurrentGridBefore( )
+{
+    return m_aGridBefore.back( );
+}
+
 bool DomainMapperTableManager::hasCurrentSpans() const
 {
     return !m_aGridSpans.empty();
@@ -449,6 +454,7 @@ void DomainMapperTableManager::startLevel( )
     m_aTmpPosition.push_back( pTmpPosition );
     m_aTmpTableProperties.push_back( pTmpProperties );
     m_nCell.push_back( 0 );
+    m_aGridBefore.push_back( 0 );
     m_nTableWidth = 0;
     m_nLayoutType = 0;
 
@@ -478,6 +484,7 @@ void DomainMapperTableManager::endLevel( )
         m_aCellWidths.back()->push_back(*oCurrentWidth);
 
     m_nCell.pop_back( );
+    m_aGridBefore.pop_back( );
     m_nTableWidth = 0;
     m_nLayoutType = 0;
 
@@ -533,6 +540,7 @@ void DomainMapperTableManager::endOfRowAction()
         IntVectorPtr pTmpGridSpans = m_aGridSpans.back();
         IntVectorPtr pTmpCellWidths = m_aCellWidths.back();
         sal_uInt32 nTmpCell = m_nCell.back();
+        sal_uInt32 nTmpGridBefore = m_aGridBefore.back();
 
         // endLevel and startLevel are taking care of the non finished row
         // to carry it over to the next table
@@ -545,10 +553,12 @@ void DomainMapperTableManager::endOfRowAction()
         m_aGridSpans.pop_back();
         m_aCellWidths.pop_back();
         m_nCell.pop_back();
+        m_aGridBefore.pop_back();
         m_aTableGrid.push_back(pTmpTableGrid);
         m_aGridSpans.push_back(pTmpGridSpans);
         m_aCellWidths.push_back(pTmpCellWidths);
         m_nCell.push_back(nTmpCell);
+        m_aGridBefore.push_back(nTmpGridBefore);
     }
 
     // Push the tmp position now that we compared it
@@ -589,10 +599,16 @@ void DomainMapperTableManager::endOfRowAction()
     }
 
     IntVectorPtr pCurrentSpans = getCurrentSpans( );
-    if( pCurrentSpans->size() < m_nCell.back( ) )
+
+    if( m_aGridBefore.back() > 0 )
+    {
+        //fill missing gridBefore elements with '1'
+        pCurrentSpans->insert( pCurrentSpans->begin( ), m_aGridBefore.back(), 1 );
+    }
+    if( pCurrentSpans->size() < m_aGridBefore.back() + m_nCell.back( ))
     {
         //fill missing elements with '1'
-        pCurrentSpans->insert( pCurrentSpans->end( ), m_nCell.back( ) - pCurrentSpans->size(), 1 );
+        pCurrentSpans->insert( pCurrentSpans->end( ), m_aGridBefore.back() + m_nCell.back( ) - pCurrentSpans->size(), 1 );
     }
 
 #ifdef DEBUG_WRITERFILTER
@@ -619,7 +635,7 @@ void DomainMapperTableManager::endOfRowAction()
     for (int i : (*pTableGrid.get()))
         nFullWidthRelative = o3tl::saturating_add(nFullWidthRelative, i);
 
-    if( pTableGrid->size() == ( m_nGridBefore + nGrids + m_nGridAfter ) && m_nCell.back( ) > 0 )
+    if( pTableGrid->size() == ( nGrids + m_nGridAfter ) && m_nCell.back( ) > 0 )
     {
         /*
          * If table width property set earlier is smaller than the current table width,
@@ -641,12 +657,12 @@ void DomainMapperTableManager::endOfRowAction()
                 pTablePropMap->setValue(TablePropertyMap::TABLE_WIDTH_TYPE, text::SizeType::FIX);
             }
         }
-        uno::Sequence< text::TableColumnSeparator > aSeparators( m_nCell.back( ) - 1 );
+        uno::Sequence< text::TableColumnSeparator > aSeparators( m_aGridBefore.back() + m_nCell.back( ) - 1 );
         text::TableColumnSeparator* pSeparators = aSeparators.getArray();
         sal_Int16 nLastRelPos = 0;
-        sal_uInt32 nBorderGridIndex = m_nGridBefore;
+        sal_uInt32 nBorderGridIndex = 0;
 
-        size_t nWidthsBound =  m_nCell.back( ) - 1;
+        size_t nWidthsBound = m_aGridBefore.back() + m_nCell.back() - 1;
         if (nWidthsBound)
         {
             if (nFullWidthRelative == 0)
@@ -680,7 +696,7 @@ void DomainMapperTableManager::endOfRowAction()
     }
     else if ( !pCellWidths->empty() &&
                ( m_nLayoutType == NS_ooxml::LN_Value_doc_ST_TblLayout_fixed
-                 || pCellWidths->size() == ( m_nGridBefore + nGrids + m_nGridAfter ) )
+                 || pCellWidths->size() == ( nGrids + m_nGridAfter ) )
              )
     {
         // If we're here, then the number of cells does not equal to the amount
@@ -734,11 +750,12 @@ void DomainMapperTableManager::endOfRowAction()
 
     ++m_nRow;
     m_nCell.back( ) = 0;
+    m_aGridBefore.back( ) = 0;
     getCurrentGrid()->clear();
     pCurrentSpans->clear();
     pCellWidths->clear();
 
-    m_nGridBefore = m_nGridAfter = 0;
+    m_nGridAfter = 0;
     m_bRowSizeTypeInserted = false;
     m_bHasBtlrCell = false;
     m_bTableSizeTypeInserted = false;
diff --git a/writerfilter/source/dmapper/DomainMapperTableManager.hxx b/writerfilter/source/dmapper/DomainMapperTableManager.hxx
index cf130a021462..1d2f54bf474c 100644
--- a/writerfilter/source/dmapper/DomainMapperTableManager.hxx
+++ b/writerfilter/source/dmapper/DomainMapperTableManager.hxx
@@ -42,7 +42,7 @@ class DomainMapperTableManager : public TableManager
     sal_uInt32      m_nRow;
     ::std::vector< sal_uInt32 > m_nCell;
     sal_uInt32      m_nGridSpan;
-    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
+    ::std::vector< sal_uInt32 > m_aGridBefore; ///< 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
     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
@@ -97,6 +97,7 @@ public:
     bool hasCurrentSpans() const;
     IntVectorPtr const & getCurrentSpans( );
     IntVectorPtr const & getCurrentCellWidths( );
+    sal_uInt32 getCurrentGridBefore( );
 
     /// Turn the attributes collected so far in m_aTableLook into a property and clear the container.
     void finishTableLook();
diff --git a/writerfilter/source/dmapper/TableData.hxx b/writerfilter/source/dmapper/TableData.hxx
index 8381a9899125..423e837cc336 100644
--- a/writerfilter/source/dmapper/TableData.hxx
+++ b/writerfilter/source/dmapper/TableData.hxx
@@ -131,11 +131,18 @@ public:
        @param start     the start handle of the cell
        @param end       the end handle of the cell
        @param pProps    the properties of the cell
+       @param bAddBefore true: add an empty cell at beginning of the row for gridBefore
      */
-    void addCell(const css::uno::Reference<css::text::XTextRange>& start, TablePropertyMapPtr pProps)
+    void addCell(const css::uno::Reference<css::text::XTextRange>& start, TablePropertyMapPtr pProps, bool bAddBefore = false)
     {
         CellData::Pointer_t pCellData(new CellData(start, pProps));
-        mCells.push_back(pCellData);
+        if (bAddBefore)
+        {
+            mCells.insert(mCells.begin(), pCellData);
+            mCells[0]->setEnd(start);
+        }
+        else
+            mCells.push_back(pCellData);
     }
 
     void endCell(const css::uno::Reference<css::text::XTextRange>& end)
diff --git a/writerfilter/source/dmapper/TableManager.cxx b/writerfilter/source/dmapper/TableManager.cxx
index 5ff7f26af824..76dae9aff965 100644
--- a/writerfilter/source/dmapper/TableManager.cxx
+++ b/writerfilter/source/dmapper/TableManager.cxx
@@ -407,6 +407,26 @@ void TableManager::endRow()
 #ifdef DEBUG_WRITERFILTER
     TagLogger::getInstance().element("tablemanager.endRow");
 #endif
+    TableData::Pointer_t pTableData = mTableDataStack.top();
+
+    // Add borderless w:gridBefore cell(s) to the row
+    if (pTableData)
+    {
+        sal_uInt32 nGridBefore = mpTableDataHandler->getDomainMapperImpl().getTableManager().getCurrentGridBefore();
+        for (unsigned int i = 0; i < nGridBefore; ++i)
+        {
+            css::table::BorderLine2 aBorderLine;
+            aBorderLine.Color = 0;
+            aBorderLine.InnerLineWidth = 0;
+            aBorderLine.OuterLineWidth = 0;
+            TablePropertyMapPtr pCellProperties(new TablePropertyMap);
+            pCellProperties->Insert(PROP_TOP_BORDER, css::uno::makeAny(aBorderLine));
+            pCellProperties->Insert(PROP_LEFT_BORDER, css::uno::makeAny(aBorderLine));
+            pCellProperties->Insert(PROP_BOTTOM_BORDER, css::uno::makeAny(aBorderLine));
+            pCellProperties->Insert(PROP_RIGHT_BORDER, css::uno::makeAny(aBorderLine));
+            pTableData->getCurrentRow()->addCell(pTableData->getCurrentRow()->getCellStart(0), pCellProperties, /*bAddBefore=*/true);
+        }
+    }
 
     setRowEnd(true);
 }
diff --git a/writerfilter/source/ooxml/OOXMLFastContextHandler.cxx b/writerfilter/source/ooxml/OOXMLFastContextHandler.cxx
index 906320088987..785022139ed1 100644
--- a/writerfilter/source/ooxml/OOXMLFastContextHandler.cxx
+++ b/writerfilter/source/ooxml/OOXMLFastContextHandler.cxx
@@ -1466,6 +1466,10 @@ OOXMLValue::Pointer_t fakeNoBorder()
 // not insane enough to find out how to add cells in dmapper.
 void OOXMLFastContextHandlerTextTableRow::handleGridBefore( const OOXMLValue::Pointer_t& val )
 {
+    // start removing: disable for w:gridBefore
+    if (!mpGridAfter)
+        return;
+
     int count = val->getInt();
     for( int i = 0;
          i < count;
diff --git a/writerfilter/source/ooxml/model.xml b/writerfilter/source/ooxml/model.xml
index cdc0f040fb34..2d4516fd732b 100644
--- a/writerfilter/source/ooxml/model.xml
+++ b/writerfilter/source/ooxml/model.xml
@@ -14442,7 +14442,7 @@
             <ref name="CT_DecimalNumber"/>
           </element>
           <element name="gridBefore">
-            <ref name="CT_TrPrBaseGridBefore"/>
+            <ref name="CT_DecimalNumber"/>
           </element>
           <element name="gridAfter">
             <ref name="CT_TrPrBaseGridAfter"/>
@@ -18409,6 +18409,7 @@
     <resource name="CT_TrPrBase" resource="Properties">
       <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="wBefore" tokenid="ooxml:CT_TrPrBase_wBefore"/>
       <element name="wAfter" tokenid="ooxml:CT_TrPrBase_wAfter"/>
       <element name="cantSplit" tokenid="ooxml:CT_TrPrBase_cantSplit"/>
@@ -18419,7 +18420,7 @@
       <element name="hidden" tokenid="ooxml:CT_TrPrBase_hidden"/>
     </resource>
     <resource name="CT_TrPrBaseGridBefore" resource="TextTableRow">
-      <attribute name="val" tokenid="ooxml:CT_TrPrBase_gridBefore" action="handleGridBefore"/>
+      <attribute name="val" tokenid="ooxml:CT_TrPrBase_gridBefore"/>
     </resource>
     <resource name="CT_TrPrBaseGridAfter" resource="TextTableRow">
       <attribute name="val" tokenid="ooxml:CT_TrPrBase_gridAfter" action="handleGridAfter"/>


More information about the Libreoffice-commits mailing list