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

Miklos Vajna (via logerrit) logerrit at kemper.freedesktop.org
Mon Oct 28 17:08:09 UTC 2019


 sw/qa/extras/ooxmlexport/data/tdf125038.docx      |binary
 sw/qa/extras/ooxmlexport/ooxmlexport14.cxx        |    9 ++++
 writerfilter/source/dmapper/DomainMapper_Impl.cxx |   45 +++++++++++++++-------
 writerfilter/source/dmapper/DomainMapper_Impl.hxx |    2 
 4 files changed, 41 insertions(+), 15 deletions(-)

New commits:
commit 7b0534cb70e96028c8525285c42a71415704cede
Author:     Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Mon Oct 28 16:58:41 2019 +0100
Commit:     Miklos Vajna <vmiklos at collabora.com>
CommitDate: Mon Oct 28 18:07:21 2019 +0100

    Related: tdf#125038 DOCX import: fix unexpected MERGEFIELD result inside IF
    
    The problem is that DOCX supports nesting MERGEFIELD fields inside IF
    fields, while SwHiddenTextField only supports a single string as a
    condition.
    
    This means in case there are MERGEFIELD fields inside the IF field,
    those fields will be inserted to the doc model before the IF field,
    exposing their value, while Word only uses their value during the
    evaluation of the IF expression.
    
    Fix the problem by inspecting the parent field command before setting
    the MERGEFIELD result.
    
    Change-Id: Ieca098f16f756bab5d23f219fa4ca30d077d4bb7
    Reviewed-on: https://gerrit.libreoffice.org/81615
    Reviewed-by: Miklos Vajna <vmiklos at collabora.com>
    Tested-by: Jenkins

diff --git a/sw/qa/extras/ooxmlexport/data/tdf125038.docx b/sw/qa/extras/ooxmlexport/data/tdf125038.docx
new file mode 100644
index 000000000000..b4dd622f95e0
Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/tdf125038.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx
index b06b342c7071..6dd91d44960a 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx
@@ -39,6 +39,15 @@ DECLARE_OOXMLEXPORT_TEST(testTdf108350_noFontdefaults, "tdf108350_noFontdefaults
     //CPPUNIT_ASSERT_EQUAL_MESSAGE("Font size", 10.f, getProperty<float>(xStyleProps, "CharHeight"));
 }
 
+DECLARE_OOXMLIMPORT_TEST(testTdf125038, "tdf125038.docx")
+{
+    OUString aActual = getParagraph(1)->getString();
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: phone:...
+    // - Actual  : result1result2phone:...
+    // i.e. the result if the inner MERGEFIELD fields ended up in the body text.
+    CPPUNIT_ASSERT_EQUAL(OUString("phone: \t1234567890"), aActual);
+}
 
 CPPUNIT_PLUGIN_IMPLEMENT();
 
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index 76aca3880e00..31971d21c2c5 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -668,7 +668,7 @@ uno::Reference< text::XTextAppend > const &  DomainMapper_Impl::GetTopTextAppend
 FieldContextPtr const &  DomainMapper_Impl::GetTopFieldContext()
 {
     SAL_WARN_IF(m_aFieldStack.empty(), "writerfilter.dmapper", "Field stack is empty");
-    return m_aFieldStack.top();
+    return m_aFieldStack.back();
 }
 
 void DomainMapper_Impl::InitTabStopFromStyle( const uno::Sequence< style::TabStop >& rInitTabStops )
@@ -3355,14 +3355,14 @@ void DomainMapper_Impl::PushFieldContext()
     uno::Reference< text::XTextRange > xStart;
     if (xCrsr.is())
         xStart = xCrsr->getStart();
