[Libreoffice-commits] core.git: Branch 'libreoffice-6-2' - sw/qa writerfilter/source

[Justin Luth (via logerrit)]

 sw/qa/extras/ooxmlexport/data/tdf126723.docx      |binary
 sw/qa/extras/ooxmlexport/ooxmlexport13.cxx        |    5 +++++
 writerfilter/source/dmapper/DomainMapper_Impl.cxx |   19 ++++++++++---------
 3 files changed, 15 insertions(+), 9 deletions(-)

New commits:
commit df28caa1fc24c6288f4c10854b03612c929ee676
Author:     Justin Luth <justin.luth at collabora.com>
AuthorDate: Fri Aug 9 11:42:47 2019 +0300
Commit:     Michael Stahl <Michael.Stahl at cib.de>
CommitDate: Mon Aug 12 16:53:05 2019 +0200

    tdf#126723 writerfilter::finishParagraph - me, not previous
    
    In LO 6.2 commit 480ac84f2f5049fb4337b36f12fd6796e005761b
    the existing m_xPreviousParagraph was conveniently used to
    apply the changed properties. I never did like that choice,
    but despite looking at it, I failed to see that it is set
    in an inside loop, which means that it was NOT NECESSARILY
    reset to the current paragaph. So I'm happy to have proof
    that we should not use m_xPreviousParagraph.
    
    Reviewed-on: https://gerrit.libreoffice.org/77185
    Tested-by: Jenkins
    Reviewed-by: Justin Luth <justin_luth at sil.org>
    Reviewed-by: László Németh <nemeth at numbertext.org>
    (cherry picked from commit d03c92b93d6ba1808a6641b4aa8cb4aae38058bf)
    
    Change-Id: I5c7f1b0f097711d65ae0d0be1f0fbc40c8b96e9d
    Reviewed-on: https://gerrit.libreoffice.org/77249
    Tested-by: Jenkins
    Reviewed-by: Michael Stahl <Michael.Stahl at cib.de>

diff --git a/sw/qa/extras/ooxmlexport/data/tdf126723.docx b/sw/qa/extras/ooxmlexport/data/tdf126723.docx
new file mode 100644
index 000000000000..297ea322fa7d
Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/tdf126723.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx
index d9f1978705cf..a3a7184b34e0 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx
@@ -52,6 +52,11 @@ DECLARE_OOXMLEXPORT_TEST(testTdf121374_sectionHF2, "tdf121374_sectionHF2.doc")
     CPPUNIT_ASSERT( xHeaderText->getString().startsWith("virkamatka-anomus") );
 }
 
+DECLARE_OOXMLEXPORT_TEST(testTdf126723, "tdf126723.docx")
+{
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0), getProperty<sal_Int32>(getParagraph(2), "ParaLeftMargin"));
+}
+
 DECLARE_OOXMLEXPORT_TEST(testTdf121867, "tdf121867.odt")
 {
     SwXTextDocument* pTextDoc = dynamic_cast<SwXTextDocument*>(mxComponent.get());
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index d0cbcaa1c5d6..c6b7413f741e 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -1496,39 +1496,40 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
                     CheckParaMarkerRedline( xParaEnd );
                 }
 
+                css::uno::Reference<css::beans::XPropertySet> xParaProps(xTextRange, uno::UNO_QUERY);
                 // tdf#118521 set paragraph top or bottom margin based on the paragraph style
                 // if we already set the other margin with direct formatting
-                if ( pParaContext && m_xPreviousParagraph.is() )
+                if (xParaProps)
                 {
                     const bool bTopSet = pParaContext->isSet(PROP_PARA_TOP_MARGIN);
                     const bool bBottomSet = pParaContext->isSet(PROP_PARA_BOTTOM_MARGIN);
                     const bool bContextSet = pParaContext->isSet(PROP_PARA_CONTEXT_MARGIN);
                     if ( !(bTopSet == bBottomSet && bBottomSet == bContextSet) )
                     {
+
                         if ( !bTopSet )
                         {
                             uno::Any aMargin = GetPropertyFromStyleSheet(PROP_PARA_TOP_MARGIN);
                             if ( aMargin != uno::Any() )
-                                m_xPreviousParagraph->setPropertyValue("ParaTopMargin", aMargin);
+                                xParaProps->setPropertyValue("ParaTopMargin", aMargin);
                         }
                         if ( !bBottomSet )
                         {
                             uno::Any aMargin = GetPropertyFromStyleSheet(PROP_PARA_BOTTOM_MARGIN);
                             if ( aMargin != uno::Any() )
-                                m_xPreviousParagraph->setPropertyValue("ParaBottomMargin", aMargin);
+                                xParaProps->setPropertyValue("ParaBottomMargin", aMargin);
                         }
                         if ( !bContextSet )
                         {
                             uno::Any aMargin = GetPropertyFromStyleSheet(PROP_PARA_CONTEXT_MARGIN);
                             if ( aMargin != uno::Any() )
-                                m_xPreviousParagraph->setPropertyValue("ParaContextMargin", aMargin);
+                                xParaProps->setPropertyValue("ParaContextMargin", aMargin);
                         }
                     }
                 }
 
                 // Left, Right, and Hanging settings are also grouped. Ensure that all or none are set.
-                // m_xPreviousParagraph was set earlier, so really it still is the current paragraph...
-                if ( pParaContext && m_xPreviousParagraph.is() )
+                if (xParaProps)
                 {
                     const bool bLeftSet  = pParaContext->isSet(PROP_PARA_LEFT_MARGIN);
                     const bool bRightSet = pParaContext->isSet(PROP_PARA_RIGHT_MARGIN);
@@ -1539,19 +1540,19 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
                         {
                             uno::Any aMargin = GetPropertyFromStyleSheet(PROP_PARA_LEFT_MARGIN);
                             if ( aMargin != uno::Any() )
-                                m_xPreviousParagraph->setPropertyValue("ParaLeftMargin", aMargin);
+                                xParaProps->setPropertyValue("ParaLeftMargin", aMargin);
                         }
                         if ( !bRightSet )
                         {
                             uno::Any aMargin = GetPropertyFromStyleSheet(PROP_PARA_RIGHT_MARGIN);
                             if ( aMargin != uno::Any() )
-                                m_xPreviousParagraph->setPropertyValue("ParaRightMargin", aMargin);
+                                xParaProps->setPropertyValue("ParaRightMargin", aMargin);
                         }
                         if ( !bFirstSet )
                         {
                             uno::Any aMargin = GetPropertyFromStyleSheet(PROP_PARA_FIRST_LINE_INDENT);
                             if ( aMargin != uno::Any() )
-                                m_xPreviousParagraph->setPropertyValue("ParaFirstLineIndent", aMargin);
+                                xParaProps->setPropertyValue("ParaFirstLineIndent", aMargin);
                         }
                     }
                 }