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

[Miklos Vajna]

 sw/qa/extras/rtfimport/data/tdf104744.rtf |   24 +++++++++
 sw/qa/extras/rtfimport/rtfimport.cxx      |    7 ++
 writerfilter/source/rtftok/rtfsprm.cxx    |   74 ++++++++++++++++++------------
 3 files changed, 76 insertions(+), 29 deletions(-)

New commits:
commit d66a9721913e89f1cc76068ee1a64b1cf0eef8d8
Author: Miklos Vajna <vmiklos at collabora.co.uk>
Date:   Tue Jan 3 08:49:43 2017 +0100

    tdf#104744 RTF import: fix unexpected zero para left margin wrt style dedup
    
    See commit 1be0a3fa9ebb22b607c54b47739d4467acfed259 (n#825305:
    writerfilter RTF import: override style properties like Word,
    2014-06-17) for the details on style override in RTF.
    
    Here the problem was that we added an unneeded "reset to 0" property, the
    opposite situation that commit 657c6cc3acec0528209a8584b838cd6de581c437
    (tdf#104228 RTF import: fix override of style left/right para margin,
    2016-12-13) was fixing (there a "reset to 0" was missing).
    
    (cherry picked from commit a9e029ace41562e28e9242d63230ad1ca275f5d3)
    
    Change-Id: I37f079b9cb4773214d2531c2e34920b3b8927211
    Reviewed-on: https://gerrit.libreoffice.org/32718
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Michael Stahl <mstahl at redhat.com>

diff --git a/sw/qa/extras/rtfimport/data/tdf104744.rtf b/sw/qa/extras/rtfimport/data/tdf104744.rtf
new file mode 100644
index 0000000..ef32998
--- /dev/null
+++ b/sw/qa/extras/rtfimport/data/tdf104744.rtf
@@ -0,0 +1,24 @@
+{\rtf1
+{\fonttbl
+{\f3\fcharset2 Symbol;}
+}
+{\stylesheet
+{\s15\li720 List Paragraph;}
+}
+{\*\listtable
+{\list\listtemplateid1145476866\listhybrid
+{\listlevel\levelnfc23\levelnfcn23\leveljc0\leveljcn0\levelfollow0\levelstartat1
+\levelspace360\levelindent0
+{\leveltext\leveltemplateid67698689\'01\u-3913 ?;}
+{\levelnumbers;}
+\f3\li720 }
+\listid1805847239}
+}
+{\*\listoverridetable
+{\listoverride\listid1805847239\listoverridecount0\ls1}
+}
+{\pard\plain \ltrpar\s15\li720\ls1 bullet
+\par 
+}
+{\pard\plain after}
+}
diff --git a/sw/qa/extras/rtfimport/rtfimport.cxx b/sw/qa/extras/rtfimport/rtfimport.cxx
index 2f554e1..c860f3d 100644
--- a/sw/qa/extras/rtfimport/rtfimport.cxx
+++ b/sw/qa/extras/rtfimport/rtfimport.cxx
@@ -2721,6 +2721,13 @@ DECLARE_RTFIMPORT_TEST(testTdf104317, "tdf104317.rtf")
     CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(1), xDrawPage->getCount());
 }
 
+DECLARE_RTFIMPORT_TEST(testTdf104744, "tdf104744.rtf")
+{
+    // This was 0, as an unexpected "left margin is 0" token was created during
+    // import.
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(1270), getProperty<sal_Int32>(getParagraph(1), "ParaLeftMargin"));
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/writerfilter/source/rtftok/rtfsprm.cxx b/writerfilter/source/rtftok/rtfsprm.cxx
index c7096f1..da7bc81 100644
--- a/writerfilter/source/rtftok/rtfsprm.cxx
+++ b/writerfilter/source/rtftok/rtfsprm.cxx
@@ -153,6 +153,43 @@ static RTFValue::Pointer_t getDefaultSPRM(Id const id)
     }
 }
 