-    m_aFieldStack.push( new FieldContext( xStart ) );
+    m_aFieldStack.push_back(new FieldContext(xStart));
 }
 /*-------------------------------------------------------------------------
 //the current field context waits for the completion of the command
   -----------------------------------------------------------------------*/
 bool DomainMapper_Impl::IsOpenFieldCommand() const
 {
-    return !m_aFieldStack.empty() && !m_aFieldStack.top()->IsCommandCompleted();
+    return !m_aFieldStack.empty() && !m_aFieldStack.back()->IsCommandCompleted();
 }
 /*-------------------------------------------------------------------------
 //the current field context waits for the completion of the command
@@ -3376,7 +3376,7 @@ bool DomainMapper_Impl::IsOpenField() const
 void DomainMapper_Impl::SetFieldLocked()
 {
     if (IsOpenField())
-        m_aFieldStack.top()->SetFieldLocked();
+        m_aFieldStack.back()->SetFieldLocked();
 }
 
 HeaderFooterContext::HeaderFooterContext(bool bTextInserted)
@@ -3470,7 +3470,7 @@ void DomainMapper_Impl::AppendFieldCommand(OUString const & rPartOfCommand)
     TagLogger::getInstance().endElement();
 #endif
 
-    FieldContextPtr pContext = m_aFieldStack.top();
+    FieldContextPtr pContext = m_aFieldStack.back();
     OSL_ENSURE( pContext.get(), "no field context available");
     if( pContext.get() )
     {
@@ -4388,7 +4388,7 @@ void DomainMapper_Impl::CloseFieldCommand()
 
     FieldContextPtr pContext;
     if(!m_aFieldStack.empty())
-        pContext = m_aFieldStack.top();
+        pContext = m_aFieldStack.back();
     OSL_ENSURE( pContext.get(), "no field context available");
     if( pContext.get() )
     {
@@ -5064,7 +5064,8 @@ void DomainMapper_Impl::CloseFieldCommand()
                         }
                         uno::Reference< text::XTextContent > xToInsert( xTC, uno::UNO_QUERY );
 
-                        uno::Sequence<beans::PropertyValue> aValues = m_aFieldStack.top()->getProperties()->GetPropertyValues();
+                        uno::Sequence<beans::PropertyValue> aValues
+                            = m_aFieldStack.back()->getProperties()->GetPropertyValues();
                         appendTextContent(xToInsert, aValues);
                         m_bSetCitation = true;
                     }
@@ -5152,7 +5153,7 @@ bool DomainMapper_Impl::IsFieldResultAsString()
 {
     bool bRet = false;
     OSL_ENSURE( !m_aFieldStack.empty(), "field stack empty?");
-    FieldContextPtr pContext = m_aFieldStack.top();
+    FieldContextPtr pContext = m_aFieldStack.back();
     OSL_ENSURE( pContext.get(), "no field context available");
     if( pContext.get() )
     {
@@ -5164,7 +5165,7 @@ bool DomainMapper_Impl::IsFieldResultAsString()
 void DomainMapper_Impl::AppendFieldResult(OUString const& rString)
 {
     assert(!m_aFieldStack.empty());
-    FieldContextPtr pContext = m_aFieldStack.top();
+    FieldContextPtr pContext = m_aFieldStack.back();
     SAL_WARN_IF(!pContext.get(), "writerfilter.dmapper", "no field context");
     if (pContext.get())
     {
@@ -5202,8 +5203,24 @@ void DomainMapper_Impl::SetFieldResult(OUString const& rResult)
     TagLogger::getInstance().chars(rResult);
 #endif
 
-    FieldContextPtr pContext = m_aFieldStack.top();
+    FieldContextPtr pContext = m_aFieldStack.back();
     OSL_ENSURE( pContext.get(), "no field context available");
+
+    if (m_aFieldStack.size() > 1)
+    {
+        // This is a nested field. See if the parent supports nesting on the Writer side.
+        FieldContextPtr pParentContext = m_aFieldStack[m_aFieldStack.size() - 2];
+        if (pParentContext)
+        {
+            std::vector<OUString> aParentParts = pParentContext->GetCommandParts();
+            // Conditional text fields don't support nesting in Writer.
+            if (!aParentParts.empty() && aParentParts[0] == "IF")
+            {
+                return;
+            }
+        }
+    }
+
     if( pContext.get() )
     {
         uno::Reference<text::XTextField> xTextField = pContext->GetTextField();
@@ -5324,7 +5341,7 @@ void DomainMapper_Impl::SetFieldFFData(const FFDataHandler::Pointer_t& pFFDataHa
 
     if (!m_aFieldStack.empty())
     {
-        FieldContextPtr pContext = m_aFieldStack.top();
+        FieldContextPtr pContext = m_aFieldStack.back();
         if (pContext.get())
         {
             pContext->setFFDataHandler(pFFDataHandler);
@@ -5347,7 +5364,7 @@ void DomainMapper_Impl::PopFieldContext()
     if (m_aFieldStack.empty())
         return;
 
-    FieldContextPtr pContext = m_aFieldStack.top();
+    FieldContextPtr pContext = m_aFieldStack.back();
     OSL_ENSURE( pContext.get(), "no field context available");
     if( pContext.get() )
     {
@@ -5423,7 +5440,7 @@ void DomainMapper_Impl::PopFieldContext()
                         // e.g. SdtEndBefore.
                         if (m_pLastCharacterContext.get())
                             aMap.InsertProps(m_pLastCharacterContext);
-                        aMap.InsertProps(m_aFieldStack.top()->getProperties());
+                        aMap.InsertProps(m_aFieldStack.back()->getProperties());
                         appendTextContent(xToInsert, aMap.GetPropertyValues());
                         CheckRedline( xToInsert->getAnchor( ) );
                     }
@@ -5511,7 +5528,7 @@ void DomainMapper_Impl::PopFieldContext()
 
     }
     //remove the field context
-    m_aFieldStack.pop();
+    m_aFieldStack.pop_back();
 }
 
 
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.hxx b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
index c11ef87202ab..7dbd7032c4fa 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.hxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
@@ -417,7 +417,7 @@ private:
     std::stack<TextAppendContext>                                                   m_aTextAppendStack;
     std::stack<AnchoredContext>                                                     m_aAnchoredStack;
     std::stack<HeaderFooterContext>                                                 m_aHeaderFooterStack;
-    std::stack<FieldContextPtr>                                                     m_aFieldStack;
+    std::deque<FieldContextPtr> m_aFieldStack;
     bool m_bForceGenericFields;
     bool                                                                            m_bSetUserFieldContent;
     bool                                                                            m_bSetCitation;


More information about the Libreoffice-commits mailing list