tdf#119599 RTF import: fix missing deduplication of font size

Deciding when to and when not to deduplicate repeated direct formatting
of paragraph / character properties is stricky, this bug is about a case
when deduplication should happen and did not, since commit
1970a686 (tdf#113408 RTF import style
dedup: separate paragraph and character handling, 2017-10-31).
Especially that deduplication works in both directions: it should remove
properties which are duplicated and also should insert explicit default
values for not repeated properties.

Fix the problem by making the getDefaultSPRM() aware of the context
(which style type it deals with), and then by making sure that only
default properties relevant for the given style type are inserted.

Change-Id: I35b6599cc47fa51b8754fd921c61a3b31a283547
Reviewed-on: https://gerrit.libreoffice.org/60946Reviewed-by: Miklos Vajna <vmiklos@collabora.co.uk>
Tested-by: Jenkins

sw/qa/extras/rtfimport/data/tdf119599.rtf

+{\rtf1\ansi
+{\stylesheet
+{\s0 Normal;}
+{\s146\fs32 para style;}
+}
+\paperh15840\paperw12240\margl1800\margr1800\margt1440\margb1440
+\pard\plain \s146\fs32
+hello.
+\par }

sw/qa/extras/rtfimport/rtfimport.cxx

@@ -1148,6 +1148,14 @@ DECLARE_RTFIMPORT_TEST(testTdf90260Par, "hello.rtf")
     CPPUNIT_ASSERT_EQUAL(2, getParagraphs());
 }
 
+DECLARE_RTFIMPORT_TEST(testTdf119599, "tdf119599.rtf")
+{
+    uno::Reference<beans::XPropertyState> xRun(getRun(getParagraph(1), 1), uno::UNO_QUERY);
+    // This was beans::PropertyState_DIRECT_VALUE, changing the font size in
+    // the style had no effect on the rendering result.
+    CPPUNIT_ASSERT_EQUAL(beans::PropertyState_DEFAULT_VALUE, xRun->getPropertyState("CharHeight"));
+}
+
 DECLARE_RTFIMPORT_TEST(testTdf90315, "tdf90315.rtf")
 {
     uno::Reference<text::XTextSectionsSupplier> xTextSectionsSupplier(mxComponent, uno::UNO_QUERY);

writerfilter/source/rtftok/rtfdocumentimpl.cxx

@@ -487,12 +487,10 @@ RTFDocumentImpl::getProperties(RTFSprms& rAttributes, RTFSprms const& rSprms, Id
         RTFSprms aStyleSprms;
         RTFSprms aStyleAttributes;
         // Ensure the paragraph style is a flat list.
-        if (!nStyleType || nStyleType == NS_ooxml::LN_Value_ST_StyleType_paragraph)
-        {
-            RTFReferenceProperties& rProps
-                = *static_cast<RTFReferenceProperties*>(it->second.get());
-            lcl_copyFlatten(rProps, aStyleAttributes, aStyleSprms);
-        }
+        // Take paragraph style into account for character properties as well,
+        // as paragraph style may contain character properties.
+        RTFReferenceProperties& rProps = *static_cast<RTFReferenceProperties*>(it->second.get());
+        lcl_copyFlatten(rProps, aStyleAttributes, aStyleSprms);
 
         if (itChar != m_aStyleTableEntries.end())
         {
@@ -506,8 +504,8 @@ RTFDocumentImpl::getProperties(RTFSprms& rAttributes, RTFSprms const& rSprms, Id
         }
 
         // Get rid of direct formatting what is already in the style.
-        RTFSprms const sprms(aSprms.cloneAndDeduplicate(aStyleSprms));
-        RTFSprms const attributes(rAttributes.cloneAndDeduplicate(aStyleAttributes));
+        RTFSprms const sprms(aSprms.cloneAndDeduplicate(aStyleSprms, nStyleType));
+        RTFSprms const attributes(rAttributes.cloneAndDeduplicate(aStyleAttributes, nStyleType));
         return new RTFReferenceProperties(attributes, sprms);
     }
 

writerfilter/source/rtftok/rtfsprm.cxx

@@ -137,21 +137,36 @@ void RTFSprms::eraseLast(Id nKeyword)
     }
 }
 
-static RTFValue::Pointer_t getDefaultSPRM(Id const id)
+static RTFValue::Pointer_t getDefaultSPRM(Id const id, Id nStyleType)
 {
-    switch (id)
+    if (!nStyleType || nStyleType == NS_ooxml::LN_Value_ST_StyleType_character)
     {
-        case NS_ooxml::LN_CT_Spacing_before:
-        case NS_ooxml::LN_CT_Spacing_after:
-        case NS_ooxml::LN_EG_RPrBase_b:
-        case NS_ooxml::LN_CT_Ind_left:
-        case NS_ooxml::LN_CT_Ind_right:
-        case NS_ooxml::LN_CT_Ind_firstLine:
-            return new RTFValue(0);
+        switch (id)
+        {
+            case NS_ooxml::LN_EG_RPrBase_b:
+                return new RTFValue(0);
+            default:
+                break;
+        }
+    }
 
-        default:
-            return RTFValue::Pointer_t();
+    if (!nStyleType || nStyleType == NS_ooxml::LN_Value_ST_StyleType_paragraph)
+    {
+        switch (id)
+        {
+            case NS_ooxml::LN_CT_Spacing_before:
+            case NS_ooxml::LN_CT_Spacing_after:
+            case NS_ooxml::LN_CT_Ind_left:
+            case NS_ooxml::LN_CT_Ind_right:
+            case NS_ooxml::LN_CT_Ind_firstLine:
+                return new RTFValue(0);
+
+            default:
+                break;
+        }
     }
+
+    return RTFValue::Pointer_t();
 }
 
 /// Is it problematic to deduplicate this SPRM?
