tdf#104354 DOCX import: fix paragraph auto spacing in tables

Top margin of first paragraph of a table cell with auto spacing, and
bottom margin of last paragraph of a table cell with auto spacing are
zero (except in numbered last paragraphs), but LibreOffice set 14pt
instead of them, resulting much longer tables. Following cases needed
special handling:

- auto spacing in style
- direct top and bottom auto spacing
- direct top and bottom margins
- footnotes in cell paragraphs

Change-Id: I462847616dd43b4ba30fae2c4eb99abb49bfb9a3
Reviewed-on: https://gerrit.libreoffice.org/57352
Tested-by: Jenkins
Reviewed-by: László Németh <nemeth@numbertext.org>

5 files changed, 126 insertions, 4 deletions

diff --git a/sw/qa/extras/ooxmlexport/data/tdf104354-2.docx b/sw/qa/extras/ooxmlexport/data/tdf104354-2.docx
new file mode 100644
index 000000000000..9f40bf77976e
--- /dev/null
+++ b/sw/qa/extras/ooxmlexport/data/tdf104354-2.docx
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx
index f1a260b87057..1d40cfef8268 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx
@@ -1170,13 +1170,52 @@ DECLARE_OOXMLEXPORT_TEST(testTdf90789, "tdf90789.docx")
     CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int16>(1), xPageCursor->getPage());
 }
 
-
 DECLARE_OOXMLEXPORT_TEST(testTdf90789_2, "tdf90789-2.docx")
 {
     // Section break before frame and shape was ignored
     CPPUNIT_ASSERT_EQUAL( 3, getPages() );
 }
 
+DECLARE_OOXMLEXPORT_TEST(testTdf104354_2, "tdf104354-2.docx")
+{
+    uno::Reference<text::XTextTablesSupplier> xTablesSupplier(mxComponent, uno::UNO_QUERY);
+    uno::Reference<container::XIndexAccess> xTables(xTablesSupplier->getTextTables(), uno::UNO_QUERY);
+    uno::Reference<text::XTextTable> xTable(xTables->getByIndex(0), uno::UNO_QUERY);
+    uno::Reference<text::XTextRange> xCell(xTable->getCellByName("A1"), uno::UNO_QUERY);
+
+    // top margin of the first paragraph and bottom margin of the last paragraph
+    // is zero, when auto spacing is used.
+
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0), getProperty<sal_Int32>(getParagraphOfText(1, xCell->getText()), "ParaTopMargin"));
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(494), getProperty<sal_Int32>(getParagraphOfText(1, xCell->getText()), "ParaBottomMargin"));
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(494), getProperty<sal_Int32>(getParagraphOfText(2, xCell->getText()), "ParaTopMargin"));
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(494), getProperty<sal_Int32>(getParagraphOfText(2, xCell->getText()), "ParaBottomMargin"));
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(494), getProperty<sal_Int32>(getParagraphOfText(3, xCell->getText()), "ParaTopMargin"));
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0), getProperty<sal_Int32>(getParagraphOfText(3, xCell->getText()), "ParaBottomMargin"));
+
+    // top margin is not auto spacing
+    uno::Reference<text::XTextRange> xCell2(xTable->getCellByName("A2"), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(847), getProperty<sal_Int32>(getParagraphOfText(1, xCell2->getText()), "ParaTopMargin"));
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0), getProperty<sal_Int32>(getParagraphOfText(1, xCell2->getText()), "ParaBottomMargin"));
+
+    // bottom margin is not auto spacing
+    uno::Reference<text::XTextRange> xCell3(xTable->getCellByName("A3"), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0), getProperty<sal_Int32>(getParagraphOfText(1, xCell3->getText()), "ParaTopMargin"));
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(847), getProperty<sal_Int32>(getParagraphOfText(1, xCell3->getText()), "ParaBottomMargin"));
+
+    // auto spacing, if the paragraph contains footnotes
+    uno::Reference<text::XTextRange> xCell4(xTable->getCellByName("A4"), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0), getProperty<sal_Int32>(getParagraphOfText(1, xCell4->getText()), "ParaTopMargin"));
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0), getProperty<sal_Int32>(getParagraphOfText(1, xCell4->getText()), "ParaBottomMargin"));
+
+    // auto spacing on a paragraph
+    uno::Reference<text::XTextTable> xTable2(xTables->getByIndex(1), uno::UNO_QUERY);
+    uno::Reference<text::XTextRange> xCell5(xTable2->getCellByName("A1"), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0), getProperty<sal_Int32>(getParagraphOfText(1, xCell5->getText()), "ParaTopMargin"));
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0), getProperty<sal_Int32>(getParagraphOfText(1, xCell5->getText()), "ParaBottomMargin"));
+}
+
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/writerfilter/source/dmapper/DomainMapper.cxx b/writerfilter/source/dmapper/DomainMapper.cxx
index 80bcfa67175d..443be5a54930 100644
--- a/writerfilter/source/dmapper/DomainMapper.cxx
+++ b/writerfilter/source/dmapper/DomainMapper.cxx
@@ -688,6 +688,7 @@ void DomainMapper::lcl_attribute(Id nName, Value & val)
             }
             if  (nIntValue) // If auto spacing is set, then only store set value in InteropGrabBag
             {
+                m_pImpl->SetParaAutoAfter(true);
                 m_pImpl->GetTopContext()->Insert( PROP_PARA_BOTTOM_MARGIN, uno::makeAny( ConversionHelper::convertTwipToMM100(default_spacing) ) );
             }
             else
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index 7a055b629c20..00effcd10081 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -251,7 +251,12 @@ DomainMapper_Impl::DomainMapper_Impl(
         m_bIsSplitPara(false),
         m_vTextFramesForChaining(),
         m_bParaHadField(false),
-        m_bParaAutoBefore(false)
+        m_bParaAutoBefore(false),
+        m_bParaAutoAfter(false),
+        m_bPrevParaAutoAfter(false),
+        m_bParaChangedBottomMargin(false),
+        m_bFirstParagraphInCell(true),
+        m_bSaveFirstParagraphInCell(false)
 {
     m_aBaseUrl = rMediaDesc.getUnpackedValueOrDefault(
         utl::MediaDescriptor::PROP_DOCUMENTBASEURL(), OUString());
@@ -1103,7 +1108,6 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap )
 {
     if (m_bDiscardHeaderFooter)
         return;
-
 #ifdef DEBUG_WRITERFILTER
     TagLogger::getInstance().startElement("finishParagraph");
 #endif
@@ -1114,7 +1118,6 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap )
         return;
     TextAppendContext& rAppendContext = m_aTextAppendStack.top();
     uno::Reference< text::XTextAppend > xTextAppend(rAppendContext.xTextAppend);
-
 #ifdef DEBUG_WRITERFILTER
     TagLogger::getInstance().attribute("isTextAppend", sal_uInt32(xTextAppend.is()));
 #endif
@@ -1383,6 +1386,35 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap )
                     uno::Reference< text::XTextRange > xParaEnd( xCur, uno::UNO_QUERY );
                     CheckParaMarkerRedline( xParaEnd );
                 }
