n#816593: docx consecutive tables with different tblpPr needs to be split

When importing docx with 2 <w:tbl> following each other, we have 2
possible behaviors: either merge them as one table as we did before or
split them into two tables. The tables need to be split if they have
different floating position properties.

This required the ooxml tokenizer to repeat the table properties for
each row of the table: or how would we know we don't need to split the
table?

The basic idea behind this hack is to temporarily store the table
position and table properties before saving them. Thus we can compare
them at the end of the row and decide to split the table or not.

Change-Id: I2e3e70dfe7386090fe356575ee9d0e81aa031dc4

sw/qa/extras/ooxmlimport/ooxmlimport.cxx

@@ -116,6 +116,7 @@ public:
     void testPageBackground();
     void testWatermark();
     void testPageBorderShadow();
+    void testN816593();
 
     CPPUNIT_TEST_SUITE(Test);
 #if !defined(MACOSX) && !defined(WNT)
@@ -200,6 +201,7 @@ void Test::run()
         {"page-background.docx", &Test::testPageBackground},
         {"watermark.docx", &Test::testWatermark},
         {"page-border-shadow.docx", &Test::testPageBorderShadow},
+        {"n816593.docx", &Test::testN816593},
     };
     header();
     for (unsigned int i = 0; i < SAL_N_ELEMENTS(aMethods); ++i)
@@ -1430,6 +1432,15 @@ void Test::testPageBorderShadow()
     CPPUNIT_ASSERT_EQUAL(sal_Int16(TWIP_TO_MM100(48/8*20)), aShadow.ShadowWidth);
 }
 
+void Test::testN816593()
+{
+    // Two consecutive <w:tbl> without any paragraph in between, but with different tblpPr. In this
+    // case we need to have 2 different tables instead of 1
+    uno::Reference<text::XTextTablesSupplier> xTablesSupplier(mxComponent, uno::UNO_QUERY);
+    uno::Reference<container::XIndexAccess> xTables(xTablesSupplier->getTextTables( ), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(2), xTables->getCount());
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(Test);
 
 CPPUNIT_PLUGIN_IMPLEMENT();

writerfilter/inc/resourcemodel/TableData.hxx

@@ -409,6 +409,11 @@ public:
     {
         return mRows[i];
     }
+
+    const RowPointer_t getCurrentRow() const
+    {
+        return mpRow;
+    }
 };
 
 }

writerfilter/inc/resourcemodel/TableManager.hxx

@@ -408,6 +408,8 @@ private:
        for each level of nested tables there is one frame in the stack
      */
     stack<typename TableData<T, PropertiesPointer>::Pointer_t > mTableDataStack;
+    typename RowData<T, PropertiesPointer>::Pointer_t mpUnfinishedRow;
+    bool mbKeepUnfinishedRow;
 
     typedef typename TableDataHandler<T, PropertiesPointer>::Pointer_t TableDataHandlerPointer_t;
 
@@ -486,6 +488,14 @@ protected:
      */
     virtual void clearData();
 
+    /** Should we keep the unfinished row in endLevel to initialize the table
+        data in the following startLevel.
+      */
+    void setKeepUnfinishedRow(bool bKeep)
+    {
+        mbKeepUnfinishedRow = bKeep;
+    }
+
 
 public:
     TableManager();
@@ -618,7 +628,7 @@ public:
 
 template <typename T, typename PropertiesPointer>
 TableManager<T, PropertiesPointer>::TableManager()
-: mnTableDepthNew(0), mnTableDepth(0)
+: mnTableDepthNew(0), mnTableDepth(0), mbKeepUnfinishedRow( false )
 {
     setRowEnd(false);
     setInCell(false);
@@ -731,6 +741,18 @@ void TableManager<T, PropertiesPointer>::startLevel()
     typename TableData<T, PropertiesPointer>::Pointer_t pTableData
         (new TableData<T, PropertiesPointer>(mTableDataStack.size()));
 
+    // If we have an unfinished row stored here, then push it to the new TableData
+    if ( mpUnfinishedRow )
+    {
+        for (unsigned int i = 0; i < mpUnfinishedRow->getCellCount(); ++i)
+        {
+            pTableData->addCell( mpUnfinishedRow->getCellStart(i),
+                                 mpUnfinishedRow->getCellProperties(i) );
+            pTableData->endCell( mpUnfinishedRow->getCellEnd(i) );
+        }
+        mpUnfinishedRow.reset();
+    }
+
     mTableDataStack.push(pTableData);
     mState.startLevel();
 }
