From d477fa8ac1b0d3ee81427217bbb5950278ab16db Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Fri, 17 Mar 2023 14:00:17 +0100 Subject: [PATCH] sw floattable: unconditionally map to SwFormatFlySplit CppunitTest_sw_ooxmlexport8's testN816593 failed in the SW_FORCE_FLY_SPLIT=1 case because the re-import of the document merged two tables into one. The problem starts earlier, we didn't import the table with a as a floating table, but we should. Fix the problem by never going via m_aPendingFloatingTables in the SW_FORCE_FLY_SPLIT=1 case, since that was only a workaround for layout limitations. This conditionally reverts commit 78d1f1c2835b9fae0f91ed771fc1d594c7817502 (fdo#68607 bnc#816593 DomainMapperTableHandler: don't always start a frame, 2013-09-03). Also add a SwModelTestBase::FlySplitGuard, so it's just a one-liner change to test the SW_FORCE_FLY_SPLIT=1 case from cppunit. The goal is to have this on by default once it's mature enough. Change-Id: I9d94a49f7a0c27dd43e8fd388867c65d6d25f2e5 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/149044 Reviewed-by: Miklos Vajna Tested-by: Jenkins --- sw/Library_swqahelper.mk | 4 + sw/qa/core/layout/flycnt.cxx | 125 ++---------------- sw/qa/extras/ooxmlexport/ooxmlexport8.cxx | 1 + sw/qa/inc/swmodeltestbase.hxx | 7 + sw/qa/unit/swmodeltestbase.cxx | 20 +++ .../dmapper/DomainMapperTableHandler.cxx | 3 +- 6 files changed, 45 insertions(+), 115 deletions(-) diff --git a/sw/Library_swqahelper.mk b/sw/Library_swqahelper.mk index d1d696ce4e98..ec379d979658 100644 --- a/sw/Library_swqahelper.mk +++ b/sw/Library_swqahelper.mk @@ -26,6 +26,10 @@ $(eval $(call gb_Library_use_externals,swqahelper, \ libxml2 \ )) +$(eval $(call gb_Library_use_custom_headers,swqahelper,\ + officecfg/registry \ +)) + $(eval $(call gb_Library_add_defs,swqahelper,\ -DSWQAHELPER_DLLIMPLEMENTATION \ )) diff --git a/sw/qa/core/layout/flycnt.cxx b/sw/qa/core/layout/flycnt.cxx index a9da83353ecd..528b3bc4cbc8 100644 --- a/sw/qa/core/layout/flycnt.cxx +++ b/sw/qa/core/layout/flycnt.cxx @@ -9,10 +9,6 @@ #include -#include -#include -#include - #include #include #include @@ -45,16 +41,7 @@ public: CPPUNIT_TEST_FIXTURE(Test, testSplitFlyWithTable) { // Given a document with a multi-page floating table: - std::shared_ptr pChanges( - comphelper::ConfigurationChanges::create()); - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set(true, - pChanges); - pChanges->commit(); - comphelper::ScopeGuard g([pChanges] { - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set( - false, pChanges); - pChanges->commit(); - }); + SwModelTestBase::FlySplitGuard aGuard; createSwDoc("floattable.docx"); // When laying out that document: @@ -103,16 +90,7 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyWithTable) CPPUNIT_TEST_FIXTURE(Test, testSplitFlyVertOffset) { // Given a document with a floattable, split on 2 pages and a positive vertical offset: - std::shared_ptr pChanges( - comphelper::ConfigurationChanges::create()); - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set(true, - pChanges); - pChanges->commit(); - comphelper::ScopeGuard g([pChanges] { - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set( - false, pChanges); - pChanges->commit(); - }); + SwModelTestBase::FlySplitGuard aGuard; createSwDoc("floattable-vertoffset.docx"); // When laying out that document: @@ -156,16 +134,7 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyVertOffset) CPPUNIT_TEST_FIXTURE(Test, testSplitFly3Pages) { // Given a document with a floattable, split on 3 pages: - std::shared_ptr pChanges( - comphelper::ConfigurationChanges::create()); - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set(true, - pChanges); - pChanges->commit(); - comphelper::ScopeGuard g([pChanges] { - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set( - false, pChanges); - pChanges->commit(); - }); + SwModelTestBase::FlySplitGuard aGuard; createSwDoc("floattable-3pages.docx"); // When laying out that document: @@ -221,16 +190,7 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFly3Pages) CPPUNIT_TEST_FIXTURE(Test, testSplitFlyRow) { // Given a document with a floattable, single row split on 2 pages: - std::shared_ptr pChanges( - comphelper::ConfigurationChanges::create()); - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set(true, - pChanges); - pChanges->commit(); - comphelper::ScopeGuard g([pChanges] { - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set( - false, pChanges); - pChanges->commit(); - }); + SwModelTestBase::FlySplitGuard aGuard; createSwDoc("floattable-rowsplit.docx"); // When laying out that document: @@ -317,16 +277,7 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyEnable) CPPUNIT_TEST_FIXTURE(Test, testSplitFlyFooter) { // Given a document with a floattable, table split on 2 pages with headers/footers: - std::shared_ptr pChanges( - comphelper::ConfigurationChanges::create()); - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set(true, - pChanges); - pChanges->commit(); - comphelper::ScopeGuard g([pChanges] { - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set( - false, pChanges); - pChanges->commit(); - }); + SwModelTestBase::FlySplitGuard aGuard; createSwDoc("floattable-footer.docx"); // When laying out that document: @@ -365,16 +316,7 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyFooter) CPPUNIT_TEST_FIXTURE(Test, testSplitFlyFooter2Rows) { // Given a document with a 2nd page that contains the second half of a split row + a last row: - std::shared_ptr pChanges( - comphelper::ConfigurationChanges::create()); - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set(true, - pChanges); - pChanges->commit(); - comphelper::ScopeGuard g([pChanges] { - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set( - false, pChanges); - pChanges->commit(); - }); + SwModelTestBase::FlySplitGuard aGuard; createSwDoc("floattable-footer-2rows.docx"); // When laying out that document: @@ -395,16 +337,7 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyFooter2Rows) CPPUNIT_TEST_FIXTURE(Test, testSplitFly2Cols) { // Given a document with a 2nd page that contains the second half of a split row and 2 columns: - std::shared_ptr pChanges( - comphelper::ConfigurationChanges::create()); - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set(true, - pChanges); - pChanges->commit(); - comphelper::ScopeGuard g([pChanges] { - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set( - false, pChanges); - pChanges->commit(); - }); + SwModelTestBase::FlySplitGuard aGuard; createSwDoc("floattable-2cols.docx"); // When laying out that document: @@ -425,16 +358,7 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFly2Cols) CPPUNIT_TEST_FIXTURE(Test, testSplitFlyWidow) { // Given a document with a 2nd page that contains 2 lines, due to widow control: - std::shared_ptr pChanges( - comphelper::ConfigurationChanges::create()); - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set(true, - pChanges); - pChanges->commit(); - comphelper::ScopeGuard g([pChanges] { - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set( - false, pChanges); - pChanges->commit(); - }); + SwModelTestBase::FlySplitGuard aGuard; createSwDoc("floattable-widow.docx"); // When laying out that document: @@ -480,16 +404,7 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyWidow) CPPUNIT_TEST_FIXTURE(Test, testSplitFlyCompat14) { // Given a Word 2010 document with 2 pages, one table row each: - std::shared_ptr pChanges( - comphelper::ConfigurationChanges::create()); - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set(true, - pChanges); - pChanges->commit(); - comphelper::ScopeGuard g([pChanges] { - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set( - false, pChanges); - pChanges->commit(); - }); + SwModelTestBase::FlySplitGuard aGuard; createSwDoc("floattable-compat14.docx"); // When laying out that document: @@ -528,16 +443,7 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyCompat14) CPPUNIT_TEST_FIXTURE(Test, testSplitFlyCompat14Nosplit) { // Given a Word 2010 document with 2 pages, 2 rows on page 1, 1 row on page 2: - std::shared_ptr pChanges( - comphelper::ConfigurationChanges::create()); - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set(true, - pChanges); - pChanges->commit(); - comphelper::ScopeGuard g([pChanges] { - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set( - false, pChanges); - pChanges->commit(); - }); + SwModelTestBase::FlySplitGuard aGuard; createSwDoc("floattable-compat14-nosplit.docx"); // When laying out that document: @@ -570,16 +476,7 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyCompat14Nosplit) CPPUNIT_TEST_FIXTURE(Test, testSplitFlyCompat14Body) { // Given a Word 2010 document with 2 pages, 1 row on page 1, 1 row on page 2: - std::shared_ptr pChanges( - comphelper::ConfigurationChanges::create()); - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set(true, - pChanges); - pChanges->commit(); - comphelper::ScopeGuard g([pChanges] { - officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set( - false, pChanges); - pChanges->commit(); - }); + SwModelTestBase::FlySplitGuard aGuard; createSwDoc("floattable-compat14-body.docx"); // When laying out that document: diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport8.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport8.cxx index 5bd2c3164fc8..8cd6d4961f38 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport8.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport8.cxx @@ -990,6 +990,7 @@ DECLARE_OOXMLEXPORT_TEST(testPageBorderShadow, "page-border-shadow.docx") DECLARE_OOXMLEXPORT_TEST(testN816593, "n816593.docx") { + SwModelTestBase::FlySplitGuard aGuard; // Two consecutive without any paragraph in between, but with different tblpPr. In this // case we need to have 2 different tables instead of 1 uno::Reference xTablesSupplier(mxComponent, uno::UNO_QUERY); diff --git a/sw/qa/inc/swmodeltestbase.hxx b/sw/qa/inc/swmodeltestbase.hxx index 6ef965723fd4..380b59040a16 100644 --- a/sw/qa/inc/swmodeltestbase.hxx +++ b/sw/qa/inc/swmodeltestbase.hxx @@ -107,6 +107,13 @@ protected: void paste(std::u16string_view aFilename, css::uno::Reference const& xTextRange); public: + /// Temporarily enables DOCX::ImportFloatingTableAsSplitFly. + class SWQAHELPER_DLLPUBLIC FlySplitGuard + { + public: + FlySplitGuard(); + ~FlySplitGuard(); + }; SwModelTestBase(const OUString& pTestDocumentPath = OUString(), const char* pFilter = ""); protected: diff --git a/sw/qa/unit/swmodeltestbase.cxx b/sw/qa/unit/swmodeltestbase.cxx index 035866b68838..823148151377 100644 --- a/sw/qa/unit/swmodeltestbase.cxx +++ b/sw/qa/unit/swmodeltestbase.cxx @@ -23,6 +23,8 @@ #include #include #include +#include +#include #include #include @@ -34,6 +36,24 @@ using namespace css; +SwModelTestBase::FlySplitGuard::FlySplitGuard() +{ + std::shared_ptr pChanges( + comphelper::ConfigurationChanges::create()); + officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set(true, + pChanges); + pChanges->commit(); +} + +SwModelTestBase::FlySplitGuard::~FlySplitGuard() +{ + std::shared_ptr pChanges( + comphelper::ConfigurationChanges::create()); + officecfg::Office::Writer::Filter::Import::DOCX::ImportFloatingTableAsSplitFly::set(false, + pChanges); + pChanges->commit(); +} + void SwModelTestBase::paste(std::u16string_view aFilename, uno::Reference const& xTextRange) { diff --git a/writerfilter/source/dmapper/DomainMapperTableHandler.cxx b/writerfilter/source/dmapper/DomainMapperTableHandler.cxx index c879091f01ca..968e3c0835a7 100644 --- a/writerfilter/source/dmapper/DomainMapperTableHandler.cxx +++ b/writerfilter/source/dmapper/DomainMapperTableHandler.cxx @@ -1604,7 +1604,8 @@ void DomainMapperTableHandler::endTable(unsigned int nestedTableLevel, bool bTab m_aTableProperties->getValue(TablePropertyMap::TABLE_WIDTH, nTableWidth); sal_Int32 nTableWidthType = text::SizeType::FIX; m_aTableProperties->getValue(TablePropertyMap::TABLE_WIDTH_TYPE, nTableWidthType); - if (m_rDMapper_Impl.GetSectionContext() && nestedTableLevel <= 1 && !m_rDMapper_Impl.IsInHeaderFooter()) + // Split flys don't need to go via m_aPendingFloatingTables. + if (m_rDMapper_Impl.GetSectionContext() && nestedTableLevel <= 1 && !m_rDMapper_Impl.IsInHeaderFooter() && !IsFlySplitAllowed()) { m_rDMapper_Impl.m_aPendingFloatingTables.emplace_back(xStart, xEnd, comphelper::containerToSequence(aFrameProperties),