From da1f71edfc72928b07a569b98e2766a8a7de9d2a Mon Sep 17 00:00:00 2001 From: László Németh Date: Mon, 2 Dec 2019 19:02:43 +0100 Subject: tdf#116194 DOCX import: fix missing tables with w:gridBefore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Change-Id: Id25f5fb4d9021c87ee8c82782b2038e6fb255673 Reviewed-on: https://gerrit.libreoffice.org/84263 Reviewed-by: László Németh Tested-by: László Németh --- sw/qa/extras/ooxmlexport/data/tdf116194.docx | Bin 0 -> 17227 bytes sw/qa/extras/ooxmlexport/ooxmlexport10.cxx | 9 +++++ .../source/dmapper/DomainMapperTableManager.cxx | 37 +++++++++++++++------ .../source/dmapper/DomainMapperTableManager.hxx | 3 +- writerfilter/source/dmapper/TableData.hxx | 11 ++++-- writerfilter/source/dmapper/TableManager.cxx | 20 +++++++++++ .../source/ooxml/OOXMLFastContextHandler.cxx | 4 +++ writerfilter/source/ooxml/model.xml | 5 +-- 8 files changed, 74 insertions(+), 15 deletions(-) create mode 100644 sw/qa/extras/ooxmlexport/data/tdf116194.docx 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 b62d49a72730..6aeec25e80b6 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx @@ -595,6 +595,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 textDocument(mxComponent, uno::UNO_QUERY); diff --git a/writerfilter/source/dmapper/DomainMapperTableManager.cxx b/writerfilter/source/dmapper/DomainMapperTableManager.cxx index e25f55e68bb7..03486feba082 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), @@ -354,7 +354,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; @@ -394,6 +394,11 @@ DomainMapperTableManager::IntVectorPtr const & DomainMapperTableManager::getCurr return m_aTableGrid.back( ); } +sal_uInt32 DomainMapperTableManager::getCurrentGridBefore( ) +{ + return m_aGridBefore.back( ); +} + bool DomainMapperTableManager::hasCurrentSpans() const { return !m_aGridSpans.empty(); @@ -456,6 +461,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; @@ -485,6 +491,7 @@ void DomainMapperTableManager::endLevel( ) m_aCellWidths.back()->push_back(*oCurrentWidth); m_nCell.pop_back( ); + m_aGridBefore.pop_back( ); m_nTableWidth = 0; m_nLayoutType = 0; @@ -540,6 +547,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 @@ -552,10 +560,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 @@ -596,10 +606,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 DBG_UTIL @@ -626,7 +642,7 @@ void DomainMapperTableManager::endOfRowAction() for (int i : (*pTableGrid)) 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, @@ -656,12 +672,12 @@ void DomainMapperTableManager::endOfRowAction() } } } - 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(); double nLastRelPos = 0.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) @@ -694,7 +710,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 @@ -748,11 +764,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_bTableSizeTypeInserted = false; #ifdef DBG_UTIL diff --git a/writerfilter/source/dmapper/DomainMapperTableManager.hxx b/writerfilter/source/dmapper/DomainMapperTableManager.hxx index 243e901169bb..219986870ef3 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 @@ -93,6 +93,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 8489254f4ab8..6326dbe33704 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& start, TablePropertyMapPtr pProps) + void addCell(const css::uno::Reference& 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& end) diff --git a/writerfilter/source/dmapper/TableManager.cxx b/writerfilter/source/dmapper/TableManager.cxx index 03fab0295353..b9384b9e08df 100644 --- a/writerfilter/source/dmapper/TableManager.cxx +++ b/writerfilter/source/dmapper/TableManager.cxx @@ -407,6 +407,26 @@ void TableManager::endRow() #ifdef DBG_UTIL 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 11731557a44e..6f7839d0512b 100644 --- a/writerfilter/source/ooxml/OOXMLFastContextHandler.cxx +++ b/writerfilter/source/ooxml/OOXMLFastContextHandler.cxx @@ -1461,6 +1461,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 66eedcaa63e4..17c8f5217fec 100644 --- a/writerfilter/source/ooxml/model.xml +++ b/writerfilter/source/ooxml/model.xml @@ -14442,7 +14442,7 @@ - + @@ -18404,6 +18404,7 @@ + @@ -18414,7 +18415,7 @@ - + -- cgit v1.2.3