@@ -741,6 +763,9 @@ void TableManager<T, PropertiesPointer>::endLevel()
     if (mpTableDataHandler.get() != NULL)
         resolveCurrentTable();
 
+    // Store the unfinished row as it will be used for the next table
+    if ( mbKeepUnfinishedRow )
+        mpUnfinishedRow = mTableDataStack.top()->getCurrentRow();
     mState.endLevel();
     mTableDataStack.pop();
 
@@ -802,7 +827,7 @@ void TableManager<T, PropertiesPointer>::endParagraphGroup()
         if (isRowEnd())
         {
             endOfRowAction();
-            pTableData->endRow(getRowProps());
+            mTableDataStack.top()->endRow(getRowProps());
             resetRowProps();
         }
 
@@ -963,7 +988,7 @@ void TableManager<T, PropertiesPointer>::insertTableProps(PropertiesPointer pPro
         mpTableLogger->startElement("tablemanager.insertTableProps");
 #endif
 
-    if( getTableProps().get() )
+    if( getTableProps().get() && getTableProps() != pProps)
         getTableProps()->InsertProps(pProps);
     else
         setTableProps(pProps);

writerfilter/source/dmapper/DomainMapperTableManager.cxx

@@ -51,6 +51,8 @@ DomainMapperTableManager::DomainMapperTableManager(bool bOOXML) :
     m_nHeaderRepeat(0),
     m_nTableWidth(0),
     m_bOOXML( bOOXML ),
+    m_aTmpPosition(),
+    m_aTmpTableProperties(),
     m_bPushCurrentWidth(false),
     m_bRowSizeTypeInserted(false),
     m_bTableSizeTypeInserted(false),
@@ -344,14 +346,14 @@ bool DomainMapperTableManager::sprm(Sprm & rSprm)
                     writerfilter::Reference<Properties>::Pointer_t pProperties = rSprm.getProps();
                     if (pProperties.get())
                     {
-                        TablePositionHandlerPtr pHandler = m_aTablePositions.back();
+                        TablePositionHandlerPtr pHandler = m_aTmpPosition.back();
                         if ( !pHandler )
                         {
-                            m_aTablePositions.pop_back();
+                            m_aTmpPosition.pop_back();
                             pHandler.reset( new TablePositionHandler );
-                            m_aTablePositions.push_back( pHandler );
+                            m_aTmpPosition.push_back( pHandler );
                         }
-                        pProperties->resolve(*pHandler);
+                        pProperties->resolve(*m_aTmpPosition.back());
                     }
                 }
                 break;
@@ -415,6 +417,11 @@ void DomainMapperTableManager::startLevel( )
     m_aGridSpans.push_back( pNewSpans );
     m_aCellWidths.push_back( pNewCellWidths );
     m_aTablePositions.push_back( pNewPositionHandler );
+
+    TablePositionHandlerPtr pTmpPosition;
+    TablePropertyMapPtr pTmpProperties( new TablePropertyMap( ) );
+    m_aTmpPosition.push_back( pTmpPosition );
+    m_aTmpTableProperties.push_back( pTmpProperties );
     m_nCell.push_back( 0 );
     m_nTableWidth = 0;
     m_nLayoutType = 0;
@@ -433,6 +440,8 @@ void DomainMapperTableManager::endLevel( )
     m_nTableWidth = 0;
     m_nLayoutType = 0;
 
+    m_aTmpPosition.pop_back( );
+    m_aTmpTableProperties.pop_back( );
 
     DomainMapperTableManager_Base_t::endLevel( );
 #ifdef DEBUG_DOMAINMAPPER
@@ -469,6 +478,42 @@ void DomainMapperTableManager::endOfRowAction()
     dmapper_logger->startElement("endOfRowAction");
 #endif
 