+/// Does the clone / deduplication of a single sprm.
+static void cloneAndDeduplicateSprm(std::pair<Id, RTFValue::Pointer_t>& rSprm, RTFSprms& ret)
+{
+    RTFValue::Pointer_t const pValue(ret.find(rSprm.first));
+    if (pValue)
+    {
+        if (rSprm.second->equals(*pValue))
+        {
+            ret.erase(rSprm.first); // duplicate to style
+        }
+        else if (!rSprm.second->getSprms().empty() || !rSprm.second->getAttributes().empty())
+        {
+            RTFSprms const sprms(pValue->getSprms().cloneAndDeduplicate(rSprm.second->getSprms()));
+            RTFSprms const attributes(pValue->getAttributes().cloneAndDeduplicate(rSprm.second->getAttributes()));
+            ret.set(rSprm.first, RTFValue::Pointer_t(pValue->CloneWithSprms(attributes, sprms)));
+        }
+    }
+    else
+    {
+        // not found - try to override style with default
+        RTFValue::Pointer_t const pDefault(getDefaultSPRM(rSprm.first));
+        if (pDefault)
+        {
+            ret.set(rSprm.first, pDefault);
+        }
+        else if (!rSprm.second->getSprms().empty() || !rSprm.second->getAttributes().empty())
+        {
+            RTFSprms const sprms(RTFSprms().cloneAndDeduplicate(rSprm.second->getSprms()));
+            RTFSprms const attributes(RTFSprms().cloneAndDeduplicate(rSprm.second->getAttributes()));
+            if (!sprms.empty() || !attributes.empty())
+            {
+                ret.set(rSprm.first, std::make_shared<RTFValue>(attributes, sprms));
+            }
+        }
+    }
+}
+
 RTFSprms RTFSprms::cloneAndDeduplicate(RTFSprms& rReference) const
 {
     RTFSprms ret(*this);
@@ -162,38 +199,17 @@ RTFSprms RTFSprms::cloneAndDeduplicate(RTFSprms& rReference) const
     // it is probably a bad idea to mess with those in any way here?
     for (auto& rSprm : rReference)
     {
-        RTFValue::Pointer_t const pValue(ret.find(rSprm.first));
-        if (pValue)
+        // Paragraph formatting sprms are directly contained in case of
+        // paragraphs, but they are below NS_ooxml::LN_CT_Style_pPr in case of
+        // styles. So handle those children directly, to avoid unexpected
+        // addition of direct formatting sprms at the paragraph level.
+        if (rSprm.first == NS_ooxml::LN_CT_Style_pPr)
         {
-            if (rSprm.second->equals(*pValue))
-            {
-                ret.erase(rSprm.first); // duplicate to style
-            }
-            else if (!rSprm.second->getSprms().empty() || !rSprm.second->getAttributes().empty())
-            {
-                RTFSprms const sprms(pValue->getSprms().cloneAndDeduplicate(rSprm.second->getSprms()));
-                RTFSprms const attributes(pValue->getAttributes().cloneAndDeduplicate(rSprm.second->getAttributes()));
-                ret.set(rSprm.first, RTFValue::Pointer_t(pValue->CloneWithSprms(attributes, sprms)));
-            }
+            for (auto& i : rSprm.second->getSprms())
+                cloneAndDeduplicateSprm(i, ret);
         }
         else
-        {
-            // not found - try to override style with default
-            RTFValue::Pointer_t const pDefault(getDefaultSPRM(rSprm.first));
-            if (pDefault)
-            {
-                ret.set(rSprm.first, pDefault);
-            }
-            else if (!rSprm.second->getSprms().empty() || !rSprm.second->getAttributes().empty())
-            {
-                RTFSprms const sprms(RTFSprms().cloneAndDeduplicate(rSprm.second->getSprms()));
-                RTFSprms const attributes(RTFSprms().cloneAndDeduplicate(rSprm.second->getAttributes()));
-                if (!sprms.empty() || !attributes.empty())
-                {
-                    ret.set(rSprm.first, std::make_shared<RTFValue>(attributes, sprms));
-                }
-            }
-        }
+            cloneAndDeduplicateSprm(rSprm, ret);
     }
     return ret;
 }