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

9 files changed, 118 insertions, 9 deletions

diff --git a/sw/qa/extras/ooxmlimport/data/n816593.docx b/sw/qa/extras/ooxmlimport/data/n816593.docx
new file mode 100644
index 000000000000..7d784e69d4e6
--- /dev/null
+++ b/sw/qa/extras/ooxmlimport/data/n816593.docx
diff --git a/sw/qa/extras/ooxmlimport/ooxmlimport.cxx b/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
index 5a268e48597e..c422ecc829ca 100644
--- a/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
+++ b/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();
diff --git a/writerfilter/inc/resourcemodel/TableData.hxx b/writerfilter/inc/resourcemodel/TableData.hxx
index 61efa41b9c25..7c9f9dc37444 100644
--- a/writerfilter/inc/resourcemodel/TableData.hxx
+++ b/writerfilter/inc/resourcemodel/TableData.hxx
@@ -409,6 +409,11 @@ public:
     {
         return mRows[i];
     }
+
+    const RowPointer_t getCurrentRow() const
+    {
+        return mpRow;
+    }
 };
 
 }
diff --git a/writerfilter/inc/resourcemodel/TableManager.hxx b/writerfilter/inc/resourcemodel/TableManager.hxx
index a8ee39be69a1..72841ff25ee5 100644
--- a/writerfilter/inc/resourcemodel/TableManager.hxx
+++ b/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);
diff --git a/writerfilter/source/dmapper/DomainMapperTableManager.cxx b/writerfilter/source/dmapper/DomainMapperTableManager.cxx
index f0761b5ae1d1..48d1e3132690 100644
--- a/writerfilter/source/dmapper/DomainMapperTableManager.cxx
+++ b/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();
 
diff --git a/writerfilter/source/dmapper/DomainMapperTableManager.hxx b/writerfilter/source/dmapper/DomainMapperTableManager.hxx
index d529d5531404..1384197a585f 100644
--- a/writerfilter/source/dmapper/DomainMapperTableManager.hxx
+++ b/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
diff --git a/writerfilter/source/dmapper/TablePositionHandler.cxx b/writerfilter/source/dmapper/TablePositionHandler.cxx
index 46324a175eb3..3e309192371c 100644
--- a/writerfilter/source/dmapper/TablePositionHandler.cxx
+++ b/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
 
diff --git a/writerfilter/source/dmapper/TablePositionHandler.hxx b/writerfilter/source/dmapper/TablePositionHandler.hxx
index dc096e687d26..b6ddd5006d21 100644
--- a/writerfilter/source/dmapper/TablePositionHandler.hxx
+++ b/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;
diff --git a/writerfilter/source/ooxml/OOXMLParserState.cxx b/writerfilter/source/ooxml/OOXMLParserState.cxx
index c2dc79c3e847..d8b562bf0821 100644
--- a/writerfilter/source/ooxml/OOXMLParserState.cxx
+++ b/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.
         }
     }
 }