writerfilter/CppunitTest_writerfilter_rtftok.mk | 1 writerfilter/qa/cppunittests/rtftok/data/floattable-then-sect-break.rtf | 12 ++++ writerfilter/qa/cppunittests/rtftok/rtfdispatchsymbol.cxx | 26 ++++++++-- writerfilter/source/rtftok/rtfdispatchsymbol.cxx | 13 ++++- 4 files changed, 48 insertions(+), 4 deletions(-)
New commits: commit ab51c235672dd6da2feafbe3f26625ee14889bf9 Author: Miklos Vajna <[email protected]> AuthorDate: Wed Feb 28 09:41:06 2024 +0100 Commit: Michael Stahl <[email protected]> CommitDate: Thu Feb 29 11:05:11 2024 +0100 Related: tdf#158986 sw floattable, RTF import: use more setNeedPar() See <https://gerrit.libreoffice.org/c/core/+/163844/1#message-ea0bfde78fa24ad83e5c153ecaddbf897a89f547>, this keeps the bug fixed but is a better version, as pointed out by Michael S: > there was a bug where dispatching PAR here caused a deferred page > break to be lost, which was fixed by calling setNeedPar(true) instead. (cherry picked from commit c98ff922831f56253af2a050b8e07cfc89b7a387) Change-Id: Ibe6e4c14286d40d3066ce9cb7fac9f6847fb81dd Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164095 Tested-by: Jenkins Reviewed-by: Michael Stahl <[email protected]> diff --git a/writerfilter/source/rtftok/rtfdispatchsymbol.cxx b/writerfilter/source/rtftok/rtfdispatchsymbol.cxx index 9d10c7a03362..b40fd55dde9b 100644 --- a/writerfilter/source/rtftok/rtfdispatchsymbol.cxx +++ b/writerfilter/source/rtftok/rtfdispatchsymbol.cxx @@ -140,18 +140,19 @@ RTFError RTFDocumentImpl::dispatchSymbol(RTFKeyword nKeyword) } else { - if (m_bNeedCr) - { // tdf#158586 don't dispatch \par here, it eats deferred page breaks - setNeedPar(true); - } - + bool bPendingFloatingTable = false; RTFValue::Pointer_t pTblpPr = m_aStates.top().getTableRowSprms().find(NS_ooxml::LN_CT_TblPrBase_tblpPr); if (pTblpPr) { // We have a pending floating table, provide an anchor for it still in this // section. - dispatchSymbol(RTFKeyword::PAR); + bPendingFloatingTable = true; + } + + if (m_bNeedCr || bPendingFloatingTable) + { // tdf#158586 don't dispatch \par here, it eats deferred page breaks + setNeedPar(true); } sectBreak(); commit 0ed1604de9448e0898a6894d9a8262272aea4766 Author: Miklos Vajna <[email protected]> AuthorDate: Mon Feb 26 08:17:18 2024 +0100 Commit: Michael Stahl <[email protected]> CommitDate: Thu Feb 29 11:05:00 2024 +0100 tdf#158986 sw floattable: fix RTF import of table followed by \sect The bugdoc had a floating table, immediately followed by a section break. The floating table went to the second page, should be on the first page. The trouble is that RTF's section break is just a special symbol, so we can have a section break right after a floating table. This is in constrast with DOCX where a non-last section break is a paragraph property, so it's guaranteed to have at least a paragraph start after a floating table and before a section break, which can nicely serve as an anchor point for the floating table. Fix the problem similar to what the OOXML tokenizer did in a similar case in commit 01ad8ec4bb5425446e95dbada81de435646824b4 (sw floattable: fix lost tables around a floating table from DOCX, 2023-06-05), by injecting a paragraph before the section break. Handling this at a tokenizer level seems to be the right place, since the DOCX version of the same document was already imported OK. Change-Id: Ic945c472c08ba872a5c46e2b8f75e919678aa0a0 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163929 Reviewed-by: Miklos Vajna <[email protected]> Tested-by: Jenkins (cherry picked from commit b7c4c4d45f44a26283678f3dc32982b3a728c614) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163844 Reviewed-by: Michael Stahl <[email protected]> diff --git a/writerfilter/CppunitTest_writerfilter_rtftok.mk b/writerfilter/CppunitTest_writerfilter_rtftok.mk index 2c91cbb8cb2f..d0a1e37bc16c 100644 --- a/writerfilter/CppunitTest_writerfilter_rtftok.mk +++ b/writerfilter/CppunitTest_writerfilter_rtftok.mk @@ -13,6 +13,7 @@ $(eval $(call gb_CppunitTest_CppunitTest,writerfilter_rtftok)) $(eval $(call gb_CppunitTest_use_externals,writerfilter_rtftok,\ boost_headers \ + libxml2 \ )) $(eval $(call gb_CppunitTest_add_exception_objects,writerfilter_rtftok, \ diff --git a/writerfilter/qa/cppunittests/rtftok/data/floattable-then-sect-break.rtf b/writerfilter/qa/cppunittests/rtftok/data/floattable-then-sect-break.rtf new file mode 100644 index 000000000000..7ad179608c34 --- /dev/null +++ b/writerfilter/qa/cppunittests/rtftok/data/floattable-then-sect-break.rtf @@ -0,0 +1,12 @@ +{ tf1 +\paperw11907\paperh16840\margl896\margr2104\margt1440\margb720 +\pard\plain doc start\par + rowd pvpara phmrg posnegy-922 dfrmtxtLeft180 dfrmtxtRight180+\pard\intbl A1+ ow + rowd pvpara phmrg posnegy-922 dfrmtxtLeft180 dfrmtxtRight180+\pard\intbl A2+ ow +\pard\sect +\pard\plain doc end\par +} diff --git a/writerfilter/qa/cppunittests/rtftok/rtfdispatchsymbol.cxx b/writerfilter/qa/cppunittests/rtftok/rtfdispatchsymbol.cxx index 8317e50824b5..bf3ecc937ebc 100644 --- a/writerfilter/qa/cppunittests/rtftok/rtfdispatchsymbol.cxx +++ b/writerfilter/qa/cppunittests/rtftok/rtfdispatchsymbol.cxx @@ -7,22 +7,23 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -#include <test/unoapi_test.hxx> +#include <test/unoapixml_test.hxx> #include <com/sun/star/text/XTextDocument.hpp> #include <com/sun/star/beans/XPropertySet.hpp> #include <com/sun/star/style/ParagraphAdjust.hpp> +#include <com/sun/star/qa/XDumper.hpp> using namespace ::com::sun::star; namespace { /// Tests for writerfilter/source/rtftok/rtfdispatchsymbol.cxx. -class Test : public UnoApiTest +class Test : public UnoApiXmlTest { public: Test() - : UnoApiTest("/writerfilter/qa/cppunittests/rtftok/data/") + : UnoApiXmlTest("/writerfilter/qa/cppunittests/rtftok/data/") { } }; @@ -64,6 +65,25 @@ CPPUNIT_TEST_FIXTURE(Test, testCenterAfterPage) // i.e. the paragraph alignment on the second page was lost. CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int16>(style::ParagraphAdjust_CENTER), eActual); } + +CPPUNIT_TEST_FIXTURE(Test, testFloattableThenSectBreak) +{ + // Given a document with a floating table, immediately followed by \sect: + // When loading that file: + loadFromFile(u"floattable-then-sect-break.rtf"); + + // Then make sure that the floating table is on the first page: + uno::Reference<frame::XModel> xModel(mxComponent, uno::UNO_QUERY); + css::uno::Reference<qa::XDumper> xDumper(xModel->getCurrentController(), uno::UNO_QUERY); + OString aDump = xDumper->dump("layout").toUtf8(); + auto pCharBuffer = reinterpret_cast<const xmlChar*>(aDump.getStr()); + xmlDocUniquePtr pXmlDoc(xmlParseDoc(pCharBuffer)); + // Without the accompanying fix in place, this test would have failed with: + // - Expected: 1 + // - Actual : 0 + // i.e. the floating table was on the 2nd page, not on the 1st page. + assertXPath(pXmlDoc, "/root/page[1]/sorted_objs/fly"_ostr, 1); +} } /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/writerfilter/source/rtftok/rtfdispatchsymbol.cxx b/writerfilter/source/rtftok/rtfdispatchsymbol.cxx index 62ec8bf97e58..9d10c7a03362 100644 --- a/writerfilter/source/rtftok/rtfdispatchsymbol.cxx +++ b/writerfilter/source/rtftok/rtfdispatchsymbol.cxx @@ -144,6 +144,16 @@ RTFError RTFDocumentImpl::dispatchSymbol(RTFKeyword nKeyword) { // tdf#158586 don't dispatch \par here, it eats deferred page breaks setNeedPar(true); } + + RTFValue::Pointer_t pTblpPr + = m_aStates.top().getTableRowSprms().find(NS_ooxml::LN_CT_TblPrBase_tblpPr); + if (pTblpPr) + { + // We have a pending floating table, provide an anchor for it still in this + // section. + dispatchSymbol(RTFKeyword::PAR); + } + sectBreak(); if (m_nResetBreakOnSectBreak != RTFKeyword::invalid) {