@@ -199,7 +214,8 @@ static bool isSPRMChildrenExpected(Id nId)
 }
 
 /// Does the clone / deduplication of a single sprm.
-static void cloneAndDeduplicateSprm(std::pair<Id, RTFValue::Pointer_t> const& rSprm, RTFSprms& ret)
+static void cloneAndDeduplicateSprm(std::pair<Id, RTFValue::Pointer_t> const& rSprm, RTFSprms& ret,
+                                    Id nStyleType)
 {
     RTFValue::Pointer_t const pValue(ret.find(rSprm.first));
     if (pValue)
@@ -211,9 +227,10 @@ static void cloneAndDeduplicateSprm(std::pair<Id, RTFValue::Pointer_t> const& rS
         }
         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()));
+            RTFSprms const sprms(
+                pValue->getSprms().cloneAndDeduplicate(rSprm.second->getSprms(), nStyleType));
+            RTFSprms const attributes(pValue->getAttributes().cloneAndDeduplicate(
+                rSprm.second->getAttributes(), nStyleType));
             // Don't copy the sprm in case we expect it to have children but it doesn't have some.
             if (!isSPRMChildrenExpected(rSprm.first) || !sprms.empty() || !attributes.empty())
                 ret.set(rSprm.first,
@@ -223,16 +240,17 @@ static void cloneAndDeduplicateSprm(std::pair<Id, RTFValue::Pointer_t> const& rS
     else
     {
         // not found - try to override style with default
-        RTFValue::Pointer_t const pDefault(getDefaultSPRM(rSprm.first));
+        RTFValue::Pointer_t const pDefault(getDefaultSPRM(rSprm.first, nStyleType));
         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 sprms(
+                RTFSprms().cloneAndDeduplicate(rSprm.second->getSprms(), nStyleType));
             RTFSprms const attributes(
-                RTFSprms().cloneAndDeduplicate(rSprm.second->getAttributes()));
+                RTFSprms().cloneAndDeduplicate(rSprm.second->getAttributes(), nStyleType));
             if (!sprms.empty() || !attributes.empty())
             {
                 ret.set(rSprm.first, new RTFValue(attributes, sprms));
@@ -314,14 +332,14 @@ void RTFSprms::duplicateList(const RTFValue::Pointer_t& pAbstract)
                     = getNestedAttribute(*this, NS_ooxml::LN_CT_PPrBase_ind, rListLevelPair.first);
                 if (!pParagraphValue)
                     putNestedAttribute(*this, NS_ooxml::LN_CT_PPrBase_ind, rListLevelPair.first,
-                                       getDefaultSPRM(rListLevelPair.first));
+                                       getDefaultSPRM(rListLevelPair.first, 0));
 
                 break;
         }
     }
 }
 
-RTFSprms RTFSprms::cloneAndDeduplicate(RTFSprms& rReference) const
+RTFSprms RTFSprms::cloneAndDeduplicate(RTFSprms& rReference, Id nStyleType) const
 {
     RTFSprms ret(*this);
     ret.ensureCopyBeforeWrite();
@@ -337,10 +355,10 @@ RTFSprms RTFSprms::cloneAndDeduplicate(RTFSprms& rReference) const
         if (rSprm.first == NS_ooxml::LN_CT_Style_pPr)
         {
             for (auto& i : rSprm.second->getSprms())
-                cloneAndDeduplicateSprm(i, ret);
+                cloneAndDeduplicateSprm(i, ret, nStyleType);
         }
         else
-            cloneAndDeduplicateSprm(rSprm, ret);
+            cloneAndDeduplicateSprm(rSprm, ret, nStyleType);
     }
     return ret;
 }

writerfilter/source/rtftok/rtfsprm.hxx

@@ -61,7 +61,7 @@ public:
     /// Removes elements which are already in the reference set.
     /// Also insert default values to override attributes of style
     /// (yes, really; that's what Word does).
-    RTFSprms cloneAndDeduplicate(RTFSprms& rReference) const;
+    RTFSprms cloneAndDeduplicate(RTFSprms& rReference, Id nStyleType) const;
     /// Inserts default values to override attributes of pAbstract.
     void duplicateList(const RTFValue::Pointer_t& pAbstract);
     /// Removes duplicated values based on in-list properties.