+    // Compare the table position with the previous ones. We may need to split
+    // into two tables if those are different. We surely don't want to do anything
+    // if we don't have any row yet.
+    TablePositionHandlerPtr pTmpPosition = m_aTmpPosition.back();
+    TablePropertyMapPtr pTmpTableProperties = m_aTmpTableProperties.back( );
+    TablePositionHandlerPtr pCurrentPosition = m_aTablePositions.back();
+    bool bSamePosition = ( pTmpPosition == pCurrentPosition ) ||
+                         ( pTmpPosition && pCurrentPosition && *pTmpPosition == *pCurrentPosition );
+    if ( !bSamePosition && m_nRow > 0 )
+    {
+        // Save the grid infos to have them survive the end/start level
+        IntVectorPtr pTmpTableGrid = m_aTableGrid.back();
+        IntVectorPtr pTmpGridSpans = m_aGridSpans.back();
+        IntVectorPtr pTmpCellWidths = m_aCellWidths.back();
+
+        // endLevel and startLevel are taking care of the non finished row
+        // to carry it over to the next table
+        setKeepUnfinishedRow( true );
+        endLevel();
+        setKeepUnfinishedRow( false );
+        startLevel();
+
+        m_aTableGrid.pop_back();
+        m_aGridSpans.pop_back();
+        m_aCellWidths.pop_back();
+        m_aTableGrid.push_back(pTmpTableGrid);
+        m_aGridSpans.push_back(pTmpGridSpans);
+        m_aCellWidths.push_back(pTmpCellWidths);
+    }
+
+    // Push the tmp position now that we compared it
+    m_aTablePositions.pop_back();
+    m_aTablePositions.push_back( pTmpPosition );
+    m_aTmpPosition.back().reset( );
+
+
     IntVectorPtr pTableGrid = getCurrentGrid( );
     IntVectorPtr pCellWidths = getCurrentCellWidths( );
     if(!m_nTableWidth && pTableGrid->size())
@@ -607,9 +652,17 @@ void DomainMapperTableManager::endOfRowAction()
         insertRowProps(pPropMap);
     }
 
+    // Now that potentially opened table is closed, save the table properties
+    DomainMapperTableManager_Base_t::insertTableProps( pTmpTableProperties );
+
+    m_aTmpTableProperties.pop_back();
+    TablePropertyMapPtr pEmptyTableProps( new TablePropertyMap() );
+    m_aTmpTableProperties.push_back( pEmptyTableProps );
+
     ++m_nRow;
     m_nCell.back( ) = 0;
     m_nCellBorderIndex = 0;
+    getCurrentGrid()->clear();
     pCurrentSpans->clear();
     pCellWidths->clear();
 

writerfilter/source/dmapper/DomainMapperTableManager.hxx

@@ -46,6 +46,8 @@ class DomainMapperTableManager : public DomainMapperTableManager_Base_t
     bool            m_bOOXML;
     OUString m_sTableStyleName;
     std::vector< TablePositionHandlerPtr > m_aTablePositions;
+    std::vector< TablePositionHandlerPtr > m_aTmpPosition; ///< Temporarily stores the position to compare it later
+    std::vector< TablePropertyMapPtr > m_aTmpTableProperties; ///< Temporarily stores the table properties until end of row
     PropertyMapPtr  m_pTableStyleTextProperies;
 
     ::std::vector< IntVectorPtr >  m_aTableGrid;
@@ -122,7 +124,7 @@ public:
         if ( m_pStyleProps.get( ) )
             m_pStyleProps->InsertProps(pProps);
         else
-           DomainMapperTableManager_Base_t::insertTableProps( pProps );
+            m_aTmpTableProperties.back()->InsertProps(pProps);
     };
 
     bool IsRowSizeTypeInserted() const

writerfilter/source/dmapper/TablePositionHandler.cxx

@@ -164,6 +164,16 @@ uno::Sequence<beans::PropertyValue> TablePositionHandler::getTablePosition() con
     return aFrameProperties;
 }
 
+bool TablePositionHandler::operator== (const TablePositionHandler& rHandler) const
+{
+    return m_aVertAnchor == rHandler.m_aVertAnchor &&
+           m_aYSpec == rHandler.m_aYSpec &&
+           m_aHorzAnchor == rHandler.m_aHorzAnchor &&
+           m_aXSpec == rHandler.m_aXSpec &&
+           m_nY == rHandler.m_nY &&
+           m_nX == rHandler.m_nX;
+}
+
 } // namespace dmapper
 } // namespace writerfilter
 

writerfilter/source/dmapper/TablePositionHandler.hxx

@@ -43,6 +43,8 @@ namespace writerfilter {
                 properties before actually using them.
               */
             com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> getTablePosition() const;
+
+            bool operator== (const TablePositionHandler& rHandler) const;
         };
 
         typedef boost::shared_ptr<TablePositionHandler> TablePositionHandlerPtr;

writerfilter/source/ooxml/OOXMLParserState.cxx

@@ -229,7 +229,8 @@ void OOXMLParserState::resolveTableProperties(Stream & rStream)
         if (rTableProps.get() != NULL)
         {
             rStream.props(rTableProps);
-            rTableProps.reset(new OOXMLPropertySetImpl());
+            // Don't clean the table props to send them again for each row
+            // This mimics the behaviour from RTF tokenizer.
         }
     }
 }