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.

Conflicts:

	sw/qa/extras/ooxmlimport/ooxmlimport.cxx

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 2b87f4d8a81c..0518809fb7fc 100644
--- a/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
+++ b/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
@@ -124,6 +124,7 @@ public:
     void testPageBackground();
     void testWatermark();
     void testPageBorderShadow();
+    void testN816593();
     void testN820509();
     void testN820788();
     void testTableAutoColumnFixedSize();
@@ -201,6 +202,7 @@ void Test::run()
         {"page-background.docx", &Test::testPageBackground},
         {"watermark.docx", &Test::testWatermark},
         {"page-border-shadow.docx", &Test::testPageBorderShadow},
+        {"n816593.docx", &Test::testN816593},
         {"n820509.docx", &Test::testN820509},
         {"n820788.docx", &Test::testN820788},
         {"table-auto-column-fixed-size.docx", &Test::testTableAutoColumnFixedSize},
@@ -1237,6 +1239,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());
+}
+
 void Test::testN820509()
 {
     // Design mode was enabled.
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 59ce1eac2d84..6362969ecdb7 100644
--- a/writerfilter/source/dmapper/DomainMapperTableManager.cxx
+++ b/writerfilter/source/dmapper/DomainMapperTableManager.cxx
@@ -50,6 +50,8 @@ DomainMapperTableManager::DomainMapperTableManager(bool bOOXML) :
     m_nHeaderRepeat(0),
     m_nTableWidth(0),
     m_bOOXML( bOOXML ),
+    m_aTmpPosition(),
+    m_aTmpTableProperties(),
     m_bPushCurrentWidth(false),
     m_nLayoutType(0),
     m_nMaxFixedWidth(0),
@@ -386,14 +388,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;
@@ -457,6 +459,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;
@@ -475,6 +482,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
@@ -511,6 +520,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())
@@ -645,9 +690,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 c3ac115ded3f..fc4f7aa201eb 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;
@@ -119,7 +121,7 @@ public:
         if ( m_pStyleProps.get( ) )
             m_pStyleProps->InsertProps(pProps);
         else
-           DomainMapperTableManager_Base_t::insertTableProps( pProps );
+            m_aTmpTableProperties.back()->InsertProps(pProps);
     };
 
     void SetLayoutType(sal_uInt32 nLayoutType)
diff --git a/writerfilter/source/dmapper/TablePositionHandler.cxx b/writerfilter/source/dmapper/TablePositionHandler.cxx
index 8ae6ac20e6a4..7ed8bde1d230 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 723aef804b35..36be1d1e6059 100644
--- a/writerfilter/source/ooxml/OOXMLParserState.cxx
+++ b/writerfilter/source/ooxml/OOXMLParserState.cxx
@@ -232,7 +232,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.
         }
     }
 }