sw/qa/extras/layout/data/sdt+framePr.docx |binary sw/qa/extras/layout/layout3.cxx | 36 +++++++++++++++++++++++++++ writerfilter/source/dmapper/DomainMapper.cxx | 18 ++++++++++--- writerfilter/source/dmapper/SdtHelper.hxx | 1 4 files changed, 51 insertions(+), 4 deletions(-)
New commits: commit ccad5b30548072249b8d8abf1c43de8350271f54 Author: Mike Kaganski <mike.kagan...@collabora.com> AuthorDate: Mon Jan 22 19:10:29 2024 +0600 Commit: Xisco Fauli <xiscofa...@libreoffice.org> CommitDate: Tue Jan 23 09:47:15 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> 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/layout3.cxx b/sw/qa/extras/layout/layout3.cxx index 35c5cf669175..9c46d8395a1e 100644 --- a/sw/qa/extras/layout/layout3.cxx +++ b/sw/qa/extras/layout/layout3.cxx @@ -2267,6 +2267,42 @@ CPPUNIT_TEST_FIXTURE(SwLayoutWriter3, testTdf159271) assertXPath(pXmlDoc, "/root/page/body/tab/row/cell[2]/txt//SwFieldPortion"_ostr, 1); } +CPPUNIT_TEST_FIXTURE(SwLayoutWriter3, 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"_ostr, 1); + // Without the fix, this would fail: there were two paragraphs + assertXPath(pXmlDoc, "/root/page/body/txt"_ostr, 1); + assertXPath(pXmlDoc, "/root/page/body/txt/SwParaPortion"_ostr, 1); + assertXPath(pXmlDoc, "/root/page/body/txt/SwParaPortion/SwLineLayout"_ostr, 1); + // Without the fix, this would fail: there was a field portion in the line + assertXPath(pXmlDoc, "/root/page/body/txt/SwParaPortion/SwLineLayout/SwFieldPortion"_ostr, 0); + // Without the fix, this would fail: there was no anchored objects + assertXPath(pXmlDoc, "/root/page/body/txt/anchored"_ostr, 1); + assertXPath(pXmlDoc, "/root/page/body/txt/anchored/fly"_ostr, 1); + + const sal_Int32 paraRight + = getXPath(pXmlDoc, "/root/page/body/txt/infos/bounds"_ostr, "right"_ostr).toInt32(); + const sal_Int32 paraHeight + = getXPath(pXmlDoc, "/root/page/body/txt/infos/bounds"_ostr, "height"_ostr).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"_ostr, "right"_ostr) + .toInt32(); + const sal_Int32 flyHeight + = getXPath(pXmlDoc, "/root/page/body/txt/anchored/fly/infos/bounds"_ostr, "height"_ostr) + .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 6bb5071a4dd6..903954de033e 100644 --- a/writerfilter/source/dmapper/DomainMapper.cxx +++ b/writerfilter/source/dmapper/DomainMapper.cxx @@ -4479,14 +4479,24 @@ void DomainMapper::lcl_utext(const sal_Unicode *const 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;