[Libreoffice-commits] core.git: 2 commits - sw/qa writerfilter/source

Michael Stahl mstahl at redhat.com
Tue Jun 17 09:47:59 PDT 2014


 sw/qa/extras/rtfimport/data/fdo70578.rtf       |   11 +++
 sw/qa/extras/rtfimport/rtfimport.cxx           |   21 +++++++
 writerfilter/source/rtftok/rtfdocumentimpl.cxx |   48 ++++++++++++++++-
 writerfilter/source/rtftok/rtfsprm.cxx         |   70 +++++++++++++++++++++----
 writerfilter/source/rtftok/rtfsprm.hxx         |    6 +-
 writerfilter/source/rtftok/rtfvalue.cxx        |    8 ++
 writerfilter/source/rtftok/rtfvalue.hxx        |    1 
 7 files changed, 149 insertions(+), 16 deletions(-)

New commits:
commit 1be0a3fa9ebb22b607c54b47739d4467acfed259
Author: Michael Stahl <mstahl at redhat.com>
Date:   Tue Jun 17 18:40:04 2014 +0200

    n#825305: writerfilter RTF import: override style properties like Word
    
    It would certainly be immediately obvious to any reader of the RTF spec
    that \sN will apply the style with index N to the current paragraph.
    
    But actually, that is not what Word does when it reads \sN...
    what it really does is to apply the style with index N, and then for
    every attribute in that style, apply the same attribute with a default
    value to the paragraph, effectively overriding what's in the style.
    
    If that doesn't make any sense to you, well, have you heard the joke
    about how many Microsoft engineers it takes to change a light bulb?
    
    Also, \pard apparently implies \s0.
    
    To implement that, change RTFSprms::deduplicate() to recursively
    look for style SPRMs that are missing in the properties, and put
    in default ones, currently just for 2 keywords \sa and \sb.
    
    This requires changing deduplicate() to be const and return a new value,
    since it is no longer idempotent, as the erased SPRMs would get
    defaulted on the next run.
    
    While at it, fix RTFValue::equals() which did not compare m_sValue.
    
    This fixes the testParaBottomMargin test that was broken by the fix
    for fdo#70578.
    
    Change-Id: I4ced38628d76f6c41b488d608a804883493ff00b

diff --git a/sw/qa/extras/rtfimport/rtfimport.cxx b/sw/qa/extras/rtfimport/rtfimport.cxx
index 3699db8..79a4494 100644
--- a/sw/qa/extras/rtfimport/rtfimport.cxx
+++ b/sw/qa/extras/rtfimport/rtfimport.cxx
@@ -1173,8 +1173,18 @@ DECLARE_RTFIMPORT_TEST(testN825305, "n825305.rtf")
 
 DECLARE_RTFIMPORT_TEST(testParaBottomMargin, "para-bottom-margin.rtf")
 {
+    uno::Reference<beans::XPropertySet> xPropertySet(
+        getStyles("ParagraphStyles")->getByName("Standard"), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(353),
+            getProperty<sal_Int32>(xPropertySet, "ParaBottomMargin"));
+
     // This was 353, i.e. bottom margin of the paragraph was 0.35cm instead of 0.
+    // The reason why this is 0 despite the default style containing \sa200
+    // is that Word will actually interpret \sN (or \pard which apparently
+    // implies \s0) as "set style N and for every attribute of that style,
+    // set an attribute with default value on the paragraph"
     CPPUNIT_ASSERT_EQUAL(sal_Int32(0), getProperty<sal_Int32>(getParagraph(1), "ParaBottomMargin"));
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(2), getProperty<sal_Int32>(getParagraph(1), "ParaTopMargin"));
 }
 
 DECLARE_RTFIMPORT_TEST(testN823655, "n823655.rtf")
