sw/qa/extras/layout/data/sdt+framePr.docx |binary
sw/qa/extras/layout/layout2.cxx | 34 +++++++++++++++++++++++++++
writerfilter/source/dmapper/DomainMapper.cxx | 18 +++++++++++---
writerfilter/source/dmapper/SdtHelper.hxx | 1
4 files changed, 49 insertions(+), 4 deletions(-)
New commits:
commit 122b086e4f388df817ba46763e97fe4c7210b229
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Mon Jan 22 19:10:29 2024 +0600
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Thu Jan 25 08:40:45 2024 +0100
tdf#159259: make sure to set FieldStartRange in sdt helper
... also for field case.
Unfortunately, it is not really clear, if the anagement of this could
be moved to DomainMapper_Impl. There are several insertion contexts;
they may use different insertion points; there maybe could be cases
when an inserted content should not go into the current sdt (?). Thus,
I didn't put the code into DomainMapper_Impl::appendTextContent(),
where the field character is inserted. Instead, I added code to check
and set the start range in the same place as for the normal text: we
know for sure, that if it were a normal text, we would append it to
GetCurrentTextRange()->getEnd() - so do the same for fields.
My concern that still stays is that the use of hasUnusedText looks
hackish and fragile. Inserted fields don't set it - so the code that
depends on empty sdt will not notice it. OTOH, it can't set it: this
would break inserting field result for an already inserted command.
Possibly all handling of sdt should be refactored at some point.
Change-Id: I7a783aab2400d9a9c1f9f2e5607c872cb58d346b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162398
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162427
Reviewed-by: Xisco Fauli <xiscofa...@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162506
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
diff --git a/sw/qa/extras/layout/data/sdt+framePr.docx
b/sw/qa/extras/layout/data/sdt+framePr.docx
new file mode 100644
index 000000000000..d46bcbfaa774
Binary files /dev/null and b/sw/qa/extras/layout/data/sdt+framePr.docx differ
diff --git a/sw/qa/extras/layout/layout2.cxx b/sw/qa/extras/layout/layout2.cxx
index 27072238dc5c..adb9e7a4276f 100644
--- a/sw/qa/extras/layout/layout2.cxx
+++ b/sw/qa/extras/layout/layout2.cxx
@@ -2918,6 +2918,40 @@ CPPUNIT_TEST_FIXTURE(SwLayoutWriter2, testTdf156725)
"/root/page[2]/body/txt/anchored/fly/column[2]/body/section/column[2]/body/txt",
1);
}
+CPPUNIT_TEST_FIXTURE(SwLayoutWriter2, testTdf159259)
+{
+ // Given a document with a block sdt with a single field, having framePr
aligned to right
+ createSwDoc("sdt+framePr.docx");
+ xmlDocUniquePtr pXmlDoc = parseLayoutDump();
+ // Make sure there is only one page and one paragraph with one line and
one anchored object
+ assertXPath(pXmlDoc, "/root/page", 1);
+ // Without the fix, this would fail: there were two paragraphs
+ assertXPath(pXmlDoc, "/root/page/body/txt", 1);
+ assertXPath(pXmlDoc, "/root/page/body/txt/SwParaPortion", 1);
+ assertXPath(pXmlDoc, "/root/page/body/txt/SwParaPortion/SwLineLayout", 1);
+ // Without the fix, this would fail: there was a field portion in the line
+ assertXPath(pXmlDoc,
"/root/page/body/txt/SwParaPortion/SwLineLayout/SwFieldPortion", 0);
+ // Without the fix, this would fail: there was no anchored objects
+ assertXPath(pXmlDoc, "/root/page/body/txt/anchored", 1);
+ assertXPath(pXmlDoc, "/root/page/body/txt/anchored/fly", 1);
+
+ const sal_Int32 paraRight
+ = getXPath(pXmlDoc, "/root/page/body/txt/infos/bounds",
"right").toInt32();
+ const sal_Int32 paraHeight
+ = getXPath(pXmlDoc, "/root/page/body/txt/infos/bounds",
"height").toInt32();
+
+ CPPUNIT_ASSERT_GREATER(sal_Int32(0), paraRight);
+ CPPUNIT_ASSERT_GREATER(sal_Int32(0), paraHeight);
+
+ const sal_Int32 flyRight
+ = getXPath(pXmlDoc, "/root/page/body/txt/anchored/fly/infos/bounds",
"right").toInt32();
+ const sal_Int32 flyHeight
+ = getXPath(pXmlDoc, "/root/page/body/txt/anchored/fly/infos/bounds",
"height").toInt32();
+
+ CPPUNIT_ASSERT_EQUAL(paraRight, flyRight); // The fly is right-aligned
+ CPPUNIT_ASSERT_EQUAL(paraHeight, flyHeight);
+}
+
CPPUNIT_PLUGIN_IMPLEMENT();
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/writerfilter/source/dmapper/DomainMapper.cxx
b/writerfilter/source/dmapper/DomainMapper.cxx
index 6813736dc4c5..c2d7eb73ba64 100644
--- a/writerfilter/source/dmapper/DomainMapper.cxx
+++ b/writerfilter/source/dmapper/DomainMapper.cxx
@@ -4362,14 +4362,24 @@ void DomainMapper::lcl_utext(const sal_uInt8 * data_,
size_t len)
}
else if (m_pImpl->IsOpenFieldCommand() &&
!m_pImpl->IsForceGenericFields())
{
- if (bInSdtBlockText && m_pImpl->m_pSdtHelper->hasUnusedText())
- m_pImpl->m_pSdtHelper->createPlainTextControl();
+ if (bInSdtBlockText)
+ {
+ if (m_pImpl->m_pSdtHelper->hasUnusedText())
+ m_pImpl->m_pSdtHelper->createPlainTextControl();
+ else if (!m_pImpl->m_pSdtHelper->isFieldStartRangeSet())
+
m_pImpl->m_pSdtHelper->setFieldStartRange(GetCurrentTextRange()->getEnd());
+ }
m_pImpl->AppendFieldCommand(sText);
}
else if( m_pImpl->IsOpenField() &&
m_pImpl->IsFieldResultAsString())
{
- if (bInSdtBlockText && m_pImpl->m_pSdtHelper->hasUnusedText())
- m_pImpl->m_pSdtHelper->createPlainTextControl();
+ if (bInSdtBlockText)
+ {
+ if (m_pImpl->m_pSdtHelper->hasUnusedText())
+ m_pImpl->m_pSdtHelper->createPlainTextControl();
+ else if (!m_pImpl->m_pSdtHelper->isFieldStartRangeSet())
+
m_pImpl->m_pSdtHelper->setFieldStartRange(GetCurrentTextRange()->getEnd());
+ }
/*depending on the success of the field insert operation this
result will be
set at the field or directly inserted into the text*/
m_pImpl->AppendFieldResult(sText);
diff --git a/writerfilter/source/dmapper/SdtHelper.hxx
b/writerfilter/source/dmapper/SdtHelper.hxx
index 5db799bd1fd2..85b95a48818b 100644
--- a/writerfilter/source/dmapper/SdtHelper.hxx
+++ b/writerfilter/source/dmapper/SdtHelper.hxx
@@ -171,6 +171,7 @@ public:
void setDataBindingStoreItemID(const OUString& sValue) {
m_sDataBindingStoreItemID = sValue; }
const OUString& GetDataBindingStoreItemID() const { return
m_sDataBindingStoreItemID; }
+ bool isFieldStartRangeSet() const { return m_xFieldStartRange.is(); }
void setFieldStartRange(const css::uno::Reference<css::text::XTextRange>&
xStartRange)
{
m_xFieldStartRange = xStartRange;
