sw/qa/extras/ooxmlexport/data/tdf125038.docx |binary
sw/qa/extras/ooxmlexport/data/tdf125038b.docx |binary
sw/qa/extras/ooxmlexport/data/tdf125038c.docx |binary
sw/qa/extras/ooxmlexport/ooxmlexport14.cxx | 42 +++++
writerfilter/source/dmapper/DomainMapper_Impl.cxx | 165 ++++++++++++++++++++--
writerfilter/source/dmapper/DomainMapper_Impl.hxx | 13 +
6 files changed, 205 insertions(+), 15 deletions(-)
New commits:
commit 58d798ea44b9e2a57681e990a8acc747bc287c0b
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Mon Oct 28 16:58:41 2019 +0100
Commit: Tamás Zolnai <tamas.zol...@collabora.com>
CommitDate: Tue Nov 5 15:12:44 2019 +0100
tdf#125038 DOCX import: fix various issues with MERGEFIELD inside IF fields
This is a combination of 4 commits.
This is the 1st commit:
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.
(cherry picked from commit 7b0534cb70e96028c8525285c42a71415704cede)
Conflicts:
writerfilter/source/dmapper/DomainMapper_Impl.hxx
This is commit #2:
Related: tdf#125038 DOCX import: fix unexpected linebreak inside IF
condition
Writer body text is expected to only contain the result of the field. So
in case both the field command and the field result contains a
linebreak, we need to make sure that linebreaks are ignored in the field
command for field types where the Writer field implementation expects a
single string.
With this, the number of paragraphs in the bugdoc is now correct.
(cherry picked from commit 97f9af714ea1c46e498fa99f7ca34fc1708d38a6)
This is commit #3:
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.
(cherry picked from commit d09336fbdceaafd9320466b660a2b32a07dcc16a)
This is commit #4:
tdf#125038 DOCX import: better support for linebreaks in IF fields
IF fields can't contain linebreaks, so instead of just calling
finishParagraph() and hoping it does something sane, explicitly handle
them: remember the properties and perform the call only once the field
is closed.
(cherry picked from commit d40c2be38aaf56116f4dc7be9e78f4e9695407fc)
Change-Id: I676aa2c83f12cb600829177a0eb25558822b1d94
Reviewed-on: https://gerrit.libreoffice.org/81982
Tested-by: Jenkins
Reviewed-by: Tamás Zolnai <tamas.zol...@collabora.com>
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/data/tdf125038b.docx
b/sw/qa/extras/ooxmlexport/data/tdf125038b.docx
new file mode 100644
index 000000000000..3aa189daded8
Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/tdf125038b.docx
differ
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 b06b342c7071..27317edd1615 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx
@@ -39,6 +39,48 @@ 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);
+}
+
+DECLARE_OOXMLIMPORT_TEST(testTdf125038b, "tdf125038b.docx")
+{
+ // Load a document with an IF field, where the IF field command contains a
paragraph break.
+ uno::Reference<text::XTextDocument> xTextDocument(mxComponent,
uno::UNO_QUERY);
+ uno::Reference<container::XEnumerationAccess>
xParagraphAccess(xTextDocument->getText(), uno::UNO_QUERY);
+ uno::Reference<container::XEnumeration> xParagraphs =
xParagraphAccess->createEnumeration();
+ CPPUNIT_ASSERT(xParagraphs->hasMoreElements());
+ uno::Reference<text::XTextRange> xParagraph(xParagraphs->nextElement(),
uno::UNO_QUERY);
+
+ // Without the accompanying fix in place, this test would have failed with:
+ // - Expected: phone: 1234
+ // - Actual :
+ // i.e. the the first paragraph was empty and the second paragraph had the
content.
+ CPPUNIT_ASSERT_EQUAL(OUString("phone: 1234"), xParagraph->getString());
+ CPPUNIT_ASSERT(xParagraphs->hasMoreElements());
+ xParagraphs->nextElement();
+
+ // Without the accompanying fix in place, this test would have failed with:
+ // - Expression: !xParagraphs->hasMoreElements()
+ // i.e. the document had 3 paragraphs, while only 2 was expected.
+ 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: t...@test.test
+ // - Actual : email:
+ // I.e. the result of the MERGEFIELD field inside an IF field was lost.
+ CPPUNIT_ASSERT_EQUAL(OUString("email: t...@test.test"), aActual);
+}
CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index a32991c6a4cc..f6086a249fe0 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -171,6 +171,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 )
@@ -627,7 +673,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 )
@@ -1178,6 +1224,32 @@ void DomainMapper_Impl::finishParagraph( const
PropertyMapPtr& pPropertyMap, con
{
if (m_bDiscardHeaderFooter)
return;
+
+ if (!m_aFieldStack.empty())
+ {
+ FieldContextPtr pFieldContext = m_aFieldStack.back();
+ if (pFieldContext && !pFieldContext->IsCommandCompleted())
+ {
+ std::vector<OUString> aCommandParts =
pFieldContext->GetCommandParts();
+ if (!aCommandParts.empty() && aCommandParts[0] == "IF")
+ {
+ // Conditional text field conditions don't support linebreaks
in Writer.
+ return;
+ }
+ }
+
+ if (pFieldContext && pFieldContext->IsCommandCompleted())
+ {
+ if (pFieldContext->GetFieldId() == FIELD_IF)
+ {
+ // Conditional text fields can't contain newlines, finish the
paragraph later.
+ FieldParagraph aFinish{pPropertyMap, bRemove};
+ pFieldContext->GetParagraphsToFinish().push_back(aFinish);
+ return;
+ }
+ }
+ }
+
#ifdef DBG_UTIL
TagLogger::getInstance().startElement("finishParagraph");
#endif
@@ -3157,14 +3229,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
@@ -3178,7 +3250,7 @@ bool DomainMapper_Impl::IsOpenField() const
void DomainMapper_Impl::SetFieldLocked()
{
if (IsOpenField())
- m_aFieldStack.top()->SetFieldLocked();
+ m_aFieldStack.back()->SetFieldLocked();
}
HeaderFooterContext::HeaderFooterContext(bool bTextInserted)
@@ -3272,7 +3344,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() )
{
@@ -4155,7 +4227,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() )
{
@@ -4215,8 +4287,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;
@@ -4825,7 +4909,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;
}
@@ -4919,22 +5004,46 @@ 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() )
{
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;
}
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())
{
+ 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);
}
}
@@ -4969,8 +5078,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();
@@ -5092,7 +5217,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);
@@ -5115,7 +5240,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() )
{
@@ -5191,7 +5316,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( ) );
}
@@ -5271,10 +5396,22 @@ void DomainMapper_Impl::PopFieldContext()
}
//TOCs have to include all the imported content
+ }
+ std::vector<FieldParagraph> aParagraphsToFinish;
+ if (pContext)
+ {
+ aParagraphsToFinish = pContext->GetParagraphsToFinish();
}
+
//remove the field context
- m_aFieldStack.pop();
+ m_aFieldStack.pop_back();
+
+ // Finish the paragraph(s) now that the field is closed.
+ for (const auto& rFinish : aParagraphsToFinish)
+ {
+ finishParagraph(rFinish.m_pPropertyMap, rFinish.m_bRemove);
+ }
}
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.hxx
b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
index 3e8a2236da92..17728959e9f7 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.hxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
@@ -130,6 +130,13 @@ public:
bool getTextInserted();
};
+/// Information about a paragraph to be finished after a field end.
+struct FieldParagraph
+{
+ PropertyMapPtr m_pPropertyMap;
+ bool m_bRemove = false;
+};
+
/// field stack element
class FieldContext : public virtual SvRefBase
{
@@ -156,6 +163,8 @@ class FieldContext : public virtual SvRefBase
/// (Character) properties of the field itself.
PropertyMapPtr m_pProperties;
+ std::vector<FieldParagraph> m_aParagraphsToFinish;
+
public:
explicit FieldContext(css::uno::Reference<css::text::XTextRange> const&
xStart);
~FieldContext() override;
@@ -203,6 +212,8 @@ public:
const PropertyMapPtr& getProperties() { return m_pProperties; }
::std::vector<OUString> GetCommandParts() const;
+
+ std::vector<FieldParagraph>& GetParagraphsToFinish() { return
m_aParagraphsToFinish; }
};
struct TextAppendContext
@@ -417,7 +428,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_bSetUserFieldContent;
bool
m_bSetCitation;
bool
m_bSetDateValue;
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
