sw/qa/extras/rtfexport/rtfexport5.cxx | 4 sw/qa/extras/rtfimport/rtfimport.cxx | 4 writerfilter/CppunitTest_writerfilter_rtftok.mk | 1 writerfilter/qa/cppunittests/rtftok/data/page.rtf | 6 + writerfilter/qa/cppunittests/rtftok/rtfdispatchsymbol.cxx | 72 ++++++++++++++ writerfilter/source/rtftok/rtfdispatchsymbol.cxx | 13 +- 6 files changed, 93 insertions(+), 7 deletions(-)
New commits: commit 3c610336a58f644525d5e4d2566c35eee6f7a618 Author: Miklos Vajna <[email protected]> AuthorDate: Thu Sep 8 20:14:27 2022 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Fri Sep 9 08:18:28 2022 +0200 tdf#148214 RTF import: avoid fake paragraph for \page when possible The bugdoc has 2 pages, an explicit page break between them and the first page has enough content that an additional fake paragraph at the end would lead to a 3rd, unwanted page. The fake paragraph was introduced in commit 7b58fc3dafc789aa55fff5ffef6ab83c3aa8b6e0 (fdo#48104 fix RTF import of \page in inner groups, 2012-04-02), because dmapper ignores more than 1 page breaks in a paragraph. Fix the problem by only inserting the fake paragraph in case the "page break before" paragraph property would have no effect (very first paragraph in the document) or we already sent characters (para props are lazy-sent on the first character). This keeps existing cases working, but avoids the unwanted fake paragraph in the bugdoc. I suspect the root cause is that dmapper doesn't handle multiple page breaks in a paragraph -- once that's fixed, this parBreak() call can be completely removed. Adjusted tests: - testImportHeaderFooter: this asserted the presence of fake paragraphs, which are now gone in the easy case, so this change is an improvement - testTdf133437: no visual difference before/after, probably the test intent was just "make sure this page has several shapes", the exact number isn't that interesting Change-Id: Idd2b8a70b4122eb08d9d305018d384dc0bbb276a Reviewed-on: https://gerrit.libreoffice.org/c/core/+/139704 Tested-by: Jenkins Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sw/qa/extras/rtfexport/rtfexport5.cxx b/sw/qa/extras/rtfexport/rtfexport5.cxx index 59390a6fb5e0..511c749a5aec 100644 --- a/sw/qa/extras/rtfexport/rtfexport5.cxx +++ b/sw/qa/extras/rtfexport/rtfexport5.cxx @@ -1233,7 +1233,7 @@ DECLARE_RTFEXPORT_TEST(testTdf133437, "tdf133437.rtf") xmlNodeSetPtr pXmlNodes = pXmlObj->nodesetval; sal_Int32 shapesOnPage = xmlXPathNodeSetGetLength(pXmlNodes); xmlXPathFreeObject(pXmlObj); - CPPUNIT_ASSERT_EQUAL(sal_Int32(120), shapesOnPage); + CPPUNIT_ASSERT_EQUAL(sal_Int32(118), shapesOnPage); } // Third page { @@ -1242,7 +1242,7 @@ DECLARE_RTFEXPORT_TEST(testTdf133437, "tdf133437.rtf") xmlNodeSetPtr pXmlNodes = pXmlObj->nodesetval; sal_Int32 shapesOnPage = xmlXPathNodeSetGetLength(pXmlNodes); xmlXPathFreeObject(pXmlObj); - CPPUNIT_ASSERT_EQUAL(sal_Int32(86), shapesOnPage); + CPPUNIT_ASSERT_EQUAL(sal_Int32(84), shapesOnPage); } } diff --git a/sw/qa/extras/rtfimport/rtfimport.cxx b/sw/qa/extras/rtfimport/rtfimport.cxx index e3e7f887a047..86a26ca0fa1d 100644 --- a/sw/qa/extras/rtfimport/rtfimport.cxx +++ b/sw/qa/extras/rtfimport/rtfimport.cxx @@ -1416,11 +1416,11 @@ CPPUNIT_TEST_FIXTURE(Test, testImportHeaderFooter) OUString value = paragraph->getString(); CPPUNIT_ASSERT_EQUAL(OUString("First Page"), value); - paragraph = getParagraph(4); + paragraph = getParagraph(3); value = paragraph->getString(); CPPUNIT_ASSERT_EQUAL(OUString("Second Page"), value); - paragraph = getParagraph(7); + paragraph = getParagraph(5); value = paragraph->getString(); CPPUNIT_ASSERT_EQUAL(OUString("Third Page"), value); diff --git a/writerfilter/CppunitTest_writerfilter_rtftok.mk b/writerfilter/CppunitTest_writerfilter_rtftok.mk index 0fe9083c6560..614f032c32db 100644 --- a/writerfilter/CppunitTest_writerfilter_rtftok.mk +++ b/writerfilter/CppunitTest_writerfilter_rtftok.mk @@ -16,6 +16,7 @@ $(eval $(call gb_CppunitTest_use_externals,writerfilter_rtftok,\ )) $(eval $(call gb_CppunitTest_add_exception_objects,writerfilter_rtftok, \ + writerfilter/qa/cppunittests/rtftok/rtfdispatchsymbol \ writerfilter/qa/cppunittests/rtftok/rtfdispatchvalue \ writerfilter/qa/cppunittests/rtftok/rtfdocumentimpl \ writerfilter/qa/cppunittests/rtftok/rtfsdrimport \ diff --git a/writerfilter/qa/cppunittests/rtftok/data/page.rtf b/writerfilter/qa/cppunittests/rtftok/data/page.rtf new file mode 100644 index 000000000000..75e1376388f2 --- /dev/null +++ b/writerfilter/qa/cppunittests/rtftok/data/page.rtf @@ -0,0 +1,6 @@ +{\rtf1 +\pard\plain +page 1\par +\page +page 2\par +} diff --git a/writerfilter/qa/cppunittests/rtftok/rtfdispatchsymbol.cxx b/writerfilter/qa/cppunittests/rtftok/rtfdispatchsymbol.cxx new file mode 100644 index 000000000000..5f4091688b16 --- /dev/null +++ b/writerfilter/qa/cppunittests/rtftok/rtfdispatchsymbol.cxx @@ -0,0 +1,72 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include <test/bootstrapfixture.hxx> +#include <unotest/macros_test.hxx> + +#include <com/sun/star/beans/XPropertySet.hpp> +#include <com/sun/star/frame/Desktop.hpp> +#include <com/sun/star/style/XStyleFamiliesSupplier.hpp> +#include <com/sun/star/table/BorderLine2.hpp> +#include <com/sun/star/text/XTextDocument.hpp> + +using namespace ::com::sun::star; + +namespace +{ +/// Tests for writerfilter/source/rtftok/rtfdispatchsymbol.cxx. +class Test : public test::BootstrapFixture, public unotest::MacrosTest +{ +private: + uno::Reference<lang::XComponent> mxComponent; + +public: + void setUp() override; + void tearDown() override; + uno::Reference<lang::XComponent>& getComponent() { return mxComponent; } +}; + +void Test::setUp() +{ + test::BootstrapFixture::setUp(); + + mxDesktop.set(frame::Desktop::create(mxComponentContext)); +} + +void Test::tearDown() +{ + if (mxComponent.is()) + mxComponent->dispose(); + + test::BootstrapFixture::tearDown(); +} + +constexpr OUStringLiteral DATA_DIRECTORY = u"/writerfilter/qa/cppunittests/rtftok/data/"; + +CPPUNIT_TEST_FIXTURE(Test, testPage) +{ + // Given a file with a \page and 2 \par tokens: + OUString aURL = m_directories.getURLFromSrc(DATA_DIRECTORY) + "page.rtf"; + + // When loading that file: + getComponent() = loadFromDesktop(aURL); + + // Then make sure we get exactly two paragraphs: + uno::Reference<text::XTextDocument> xTextDocument(getComponent(), uno::UNO_QUERY); + uno::Reference<container::XEnumerationAccess> xText(xTextDocument->getText(), uno::UNO_QUERY); + uno::Reference<container::XEnumeration> xParagraphs = xText->createEnumeration(); + xParagraphs->nextElement(); + xParagraphs->nextElement(); + // Without the accompanying fix in place, this test would have failed, the document had 3 + // paragraphs, not 2. + CPPUNIT_ASSERT(!xParagraphs->hasMoreElements()); +} +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/writerfilter/source/rtftok/rtfdispatchsymbol.cxx b/writerfilter/source/rtftok/rtfdispatchsymbol.cxx index 3220332d0b6b..3f9ed20bfae8 100644 --- a/writerfilter/source/rtftok/rtfdispatchsymbol.cxx +++ b/writerfilter/source/rtftok/rtfdispatchsymbol.cxx @@ -391,14 +391,21 @@ RTFError RTFDocumentImpl::dispatchSymbol(RTFKeyword nKeyword) } else { + bool bFirstRun = m_bFirstRun; checkFirstRun(); checkNeedPap(); sal_uInt8 const sBreak[] = { 0xc }; Mapper().text(sBreak, 1); - if (!m_bNeedPap) + if (bFirstRun || m_bNeedCr) { - parBreak(); - m_bNeedPap = true; + // If we don't have content in the document yet (so the break-before can't move + // to a second layout page) or we already have characters sent (so the paragraph + // properties are already finalized), then continue inserting a fake paragraph. + if (!m_bNeedPap) + { + parBreak(); + m_bNeedPap = true; + } } m_bNeedCr = true; }
