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

[Miklos Vajna (via logerrit)]

 sw/qa/extras/ooxmlexport/data/tdf125038c.docx     |binary
 sw/qa/extras/ooxmlexport/ooxmlexport14.cxx        |   10 ++
 writerfilter/source/dmapper/DomainMapper_Impl.cxx |   82 ++++++++++++++++++++++
 3 files changed, 92 insertions(+)

New commits:
commit d09336fbdceaafd9320466b660a2b32a07dcc16a
Author:     Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Thu Oct 31 10:40:41 2019 +0100
Commit:     Miklos Vajna <vmiklos at collabora.com>
CommitDate: Thu Oct 31 12:35:56 2019 +0100

    tdf#125038 DOCX import: fix lost MERGEFIELD result inside an IF field
    
    The problem here was that the IF field result didn't have a plain text
    string, rather it had a MERGEFIELD in it. Writer's conditional text
    field expects a plain text string, so just use the result of the
    MERGEFIELD for an IF parent. Do this in a generic way, it's likely that
    other parent-child field combinations want to do the same in the future.
    
    With this, all lost strings are fixed from the original bugdoc + all
    unexpected content is hidden in Writer as well.
    
    Change-Id: Ic5c03b1df2f08a2cd851647b625e0c303cc5d6c5
    Reviewed-on: https://gerrit.libreoffice.org/81825
    Reviewed-by: Miklos Vajna <vmiklos at collabora.com>
    Tested-by: Jenkins

diff --git a/sw/qa/extras/ooxmlexport/data/tdf125038c.docx b/sw/qa/extras/ooxmlexport/data/tdf125038c.docx
new file mode 100644
index 000000000000..10234b864627
Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/tdf125038c.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx
index 82237bffa368..2c2d94821acf 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx
@@ -66,6 +66,16 @@ DECLARE_OOXMLIMPORT_TEST(testTdf125038b, "tdf125038b.docx")
     CPPUNIT_ASSERT(!xParagraphs->hasMoreElements());
 }
 
+DECLARE_OOXMLIMPORT_TEST(testTdf125038c, "tdf125038c.docx")
+{
+    OUString aActual = getParagraph(1)->getString();
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: email: test at test.test
+    // - Actual  : email:
+    // I.e. the result of the MERGEFIELD field inside an IF field was lost.
+    CPPUNIT_ASSERT_EQUAL(OUString("email: test at test.test"), aActual);
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index 36f17c5cebb6..7d2b7545de20 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -174,6 +174,52 @@ struct FieldConversion
 
 typedef std::unordered_map<OUString, FieldConversion> FieldConversionMap_t;
 
+/// Gives access to the parent field contenxt of the topmost one, if there is any.
+static FieldContextPtr GetParentFieldContext(const std::deque<FieldContextPtr>& rFieldStack)
+{
+    if (rFieldStack.size() < 2)
+    {
+        return nullptr;
+    }
+
+    return rFieldStack[rFieldStack.size() - 2];
+}
+
+/// Decides if the pInner field inside pOuter is allowed in Writer core, depending on their type.
+static bool IsFieldNestingAllowed(const FieldContextPtr& pOuter, const FieldContextPtr& pInner)
+{
+    if (!pOuter->GetFieldId())
+    {
+        return true;
+    }
+
+    if (!pInner->GetFieldId())
+    {
+        return true;
+    }
+
+    switch (pOuter->GetFieldId().get())
+    {
+        case FIELD_IF:
+        {
+            switch (pInner->GetFieldId().get())
+            {
+                case FIELD_MERGEFIELD:
+                {
+                    return false;
+                }
+                default:
+                    break;
+            }
+            break;
+        }
+        default:
+            break;
+    }
+
+    return true;
+}
+
 uno::Any FloatingTableInfo::getPropertyValue(const OUString &propertyName)
 {
     for( beans::PropertyValue const & propVal : m_aFrameProperties )
@@ -4490,8 +4536,20 @@ void DomainMapper_Impl::CloseFieldCommand()
                     break;
                 }
                 default:
+                {
+                    FieldContextPtr pOuter = GetParentFieldContext(m_aFieldStack);
+                    if (pOuter)
+                    {
+                        if (!IsFieldNestingAllowed(pOuter, m_aFieldStack.back()))
+                        {
+                            // Parent field can't host this child field: don't create a child field
+                            // in this case.
+                            bCreateField = false;
+                        }
+                    }
                     break;
                 }
+                }
                 if (m_bStartTOC && (aIt->second.eFieldId == FIELD_PAGEREF) )
                 {
                     bCreateField = false;
@@ -5202,6 +5260,19 @@ bool DomainMapper_Impl::IsFieldResultAsString()
     {
         bRet = pContext->GetTextField().is() || pContext->GetFieldId() == FIELD_FORMDROPDOWN;
     }
+
+    if (!bRet)
+    {
+        FieldContextPtr pOuter = GetParentFieldContext(m_aFieldStack);
+        if (pOuter)
+        {
+            if (!IsFieldNestingAllowed(pOuter, m_aFieldStack.back()))
+            {
+                // Child field has no backing SwField, but the parent has: append is still possible.
+                bRet = pOuter->GetTextField().is();
+            }
+        }
+    }
     return bRet;
 }
 
@@ -5212,6 +5283,17 @@ void DomainMapper_Impl::AppendFieldResult(OUString const& rString)
     SAL_WARN_IF(!pContext.get(), "writerfilter.dmapper", "no field context");
     if (pContext.get())
     {
+        FieldContextPtr pOuter = GetParentFieldContext(m_aFieldStack);
+        if (pOuter)
+        {
+            if (!IsFieldNestingAllowed(pOuter, pContext))
+            {
+                // Child can't host the field result, forward to parent.
+                pOuter->AppendResult(rString);
+                return;
+            }
+        }
+
         pContext->AppendResult(rString);
     }
 }