tdf#158044 handling of paragraph attributes in RTF import

style attributes need to be repeated at the paragraph
if not repeated then defaults have to be applied

Change-Id: I4ee567e8006c240c046b7e7bb75eae92e5563776
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/158839
Tested-by: Jenkins
Reviewed-by: Thorsten Behrens <thorsten.behrens@allotropia.de>

5 files changed, 163 insertions, 3 deletions

diff --git a/sw/qa/extras/rtfimport/data/tdf158044.rtf b/sw/qa/extras/rtfimport/data/tdf158044.rtf
new file mode 100755
index 000000000000..65e687f25d39
--- /dev/null
+++ b/sw/qa/extras/rtfimport/data/tdf158044.rtf
@@ -0,0 +1,20 @@
+{\rtf1\ansi

+{\colortbl;\red0\green0\blue0;\red0\green0\blue255;

+\red0\green255\blue255;\red0\green255\blue0;\red255\green0\blue255;\red255\green0\blue0;\red255\green255\blue0;\red255\green255\blue255;\red0\green0\blue128;\red0\green128\blue128;\red0\green128\blue0;\red128\green0\blue128;\red128\green0\blue0;

+\red128\green128\blue0;\red128\green128\blue128;\red192\green192\blue192;\red0\green0\blue0;\red0\green0\blue0;}

+

+{\stylesheet

+{\fs24\sa3200 Normal;}

+{\s97\tx1701\tx2835 StyleTabstops;}

+{\s98\cbpat6  StyleRed;}

+{\s99\qc SyleCentered;}

+}

+

+

+{\s97 Paragraph without tabstops\par}

+{\s98 Pargraph without color \par}

+{\s99 Paragraph without center\par}

+{\s97\tx1701\tx2835 Paragraph with tabstops\par}

+{\s98\cbpat6  Pargraph with color \par}

+{\s99\qc Paragraph with center\par}

+}

diff --git a/sw/qa/extras/rtfimport/rtfimport.cxx b/sw/qa/extras/rtfimport/rtfimport.cxx
index 78409762e827..b1b00cf2b1b4 100644
--- a/sw/qa/extras/rtfimport/rtfimport.cxx
+++ b/sw/qa/extras/rtfimport/rtfimport.cxx
@@ -1712,6 +1712,53 @@ CPPUNIT_TEST_FIXTURE(Test, testParaStyleBottomMargin)
                          getProperty<style::LineSpacing>(xPara, "ParaLineSpacing").Height);
 }
 
+CPPUNIT_TEST_FIXTURE(Test, test158044Tdf)
+{
+    createSwDoc("tdf158044.rtf");
+
+    uno::Reference<text::XTextDocument> xTextDocument(mxComponent, uno::UNO_QUERY);
+    uno::Reference<container::XEnumerationAccess> xParaEnumAccess(xTextDocument->getText(),
+                                                                  uno::UNO_QUERY);
+    uno::Reference<container::XEnumeration> xParaEnum = xParaEnumAccess->createEnumeration();
+    int paraIndex = 0;
+    while (xParaEnum->hasMoreElements())
+    {
+        uno::Reference<beans::XPropertySet> xPropertySet(xParaEnum->nextElement(), uno::UNO_QUERY);
+        sal_Int16 adjust = getProperty<sal_Int16>(xPropertySet, "ParaAdjust");
+        Color fillColor = getProperty<Color>(xPropertySet, "FillColor");
+        drawing::FillStyle fillStyle = getProperty<drawing::FillStyle>(xPropertySet, "FillStyle");
+        uno::Sequence<style::TabStop> tabStops
+            = getProperty<uno::Sequence<style::TabStop>>(xPropertySet, "ParaTabStops");
+        switch (paraIndex)
+        {
+            case 0:
+                CPPUNIT_ASSERT_EQUAL(sal_Int32(0), tabStops.getLength());
+                break;
+            case 1:
+                CPPUNIT_ASSERT_EQUAL(drawing::FillStyle_NONE, fillStyle);
+                CPPUNIT_ASSERT(0xffffff == fillColor);
+                break;
+            case 2:
+                CPPUNIT_ASSERT_EQUAL(sal_Int16(0), adjust);
+                break;
+            case 3:
+                CPPUNIT_ASSERT_EQUAL(sal_Int32(2), tabStops.getLength());
+                break;
+            case 4:
+                CPPUNIT_ASSERT(tabStops.getLength() < 2);
+                CPPUNIT_ASSERT_EQUAL(drawing::FillStyle_SOLID, fillStyle);
+                CPPUNIT_ASSERT(0xff0000 == fillColor);
+                break;
+            case 5:
+                CPPUNIT_ASSERT(tabStops.getLength() < 2);
+                CPPUNIT_ASSERT_EQUAL(drawing::FillStyle_NONE, fillStyle);
+                break;
+            default:
+                break;
+        }
+        ++paraIndex;
+    }
+}
 // tests should only be added to rtfIMPORT *if* they fail round-tripping in rtfEXPORT
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/writerfilter/source/dmapper/DomainMapper.cxx b/writerfilter/source/dmapper/DomainMapper.cxx
index f015b79520bf..a2399e44bf37 100644
--- a/writerfilter/source/dmapper/DomainMapper.cxx
+++ b/writerfilter/source/dmapper/DomainMapper.cxx
@@ -34,6 +34,7 @@
 #include <oox/drawingml/drawingmltypes.hxx>
 #include <com/sun/star/document/XDocumentPropertiesSupplier.hpp>
 #include <com/sun/star/document/XOOXMLDocumentPropertiesImporter.hpp>
