tdf#119054 DOCX: fix not table style based bottom margin

in table cells, ie. using paragraph styles with bottom
margin setting or direct paragraph formatting of bottom
margin. Both of them overwrite the table style based
bottom margin.

Change-Id: I527b16c24fe47df8412291089ff86fadd3f9430b
Reviewed-on: https://gerrit.libreoffice.org/82800
Reviewed-by: László Németh <nemeth@numbertext.org>
Tested-by: László Németh <nemeth@numbertext.org>
(cherry picked from commit 6100909c84550036932d031f4d2f652e158a1a0a)
Reviewed-on: https://gerrit.libreoffice.org/83154
Tested-by: Jenkins

6 files changed, 71 insertions, 22 deletions

diff --git a/sw/qa/extras/ooxmlexport/data/tdf119054.docx b/sw/qa/extras/ooxmlexport/data/tdf119054.docx
new file mode 100644
index 000000000000..9c3657c24a97
--- /dev/null
+++ b/sw/qa/extras/ooxmlexport/data/tdf119054.docx
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport6.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport6.cxx
index 9e0859f028e7..d460679b50ba 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport6.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport6.cxx
@@ -468,6 +468,18 @@ DECLARE_OOXMLEXPORT_TEST(testTdf128752, "tdf128752.docx")
     assertXPath(pXmlDoc, "/w:document/w:body/w:tbl/w:tr[1]/w:tc[1]/w:p[1]/w:pPr/w:spacing", "after", "0");
 }
 