+
+                // set top margin of the previous auto paragraph in cells, keeping zero bottom margin only at the first one
+                if (m_nTableDepth > 0 && m_nTableDepth == m_nTableCellDepth && m_xPreviousParagraph.is())
+                {
+                    bool bParaChangedTopMargin = false;
+                    auto itParaTopMargin = std::find_if(aProperties.begin(), aProperties.end(), [](const beans::PropertyValue& rValue)
+                    {
+                        return rValue.Name == "ParaTopMargin";
+                    });
+                    if (itParaTopMargin != aProperties.end())
+                        bParaChangedTopMargin = true;
+
+                    uno::Sequence<beans::PropertyValue> aPrevPropertiesSeq;
+                    m_xPreviousParagraph->getPropertyValue("ParaInteropGrabBag") >>= aPrevPropertiesSeq;
+                    auto aPrevProperties = comphelper::sequenceToContainer< std::vector<beans::PropertyValue> >(aPrevPropertiesSeq);
+                    auto itPrevParaAutoBefore = std::find_if(aPrevProperties.begin(), aPrevProperties.end(), [](const beans::PropertyValue& rValue)
+                    {
+                        return rValue.Name == "ParaTopMarginBeforeAutoSpacing";
+                    });
+                    bool bPrevParaAutoBefore = itPrevParaAutoBefore != aPrevProperties.end();
+
+                    if ((bPrevParaAutoBefore && !bParaChangedTopMargin) || (bParaChangedTopMargin && m_bParaAutoBefore))
+                    {
+                        sal_Int32 nSize = m_bFirstParagraphInCell ? 0 : 280;
+                        // Previous before spacing is set to auto, set previous before space to 280, except in the first paragraph.
+                        m_xPreviousParagraph->setPropertyValue("ParaTopMargin",
+                                 uno::makeAny( ConversionHelper::convertTwipToMM100(nSize)));
+                    }
+                }
             }
             if( !bKeepLastParagraphProperties )
                 rAppendContext.pLastParagraphProperties = pToBeSavedProperties;
@@ -1395,6 +1427,7 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap )
         {
             SAL_WARN( "writerfilter.dmapper", "finishParagraph() " << e );
         }
+
     }
 
     bool bIgnoreFrameState = IsInHeaderFooter();
@@ -1413,6 +1446,12 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap )
     if (m_bIsFirstParaInShape)
         m_bIsFirstParaInShape = false;
 
+    // keep m_bParaAutoAfter for table paragraphs
+    m_bPrevParaAutoAfter = m_bParaAutoAfter || m_bPrevParaAutoAfter;
+
+    // not auto margin in this paragraph
+    m_bParaChangedBottomMargin = (pParaContext && pParaContext->isSet(PROP_PARA_BOTTOM_MARGIN) && !m_bParaAutoAfter);
+
     if (pParaContext)
     {
         // Reset the frame properties for the next paragraph
@@ -1421,10 +1460,18 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap )
 
     SetIsOutsideAParagraph(true);
     m_bParaHadField = false;