+#include <com/sun/star/drawing/FillStyle.hpp>
 #include <com/sun/star/drawing/TextVerticalAdjust.hpp>
 #include <com/sun/star/table/BorderLineStyle.hpp>
 #include <com/sun/star/table/ShadowFormat.hpp>
@@ -3845,6 +3846,35 @@ void DomainMapper::lcl_endCharacterGroup()
     m_pImpl->PopProperties(CONTEXT_CHARACTER);
 }
 
+//copied from rtfsprm
+/// Is it problematic to deduplicate this SPRM?
+static bool isSPRMDeduplicateDenylist(PropertyIds nId, PropertyMapPtr pContext)
+{
+    switch (nId)
+    {
+        // See the NS_ooxml::LN_CT_PPrBase_tabs handler in DomainMapper,
+        // deduplication is explicitly not wanted for these tokens.
+    case PROP_PARA_TAB_STOPS:
+    case PROP_PARA_LINE_SPACING:
+        return true;
+    case PROP_TOP_BORDER:
+    case PROP_LEFT_BORDER:
+    case PROP_BOTTOM_BORDER:
+    case PROP_RIGHT_BORDER:
+    case META_PROP_HORIZONTAL_BORDER:
+    case META_PROP_VERTICAL_BORDER:
+        return true;
+        // Removing \fi and \li if the style has the same value would mean taking these values from
+        // \ls, while deduplication would be done to take the values from the style.
+    case PROP_PARA_FIRST_LINE_INDENT:
+    case PROP_PARA_LEFT_MARGIN:
+        return pContext && pContext->getProperty(PROP_NUMBERING_RULES);
+
+    default:
+        return false;
+    }
+}
+
 void DomainMapper::lcl_text(const sal_uInt8 * data_, size_t len)
 {
     //TODO: Determine the right text encoding (FIB?)
@@ -3895,6 +3925,65 @@ void DomainMapper::lcl_text(const sal_uInt8 * data_, size_t len)
                         pContext->Insert(PROP_BREAK_TYPE, uno::Any(style::BreakType_COLUMN_BEFORE));
                         m_pImpl->clearDeferredBreak(COLUMN_BREAK);
                     }