diff --git a/writerfilter/source/rtftok/rtfdocumentimpl.cxx b/writerfilter/source/rtftok/rtfdocumentimpl.cxx
index 2d2dad7..465ada3 100644
--- a/writerfilter/source/rtftok/rtfdocumentimpl.cxx
+++ b/writerfilter/source/rtftok/rtfdocumentimpl.cxx
@@ -443,8 +443,12 @@ writerfilter::Reference<Properties>::Pointer_t RTFDocumentImpl::getProperties(RT
     {
         RTFReferenceProperties& rProps = *(RTFReferenceProperties*)it->second.get();
         // Get rid of direct formatting what is already in the style.
-        rSprms.deduplicate(rProps.getSprms());
-        rAttributes.deduplicate(rProps.getAttributes());
+        RTFSprms const sprms(
+                rSprms.cloneAndDeduplicate(rProps.getSprms()));
+        RTFSprms const attributes(
+                rAttributes.cloneAndDeduplicate(rProps.getAttributes()));
+        return writerfilter::Reference<Properties>::Pointer_t(
+                new RTFReferenceProperties(attributes, sprms));
     }
     writerfilter::Reference<Properties>::Pointer_t pRet(new RTFReferenceProperties(rAttributes, rSprms));
     return pRet;
@@ -2759,6 +2763,7 @@ int RTFDocumentImpl::dispatchFlag(RTFKeyword nKeyword)
         // \pard is allowed between \cell and \row, but in that case it should not reset the fact that we're inside a table.
         m_aStates.top().aParagraphSprms = m_aDefaultState.aParagraphSprms;
         m_aStates.top().aParagraphAttributes = m_aDefaultState.aParagraphAttributes;
+
         if (m_nTopLevelCells == 0 && m_nNestedCells == 0)
         {
             // Reset that we're in a table.
@@ -2772,7 +2777,20 @@ int RTFDocumentImpl::dispatchFlag(RTFKeyword nKeyword)
         m_aStates.top().resetFrame();
 
         // Reset currently selected paragraph style as well.
-        m_aStates.top().nCurrentStyleIndex = -1;
+        // By default the style with index 0 is applied.
+        {
+            OUString const aName = getStyleName(0);
+            if (!aName.isEmpty())
+            {
+                m_aStates.top().aParagraphSprms.set(NS_ooxml::LN_CT_PPrBase_pStyle,
+                    RTFValue::Pointer_t(new RTFValue(aName)));
+                m_aStates.top().nCurrentStyleIndex = 0;
+            }
+            else
+            {
+                m_aStates.top().nCurrentStyleIndex = -1;
+            }
+        }
         break;
     case RTF_SECTD:
     {
diff --git a/writerfilter/source/rtftok/rtfsprm.cxx b/writerfilter/source/rtftok/rtfsprm.cxx
index c6349cb..40b8fcc 100644
--- a/writerfilter/source/rtftok/rtfsprm.cxx
+++ b/writerfilter/source/rtftok/rtfsprm.cxx
@@ -8,8 +8,10 @@
  */
 
 #include <rtfsprm.hxx>
+
 #include <rtl/strbuf.hxx>
 
+#include <ooxml/resourceids.hxx>
 #include <resourcemodel/QNameToString.hxx>
 
 
@@ -138,22 +140,68 @@ bool RTFSprms::erase(Id nKeyword)
     return false;
 }
 
-void RTFSprms::deduplicate(RTFSprms& rReference)
+static RTFValue::Pointer_t getDefaultSPRM(Id const id)
 {
-    ensureCopyBeforeWrite();
+    switch (id)
+    {
+        case NS_ooxml::LN_CT_Spacing_before:
+        case NS_ooxml::LN_CT_Spacing_after:
+            return RTFValue::Pointer_t(new RTFValue(0));
 
-    RTFSprms::Iterator_t i = m_pSprms->begin();
-    while (i != m_pSprms->end())
+        default:
+            return 0;
+    }
+}
+
+RTFSprms RTFSprms::cloneAndDeduplicate(RTFSprms& rReference) const
+{
+    RTFSprms ret(*this);
+    ret.ensureCopyBeforeWrite();
+
+    // Note: apparently some attributes are set with OVERWRITE_NO_APPEND;
+    // it is probably a bad idea to mess with those in any way here?
+    for (RTFSprms::Iterator_t i = rReference.begin(); i != rReference.end(); ++i)
     {
-        bool bIgnore = false;
-        RTFValue::Pointer_t pValue(rReference.find(i->first));
-        if (pValue.get() && i->second->equals(*pValue))
-            bIgnore = true;
-        if (bIgnore)
-            i = m_pSprms->erase(i);
+        RTFValue::Pointer_t const pValue(ret.find(i->first));
+        if (pValue)
+        {
+            if (i->second->equals(*pValue))
+            {
+                ret.erase(i->first); // duplicate to style
+            }
+            else if (!i->second->getSprms().empty() || !i->second->getAttributes().empty())
+            {
+                RTFSprms const sprms(
+                    pValue->getSprms().cloneAndDeduplicate(i->second->getSprms()));
+                RTFSprms const attributes(
+                    pValue->getAttributes().cloneAndDeduplicate(i->second->getAttributes()));
+                ret.set(i->first, RTFValue::Pointer_t(
+                            pValue->CloneWithSprms(attributes, sprms)));
+            }
+        }
         else
-            ++i;
+        {
+            // not found - try to override style with default
+            RTFValue::Pointer_t const pDefault(getDefaultSPRM(i->first));
+            if (pDefault)
+            {
+                ret.set(i->first, pDefault);
+            }
+            else if (!i->second->getSprms().empty() || !i->second->getAttributes().empty())
+            {
+                RTFSprms const sprms(
+                    RTFSprms().cloneAndDeduplicate(i->second->getSprms()));
+                RTFSprms const attributes(
+                    RTFSprms().cloneAndDeduplicate(i->second->getAttributes()));
+                if (!sprms.empty() || !attributes.empty())
+                {
+                    ret.set(i->first,
+                        RTFValue::Pointer_t(new RTFValue(attributes, sprms)));
+                }
+            }
+        }
     }
+    return ret;
 }
 
 bool RTFSprms::equals(RTFValue& rOther)
diff --git a/writerfilter/source/rtftok/rtfsprm.hxx b/writerfilter/source/rtftok/rtfsprm.hxx
index f6c9f5b..ccf25ad 100644
--- a/writerfilter/source/rtftok/rtfsprm.hxx
+++ b/writerfilter/source/rtftok/rtfsprm.hxx
@@ -67,8 +67,10 @@ public:
     /// Does the same as ->push_back(), except that it can overwrite or ignore existing entries.
     void set(Id nKeyword, RTFValue::Pointer_t pValue, RTFOverwrite eOverwrite = OVERWRITE_YES);
     bool erase(Id nKeyword);
-    /// Removes elements, which are already in the reference set.
-    void deduplicate(RTFSprms& rReference);
+    /// 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;
     size_t size() const
     {
         return m_pSprms->size();
diff --git a/writerfilter/source/rtftok/rtfvalue.cxx b/writerfilter/source/rtftok/rtfvalue.cxx
index 5d03882..adf7a77 100644
--- a/writerfilter/source/rtftok/rtfvalue.cxx
+++ b/writerfilter/source/rtftok/rtfvalue.cxx
@@ -223,10 +223,18 @@ RTFValue* RTFValue::Clone()
     return new RTFValue(m_nValue, m_sValue, *m_pAttributes, *m_pSprms, m_xShape, m_xStream, m_xObject, m_bForceString, *m_pShape);
 }
 
+RTFValue* RTFValue::CloneWithSprms(
+        RTFSprms const& rAttributes, RTFSprms const& rSprms)
+{
+    return new RTFValue(m_nValue, m_sValue, rAttributes, rSprms, m_xShape, m_xStream, m_xObject, m_bForceString, *m_pShape);
+}
+
 bool RTFValue::equals(RTFValue& rOther)
 {
     if (m_nValue != rOther.m_nValue)
         return false;
+    if (m_sValue != rOther.m_sValue)
+        return false;
     if (m_pAttributes->size() != rOther.m_pAttributes->size())
         return false;
     else if (!m_pAttributes->equals(rOther))
diff --git a/writerfilter/source/rtftok/rtfvalue.hxx b/writerfilter/source/rtftok/rtfvalue.hxx
index 5a6d204..797b340 100644
--- a/writerfilter/source/rtftok/rtfvalue.hxx
+++ b/writerfilter/source/rtftok/rtfvalue.hxx
@@ -48,6 +48,7 @@ public:
     virtual writerfilter::Reference<BinaryObj>::Pointer_t getBinary() SAL_OVERRIDE;
     virtual std::string toString() const SAL_OVERRIDE;
     virtual RTFValue* Clone();
+    virtual RTFValue* CloneWithSprms(RTFSprms const& rAttributes, RTFSprms const& rSprms);
     RTFSprms& getAttributes();
     RTFSprms& getSprms();
     RTFShape& getShape() const;
commit 6c0e1270889deb513f961f864dfc1c02ee8705f4
Author: Michael Stahl <mstahl at redhat.com>
Date:   Fri Jun 13 23:49:59 2014 +0200

    fdo#70578: writerfilter RTF import: by default style is para style 0
    
    quoth the spec:
    
    "For <style>, both <styledef> and <stylename> are optional; the default
    is paragraph style 0."
    
    Of course in order to do that we need to add support for at least
    recognizing the \dsN and \tsN keywords to override the default, so that
    table styles don't become paragraph styles.
    
    Change-Id: Ic100768581f9e8c327063ff776fbd61ac4242483

diff --git a/sw/qa/extras/rtfimport/data/fdo70578.rtf b/sw/qa/extras/rtfimport/data/fdo70578.rtf
new file mode 100644
index 0000000..b2a4ea8
--- /dev/null
+++ b/sw/qa/extras/rtfimport/data/fdo70578.rtf
@@ -0,0 +1,11 @@
+{\rtf1\ansi
+{\fonttbl
+{\f30\fswiss\fcharset0\fprq2{\*\panose 020b0706040902060204}Haettenschweiler;}
+}
+{\stylesheet
+{\ql \li0\ri0\widctlpar\aspalpha\aspnum\faauto\adjustright\rin0\lin0\itap0 \fs20\lang1033\langfe1033\cgrid\langnp1033\langfenp1033 \snext0 Normal;}
+{\s16\qc \li0\ri0\widctlpar\aspalpha\aspnum\faauto\adjustright\rin0\lin0\itap0 
+\f30\fs44\lang1033\langfe1033\cgrid\langnp1033\langfenp1033 \sbasedon0 \snext16 Subtitle;}
+}
+\par
+}
diff --git a/sw/qa/extras/rtfimport/rtfimport.cxx b/sw/qa/extras/rtfimport/rtfimport.cxx
index 2eba05a..3699db8 100644
--- a/sw/qa/extras/rtfimport/rtfimport.cxx
+++ b/sw/qa/extras/rtfimport/rtfimport.cxx
@@ -1143,6 +1143,17 @@ DECLARE_RTFIMPORT_TEST(testFdo62044, "fdo62044.rtf")
     CPPUNIT_ASSERT_EQUAL(10.f, getProperty<float>(xPropertySet, "CharHeight")); // Was 18, i.e. reset back to original value.
 }
 
+DECLARE_RTFIMPORT_TEST(testFdo70578, "fdo70578.rtf")
+{
+    // Style without explicit \s0 was not imported as the default style
+    uno::Reference<beans::XPropertySet> xPropertySet(
+        getStyles("ParagraphStyles")->getByName("Subtitle"), uno::UNO_QUERY);
+    uno::Reference<style::XStyle> xStyle(xPropertySet, uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(OUString("Standard"), xStyle->getParentStyle());
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(0), getProperty<sal_Int32>(xPropertySet, "ParaTopMargin"));
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(0), getProperty<sal_Int32>(xPropertySet, "ParaBottomMargin"));
+}
+
 DECLARE_RTFIMPORT_TEST(testPoshPosv, "posh-posv.rtf")
 {
     CPPUNIT_ASSERT_EQUAL(text::HoriOrientation::CENTER, getProperty<sal_Int16>(getShape(1), "HoriOrient"));
diff --git a/writerfilter/source/rtftok/rtfdocumentimpl.cxx b/writerfilter/source/rtftok/rtfdocumentimpl.cxx
index 0f29107..2d2dad7 100644
--- a/writerfilter/source/rtftok/rtfdocumentimpl.cxx
+++ b/writerfilter/source/rtftok/rtfdocumentimpl.cxx
@@ -3609,6 +3609,23 @@ int RTFDocumentImpl::dispatchValue(RTFKeyword nKeyword, int nParam)
                 m_aStates.top().aCharacterSprms.set(NS_ooxml::LN_EG_RPrBase_rStyle, RTFValue::Pointer_t(new RTFValue(aName)));
         }
         break;
+    case RTF_DS:
+        if (m_aStates.top().nDestinationState == DESTINATION_STYLESHEET || m_aStates.top().nDestinationState == DESTINATION_STYLEENTRY)
+        {
+            m_nCurrentStyleIndex = nParam;
+            RTFValue::Pointer_t pValue(new RTFValue(-42)); // TODO no value in enum StyleType?
+            m_aStates.top().aTableAttributes.set(
+                    NS_ooxml::LN_CT_Style_type, pValue); // section style
+        }
+        break;
+    case RTF_TS:
+        if (m_aStates.top().nDestinationState == DESTINATION_STYLESHEET || m_aStates.top().nDestinationState == DESTINATION_STYLEENTRY)
+        {
+            m_nCurrentStyleIndex = nParam;
+            RTFValue::Pointer_t pValue(new RTFValue(-43)); // FIXME the correct value would be 3 but maybe table styles mess things up in dmapper, be cautious and disable them for now
+            m_aStates.top().aTableAttributes.set(NS_ooxml::LN_CT_Style_type, pValue); // table style
+        }
+        break;
     case RTF_DEFF:
         m_nDefaultFontIndex = nParam;
         break;
@@ -4651,6 +4668,13 @@ int RTFDocumentImpl::pushState()
         break;
     case DESTINATION_STYLESHEET:
         m_aStates.top().nDestinationState = DESTINATION_STYLEENTRY;
+        {
+            // the *default* is \s0 i.e. paragraph style default
+            // this will be overwritten by \sN \csN \dsN \tsN
+            m_nCurrentStyleIndex = 0;
+            RTFValue::Pointer_t pValue(new RTFValue(1));
+            m_aStates.top().aTableAttributes.set(NS_ooxml::LN_CT_Style_type, pValue);
+        }
         break;
     case DESTINATION_FIELDRESULT:
     case DESTINATION_SHAPETEXT:


More information about the Libreoffice-commits mailing list