From a86a2a1c1ceb7203857d4317913c5b1bb9feb4aa Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Wed, 8 Jan 2020 15:40:32 +0100 Subject: DOCX import: fix lost page break when footer ends with a table Regression from commit 7d3778e0ef9f54f3c8988f1b84d58e7002d6c625 (bnc#816593 DOCX import: ignore page breaks in tables, 2013-09-02), the page break was ignored because the preceding footer ended with a table (no empty paragraph at the end of the footer stream). Fix the problem by saving/loading the table state around header/footers, that way the page break is not ignored. Adjust testTdf102466 to test the page number from Word. Change-Id: Ia4c22452ee2c37f7f941dfd922db04c851644d0c Reviewed-on: https://gerrit.libreoffice.org/c/core/+/86435 Reviewed-by: Miklos Vajna Tested-by: Jenkins --- sw/qa/extras/ooxmlexport/ooxmlexport4.cxx | 4 +- writerfilter/CppunitTest_writerfilter_dmapper.mk | 1 + .../qa/cppunittests/dmapper/DomainMapper_Impl.cxx | 85 +++++++++++++++++++++ .../dmapper/data/page-break-footer-table.docx | Bin 0 -> 15416 bytes writerfilter/source/dmapper/DomainMapper_Impl.cxx | 9 ++- writerfilter/source/dmapper/DomainMapper_Impl.hxx | 5 +- 6 files changed, 99 insertions(+), 5 deletions(-) create mode 100644 writerfilter/qa/cppunittests/dmapper/DomainMapper_Impl.cxx create mode 100644 writerfilter/qa/cppunittests/dmapper/data/page-break-footer-table.docx diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx index 6c0cfcf7cf7b..abadb9765aa1 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx @@ -997,8 +997,8 @@ DECLARE_OOXMLEXPORT_TEST(testTdf102466, "tdf102466.docx") CPPUNIT_ASSERT_MESSAGE("The table is clipped in a fly frame.", nFlyPrtHeight >= nTableHeight); } - // check how much pages we have - CPPUNIT_ASSERT_EQUAL(10, getPages()); + // check how much pages we have: it should match the Word layout result + CPPUNIT_ASSERT_EQUAL(11, getPages()); // check content of the first page { diff --git a/writerfilter/CppunitTest_writerfilter_dmapper.mk b/writerfilter/CppunitTest_writerfilter_dmapper.mk index 9805e28228a0..41b92e9bdb3e 100644 --- a/writerfilter/CppunitTest_writerfilter_dmapper.mk +++ b/writerfilter/CppunitTest_writerfilter_dmapper.mk @@ -18,6 +18,7 @@ $(eval $(call gb_CppunitTest_use_externals,writerfilter_dmapper,\ $(eval $(call gb_CppunitTest_add_exception_objects,writerfilter_dmapper, \ writerfilter/qa/cppunittests/dmapper/CellColorHandler \ writerfilter/qa/cppunittests/dmapper/DomainMapperTableHandler \ + writerfilter/qa/cppunittests/dmapper/DomainMapper_Impl \ )) $(eval $(call gb_CppunitTest_use_libraries,writerfilter_dmapper, \ diff --git a/writerfilter/qa/cppunittests/dmapper/DomainMapper_Impl.cxx b/writerfilter/qa/cppunittests/dmapper/DomainMapper_Impl.cxx new file mode 100644 index 000000000000..00c83c913d96 --- /dev/null +++ b/writerfilter/qa/cppunittests/dmapper/DomainMapper_Impl.cxx @@ -0,0 +1,85 @@ +/* -*- 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 +#include + +#include +#include +#include +#include +#include +#include + +#include + +using namespace ::com::sun::star; + +namespace +{ +/// Tests for writerfilter/source/dmapper/DomainMapper_Impl.cxx. +class Test : public test::BootstrapFixture, public unotest::MacrosTest +{ +private: + uno::Reference mxComponentContext; + uno::Reference mxComponent; + +public: + void setUp() override; + void tearDown() override; + uno::Reference& getComponent() { return mxComponent; } +}; + +void Test::setUp() +{ + test::BootstrapFixture::setUp(); + + mxComponentContext.set(comphelper::getComponentContext(getMultiServiceFactory())); + mxDesktop.set(frame::Desktop::create(mxComponentContext)); +} + +void Test::tearDown() +{ + if (mxComponent.is()) + mxComponent->dispose(); + + test::BootstrapFixture::tearDown(); +} + +char const DATA_DIRECTORY[] = "/writerfilter/qa/cppunittests/dmapper/data/"; + +CPPUNIT_TEST_FIXTURE(Test, testPageBreakFooterTable) +{ + // Load a document which refers to a footer which ends with a table, and there is a page break + // in the body text right after the footer reference. + OUString aURL = m_directories.getURLFromSrc(DATA_DIRECTORY) + "page-break-footer-table.docx"; + getComponent() = loadFromDesktop(aURL); + + // Check the last paragraph. + uno::Reference xTextDocument(getComponent(), uno::UNO_QUERY); + uno::Reference xParaEnumAccess(xTextDocument->getText(), + uno::UNO_QUERY); + uno::Reference xParaEnum = xParaEnumAccess->createEnumeration(); + uno::Reference xPara; + while (xParaEnum->hasMoreElements()) + { + xPara.set(xParaEnum->nextElement(), uno::UNO_QUERY); + } + style::BreakType eType = style::BreakType_NONE; + xPara->getPropertyValue("BreakType") >>= eType; + + // Without the accompanying fix in place, this test would have failed with: + // - Expected: 4 + // - Actual : 0 + // i.e. there was no page break before the last paragraph. + CPPUNIT_ASSERT_EQUAL(style::BreakType_PAGE_BEFORE, eType); +} +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/writerfilter/qa/cppunittests/dmapper/data/page-break-footer-table.docx b/writerfilter/qa/cppunittests/dmapper/data/page-break-footer-table.docx new file mode 100644 index 000000000000..376a1fb1e483 Binary files /dev/null and b/writerfilter/qa/cppunittests/dmapper/data/page-break-footer-table.docx differ diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx index 7842acb4a9b0..7c4cc46dba31 100644 --- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx +++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx @@ -2171,8 +2171,9 @@ void DomainMapper_Impl::appendGlossaryEntry() void DomainMapper_Impl::PushPageHeaderFooter(bool bHeader, SectionPropertyMap::PageType eType) { - m_aHeaderFooterStack.push(HeaderFooterContext(m_bTextInserted)); + m_aHeaderFooterStack.push(HeaderFooterContext(m_bTextInserted, m_nTableDepth)); m_bTextInserted = false; + m_nTableDepth = 0; const PropertyIds ePropIsOn = bHeader? PROP_HEADER_IS_ON: PROP_FOOTER_IS_ON; const PropertyIds ePropShared = bHeader? PROP_HEADER_IS_SHARED: PROP_FOOTER_IS_SHARED; @@ -2261,6 +2262,7 @@ void DomainMapper_Impl::PopPageHeaderFooter() if (!m_aHeaderFooterStack.empty()) { m_bTextInserted = m_aHeaderFooterStack.top().getTextInserted(); + m_nTableDepth = m_aHeaderFooterStack.top().getTableDepth(); m_aHeaderFooterStack.pop(); } } @@ -3566,8 +3568,9 @@ void DomainMapper_Impl::SetFieldLocked() m_aFieldStack.back()->SetFieldLocked(); } -HeaderFooterContext::HeaderFooterContext(bool bTextInserted) +HeaderFooterContext::HeaderFooterContext(bool bTextInserted, sal_Int32 nTableDepth) : m_bTextInserted(bTextInserted) + , m_nTableDepth(nTableDepth) { } @@ -3576,6 +3579,8 @@ bool HeaderFooterContext::getTextInserted() const return m_bTextInserted; } +sal_Int32 HeaderFooterContext::getTableDepth() const { return m_nTableDepth; } + FieldContext::FieldContext(uno::Reference< text::XTextRange > const& xStart) : m_bFieldCommandCompleted(false) , m_xStartRange( xStart ) diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.hxx b/writerfilter/source/dmapper/DomainMapper_Impl.hxx index b050e95b66c5..2691bbf659d6 100644 --- a/writerfilter/source/dmapper/DomainMapper_Impl.hxx +++ b/writerfilter/source/dmapper/DomainMapper_Impl.hxx @@ -140,9 +140,12 @@ enum class SkipFootnoteSeparator class HeaderFooterContext { bool const m_bTextInserted; + sal_Int32 m_nTableDepth; + public: - explicit HeaderFooterContext(bool bTextInserted); + explicit HeaderFooterContext(bool bTextInserted, sal_Int32 nTableDepth); bool getTextInserted() const; + sal_Int32 getTableDepth() const; }; /// Information about a paragraph to be finished after a field end. -- cgit v1.2.3