+
+    // don't overwrite m_bFirstParagraphInCell in table separator nodes
+    if (m_nTableDepth > 0 && m_nTableDepth == m_nTableCellDepth)
+        m_bFirstParagraphInCell = false;
+
     m_bParaAutoBefore = false;
+    m_bParaAutoAfter = false;
+
 #ifdef DEBUG_WRITERFILTER
     TagLogger::getInstance().endElement();
 #endif
+
 }
 
 void DomainMapper_Impl::appendTextPortion( const OUString& rString, const PropertyMapPtr& pPropertyMap )
@@ -1818,6 +1865,7 @@ void DomainMapper_Impl::PushFootOrEndnote( bool bIsFootnote )
 {
     m_bInFootOrEndnote = true;
     m_bCheckFirstFootnoteTab = true;
+    m_bSaveFirstParagraphInCell = m_bFirstParagraphInCell;
     try
     {
         // Redlines outside the footnote should not affect footnote content
@@ -1984,6 +2032,7 @@ void DomainMapper_Impl::PopFootOrEndnote()
     m_aRedlines.pop();
     m_bSeenFootOrEndnoteSeparator = false;
     m_bInFootOrEndnote = false;
+    m_bFirstParagraphInCell = m_bSaveFirstParagraphInCell;
 }
 
 void DomainMapper_Impl::SeenFootOrEndnoteSeparator()
@@ -2333,9 +2382,33 @@ bool DomainMapper_Impl::IsDiscardHeaderFooter()
     return m_bDiscardHeaderFooter;
 }
 
+// called from TableManager::closeCell()
 void DomainMapper_Impl::ClearPreviousParagraph()
 {
+    // in table cells, set bottom auto margin of last paragraph to 0, except in paragraphs with numbering
+    if ((m_nTableDepth == (m_nTableCellDepth + 1)) && m_xPreviousParagraph.is() && !m_bParaChangedBottomMargin)
+    {
+        uno::Sequence<beans::PropertyValue> aPrevPropertiesSeq;
+        m_xPreviousParagraph->getPropertyValue("ParaInteropGrabBag") >>= aPrevPropertiesSeq;
+        auto aPrevProperties = comphelper::sequenceToContainer< std::vector<beans::PropertyValue> >(aPrevPropertiesSeq);
+        auto itPrevParaAutoAfter = std::find_if(aPrevProperties.begin(), aPrevProperties.end(), [](const beans::PropertyValue& rValue)
+        {
+            return rValue.Name == "ParaBottomMarginAfterAutoSpacing";
+        });
+        bool bPrevParaAutoAfter = itPrevParaAutoAfter != aPrevProperties.end();
+
+        bool bPrevNumberingRules = false;
+        uno::Reference<container::XNamed> xPreviousNumberingRules(m_xPreviousParagraph->getPropertyValue("NumberingRules"), uno::UNO_QUERY);
+        if (xPreviousNumberingRules.is())
+             bPrevNumberingRules = !xPreviousNumberingRules->getName().isEmpty();
+        if (!bPrevNumberingRules && (bPrevParaAutoAfter || m_bPrevParaAutoAfter))
+            m_xPreviousParagraph->setPropertyValue("ParaBottomMargin", uno::makeAny(static_cast<sal_Int32>(0)));
+    }
+
     m_xPreviousParagraph.clear();
+
+    // next table paragraph will be first paragraph in a cell
+    m_bFirstParagraphInCell = true;
 }
 
 static sal_Int16 lcl_ParseNumberingType( const OUString& rCommand )
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.hxx b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
index 187c6f435d30..555c2dd35bb9 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.hxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
@@ -975,6 +975,7 @@ public:
     bool IsDiscardHeaderFooter();
 
     void SetParaAutoBefore(bool bParaAutoBefore) { m_bParaAutoBefore = bParaAutoBefore; }
+    void SetParaAutoAfter(bool bParaAutoAfter) { m_bParaAutoAfter = bParaAutoAfter; }
 
     /// Forget about the previous paragraph, as it's not inside the same
     /// start/end node.
@@ -988,6 +989,14 @@ private:
     css::uno::Reference<css::beans::XPropertySet> m_xPreviousParagraph;
     /// Current paragraph has automatic before spacing.
     bool m_bParaAutoBefore;
+    /// Current paragraph has automatic after spacing.
+    bool m_bParaAutoAfter;
+    /// Paragraph has direct top or bottom margin formattings
+    bool m_bPrevParaAutoAfter;
+    bool m_bParaChangedBottomMargin;
+    /// Current paragraph in a table is first paragraph of a cell
+    bool m_bFirstParagraphInCell;
+    bool m_bSaveFirstParagraphInCell;
 };
 
 } //namespace dmapper