+                    if (IsRTFImport() && pContext) {
+                        //reset paragraph style properties not repeated at the paragraph
+                        std::optional<PropertyMap::Property> paraStyleName = pContext->getProperty(PROP_PARA_STYLE_NAME);
+                        if (paraStyleName.has_value()) {
+                            OUString uStyleName;
+                            paraStyleName->second >>= uStyleName;
+                            StyleSheetEntryPtr pStyleSheet = m_pImpl->GetStyleSheetTable()->FindStyleSheetByConvertedStyleName(uStyleName);
+                            if (pStyleSheet != nullptr)
+                            {
+                                std::vector< PropertyIds > stylePropertyIds = pStyleSheet->m_pProperties->GetPropertyIds();
+                                std::vector< PropertyIds >::iterator stylePropertyIdsIt = stylePropertyIds.begin();
+                                while (stylePropertyIdsIt != stylePropertyIds.end())
+                                {
+                                    PropertyIds ePropertyId = *stylePropertyIdsIt;
+                                    std::optional< PropertyMap::Property > styleProperty = pStyleSheet->m_pProperties->getProperty(ePropertyId);
+                                    std::optional< PropertyMap::Property > paragraphProperty = pContext->getProperty(ePropertyId);
+                                    if (paragraphProperty.has_value()) {
+                                        if (paragraphProperty->second == styleProperty->second &&
+                                            !isSPRMDeduplicateDenylist(ePropertyId, pContext))
+                                        {
+                                            pContext->Erase(ePropertyId);
+                                        }
+                                    }
+                                    else
+                                    {
+                                        switch (ePropertyId)
+                                        {
+                                        case PROP_PARA_LEFT_MARGIN:
+                                            if (!pContext->getProperty(PROP_NUMBERING_RULES))
+                                            {
+                                                pContext->Insert(ePropertyId, uno::Any(0l));
+                                            }
+                                            break;
+                                        case PROP_PARA_RIGHT_MARGIN:
+                                            pContext->Insert(ePropertyId, uno::Any(0l));
+                                            break;
+                                        case PROP_PARA_LAST_LINE_ADJUST:
+                                        case PROP_PARA_ADJUST:
+                                            pContext->Insert(ePropertyId, uno::Any(style::ParagraphAdjust_LEFT));
+                                            break;
+                                        case PROP_PARA_TAB_STOPS:
+                                            pContext->Insert(ePropertyId, uno::Any(uno::Sequence< style::TabStop >()));
+                                            break;
+                                        case PROP_FILL_STYLE:
+                                            pContext->Insert(ePropertyId, uno::Any(drawing::FillStyle_NONE));
+                                            break;
+                                        case PROP_FILL_COLOR:
+                                            pContext->Insert(ePropertyId, uno::Any(sal_Int32(COL_TRANSPARENT)));
+                                            break;
+                                        case INVALID:
+                                        default:
+                                            break;
+                                        }
+                                    }
+                                    ++stylePropertyIdsIt;
+                                }
+                            }
+                        }
+                    }
                     finishParagraph();
                     return;
                 }
diff --git a/writerfilter/source/rtftok/rtfdocumentimpl.cxx b/writerfilter/source/rtftok/rtfdocumentimpl.cxx
index 11e251b0828a..45f49553b26b 100644
--- a/writerfilter/source/rtftok/rtfdocumentimpl.cxx
+++ b/writerfilter/source/rtftok/rtfdocumentimpl.cxx
@@ -558,7 +558,6 @@ RTFDocumentImpl::getProperties(const RTFSprms& rAttributes, RTFSprms const& rSpr
             }
         }
 
-        // Get rid of direct formatting what is already in the style.
         RTFSprms sprms(aSprms.cloneAndDeduplicate(aStyleSprms, nStyleType, true, &aSprms));
         RTFSprms attributes(rAttributes.cloneAndDeduplicate(aStyleAttributes, nStyleType, true));
         return new RTFReferenceProperties(std::move(attributes), std::move(sprms));
diff --git a/writerfilter/source/rtftok/rtfsprm.cxx b/writerfilter/source/rtftok/rtfsprm.cxx
index 3efb60eb29d8..d98ea74abd55 100644
--- a/writerfilter/source/rtftok/rtfsprm.cxx
+++ b/writerfilter/source/rtftok/rtfsprm.cxx
@@ -290,9 +290,14 @@ static void cloneAndDeduplicateSprm(std::pair<Id, RTFValue::Pointer_t> const& rS
     {
         if (rSprm.second->equals(*pValue))
         {
-            if (!isSPRMDeduplicateDenylist(rSprm.first, pDirect))
+            //this removes properties that are equal at the style and at the sprm
+            //don't do that for paragraph styles
+            if (nStyleType != NS_ooxml::LN_Value_ST_StyleType_paragraph)
             {
-                ret.erase(rSprm.first); // duplicate to style
+                if (!isSPRMDeduplicateDenylist(rSprm.first, pDirect))
+                {
+                    ret.erase(rSprm.first); // duplicate to style
+                }
             }
         }
         else if (!rSprm.second->getSprms().empty() || !rSprm.second->getAttributes().empty())