+DECLARE_OOXMLEXPORT_TEST(testTdf119054, "tdf119054.docx")
+{
+    xmlDocPtr pXmlDoc = parseExport();
+    if (!pXmlDoc)
+        return;
+    // Don't overwrite before and after spacing of Heading2 by table style
+    assertXPathNoAttribute(pXmlDoc, "/w:document/w:body/w:tbl/w:tr[1]/w:tc[1]/w:p[1]/w:pPr/w:spacing", "before");
+    assertXPathNoAttribute(pXmlDoc, "/w:document/w:body/w:tbl/w:tr[1]/w:tc[1]/w:p[1]/w:pPr/w:spacing", "after");
+    // Use table style based single line spacing instead of the docDefaults' 254
+    assertXPath(pXmlDoc, "/w:document/w:body/w:tbl/w:tr[1]/w:tc[1]/w:p[1]/w:pPr/w:spacing", "line", "240");
+}
+
 DECLARE_OOXMLEXPORT_TEST(testFdo69636, "fdo69636.docx")
 {
     /*
diff --git a/writerfilter/source/dmapper/DomainMapperTableHandler.cxx b/writerfilter/source/dmapper/DomainMapperTableHandler.cxx
index 10eb60471126..d6be09fbd4e4 100644
--- a/writerfilter/source/dmapper/DomainMapperTableHandler.cxx
+++ b/writerfilter/source/dmapper/DomainMapperTableHandler.cxx
@@ -809,6 +809,9 @@ CellPropertyValuesSeq_t DomainMapperTableHandler::endTableGetCellProperties(Tabl
                 // Remove properties from style/row that aren't allowed in cells
                 pAllCellProps->Erase( PROP_HEADER_ROW_COUNT );
                 pAllCellProps->Erase( PROP_TBL_HEADER );
+                // Remove paragraph properties from style/row that paragraph style can overwrite
+                pAllCellProps->Erase( PROP_PARA_BOTTOM_MARGIN );
+                pAllCellProps->Erase( PROP_PARA_LINE_SPACING );
 
                 // Then add the cell properties
                 pAllCellProps->InsertProps(*aCellIterator);
@@ -981,6 +984,36 @@ css::uno::Sequence<css::beans::PropertyValues> DomainMapperTableHandler::endTabl
     return aRowProperties;
 }
 
+// table style has got bigger precedence than docDefault style,
+// but lower precedence than the paragraph styles and direct paragraph formatting
+void DomainMapperTableHandler::ApplyParaProperty(css::beans::PropertyValues aTableProperties, PropertyIds eId)
+{
+    OUString sPropertyName = getPropertyName(eId);
+    auto pTableProp = std::find_if(aTableProperties.begin(), aTableProperties.end(),
+        [&](const beans::PropertyValue& rProp) { return rProp.Name == sPropertyName; });
+    if (pTableProp != aTableProperties.end())
+    {
+        uno::Any aValue = pTableProp->Value;
+        for (const auto& rParaProp : m_rDMapper_Impl.m_aParagraphsToEndTable)
+        {
+            // there is no direct paragraph formatting
+            if (!rParaProp.m_pPropertyMap->isSet(eId))
+            {
+                OUString sParaStyleName;
+                rParaProp.m_rPropertySet->getPropertyValue("ParaStyleName") >>= sParaStyleName;
+                StyleSheetEntryPtr pEntry = m_rDMapper_Impl.GetStyleSheetTable()->FindStyleSheetByConvertedStyleName(sParaStyleName);
+                uno::Any aMargin = m_rDMapper_Impl.GetPropertyFromStyleSheet(eId, pEntry, true, true);
+                uno::Any aMarginDocDefault = m_rDMapper_Impl.GetPropertyFromStyleSheet(eId, nullptr, true, true);
+                // use table style only when 1) both values are empty (no docDefault and paragraph style definitions) or
+                // 2) both non-empty values are equal (docDefault paragraph properties are copied to the base paragraph style during import)
+                // TODO check the case, when two parent styles modify the docDefault and the last one set back the docDefault value
+                if (aMargin == aMarginDocDefault)
+                    rParaProp.m_rPropertySet->setPropertyValue(sPropertyName, aValue);
+            }
+        }
+    }
+}
+
 void DomainMapperTableHandler::endTable(unsigned int nestedTableLevel, bool bTableStartsAtCellStart)
 {
 #ifdef DBG_UTIL
@@ -1080,14 +1113,8 @@ void DomainMapperTableHandler::endTable(unsigned int nestedTableLevel, bool bTab
                 }
 
                 // OOXML table style may container paragraph properties, apply these now.
-                auto pTableProp = std::find_if(aTableInfo.aTableProperties.begin(), aTableInfo.aTableProperties.end(),
-                    [](const beans::PropertyValue& rProp) { return rProp.Name == "ParaBottomMargin"; });
-                if (pTableProp != aTableInfo.aTableProperties.end())
-                {
-                    uno::Any aBottomMargin = pTableProp->Value;
-                    for (const auto& rParaProp : m_rDMapper_Impl.m_aPendingParaProp )
-                        rParaProp->setPropertyValue("ParaBottomMargin", aBottomMargin );
-                }
+                ApplyParaProperty(aTableInfo.aTableProperties, PROP_PARA_BOTTOM_MARGIN);
+                ApplyParaProperty(aTableInfo.aTableProperties, PROP_PARA_LINE_SPACING);
             }
         }
         catch ( const lang::IllegalArgumentException & )
@@ -1165,7 +1192,7 @@ void DomainMapperTableHandler::endTable(unsigned int nestedTableLevel, bool bTab
     m_aCellProperties.clear();
     m_aRowProperties.clear();
     m_bHadFootOrEndnote = false;
-    m_rDMapper_Impl.m_aPendingParaProp.clear();
+    m_rDMapper_Impl.m_aParagraphsToEndTable.clear();
 
 #ifdef DBG_UTIL
     TagLogger::getInstance().endElement();
diff --git a/writerfilter/source/dmapper/DomainMapperTableHandler.hxx b/writerfilter/source/dmapper/DomainMapperTableHandler.hxx
index 7a45afa5c0b9..16d2a0cc37cc 100644
--- a/writerfilter/source/dmapper/DomainMapperTableHandler.hxx
+++ b/writerfilter/source/dmapper/DomainMapperTableHandler.hxx
@@ -90,6 +90,9 @@ public:
        @param pProps  properties of the table
      */
     void startTable(const TablePropertyMapPtr& pProps);
+
+    void ApplyParaProperty(css::beans::PropertyValues aTableProperties, PropertyIds eId);
+
     /// Handle end of table.
     void endTable(unsigned int nestedTableLevel, bool bTableStartsAtCellStart);
     /**
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index c2feb59199bc..41612fefe9b8 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -792,9 +792,9 @@ OUString DomainMapper_Impl::GetDefaultParaStyleName()
     return m_sDefaultParaStyleName;
 }
 
-uno::Any DomainMapper_Impl::GetPropertyFromStyleSheet(PropertyIds eId, StyleSheetEntryPtr pEntry, const bool bDocDefaults, const bool bPara, const bool bStyles)
+uno::Any DomainMapper_Impl::GetPropertyFromStyleSheet(PropertyIds eId, StyleSheetEntryPtr pEntry, const bool bDocDefaults, const bool bPara)
 {
-    while( bStyles && pEntry.get( ) )
+    while(pEntry.get( ) )
     {
         if(pEntry->pProperties)
         {
@@ -1702,6 +1702,15 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
                 }
 
                 css::uno::Reference<css::beans::XPropertySet> xParaProps(xTextRange, uno::UNO_QUERY);
+
+                // table style has got bigger precedence than docDefault style
+                // collect these pending paragraph properties to process in endTable()
+                if (xParaProps && m_nTableDepth > 0)
+                {
+                    TableParagraph aPending{pParaContext, xParaProps};
+                    m_aParagraphsToEndTable.push_back(aPending);
+                }
+
                 // tdf#118521 set paragraph top or bottom margin based on the paragraph style
                 // if we already set the other margin with direct formatting
                 if (xParaProps)
@@ -1722,16 +1731,7 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
                         {
                             uno::Any aMargin = GetPropertyFromParaStyleSheet(PROP_PARA_BOTTOM_MARGIN);
                             if ( aMargin != uno::Any() )
-                            {
                                 xParaProps->setPropertyValue("ParaBottomMargin", aMargin);
-
-                                // table style has got bigger precedence than docDefault style
-                                // collect these pending paragraph properties to process in endTable()
-                                // TODO check the case, when two parent styles modify the docDefault and the last one set back the docDefault value
-                                uno::Any aMarginDocDefault = GetPropertyFromStyleSheet(PROP_PARA_BOTTOM_MARGIN, nullptr, true, true, false);
-                                if ( m_nTableDepth > 0 && aMargin == aMarginDocDefault )
-                                    m_aPendingParaProp.push_back(xParaProps);
-                            }
                         }
                         if ( !bContextSet )
                         {
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.hxx b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
index e21ed16a2d52..d191c1abeb3c 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.hxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
@@ -404,6 +404,13 @@ struct SymbolData
     { }
 };
 
+/// Information about a paragraph to be finished after a table end.
+struct TableParagraph
+{
+    PropertyMapPtr m_pPropertyMap;
+    css::uno::Reference<css::beans::XPropertySet> m_rPropertySet;
+};
+
 class DomainMapper;
 class DomainMapper_Impl final
 {
@@ -726,7 +733,7 @@ public:
     OUString  GetDefaultParaStyleName();
 
     // specified style - including inherited properties. Indicate whether paragraph defaults should be checked.
-    css::uno::Any GetPropertyFromStyleSheet(PropertyIds eId, StyleSheetEntryPtr pEntry, const bool bDocDefaults, const bool bPara, const bool bStyles = true);
+    css::uno::Any GetPropertyFromStyleSheet(PropertyIds eId, StyleSheetEntryPtr pEntry, const bool bDocDefaults, const bool bPara);
     // current paragraph style - including inherited properties
     css::uno::Any GetPropertyFromParaStyleSheet(PropertyIds eId);
     // context's character style - including inherited properties
@@ -1036,7 +1043,7 @@ public:
     std::vector<css::uno::Any> aFramedRedlines;
 
     /// Table paragraph properties may need style update based on table style
-    std::vector<css::uno::Reference<css::beans::XPropertySet>> m_aPendingParaProp;
+    std::vector<TableParagraph> m_aParagraphsToEndTable;
 
 private:
     void PushPageHeaderFooter(bool bHeader, SectionPropertyMap::PageType eType);