sw/qa/core/layout/tabfrm.cxx | 3 writerfilter/qa/cppunittests/dmapper/DomainMapperTableHandler.cxx | 36 +++++++++- writerfilter/qa/cppunittests/dmapper/data/floattable-header-overlap.docx |binary writerfilter/source/dmapper/DomainMapperTableHandler.cxx | 20 +++++ 4 files changed, 52 insertions(+), 7 deletions(-)
New commits: commit 97c3947b8acc8bf806cdc5ea48c7dc5f52ba2bbb Author: Miklos Vajna <[email protected]> AuthorDate: Tue Feb 13 08:09:23 2024 +0100 Commit: Caolán McNamara <[email protected]> CommitDate: Thu Feb 29 14:04:06 2024 +0100 tdf#159453 sw floattable: fix unexpected overlap of in-header fly and body text Regression from commit e2076cf7a92694bc94bdc9f3173c2bddbe881a89 (tdf#155682 sw floattable: fix DOCX with big pictures causes endless loop, 2023-10-25), the bugdoc's body text was wrapping around the floating table from the header, while the expectation was that the top of the body frame is below the bottom of the header frame. It seems IsFollowingTextFlow is only needed when the relation of the floating table is not "page", and this bugdoc has has an examplicit vertical relation of page. Solve the problem by limiting the IsFollowingTextFlow=true request for the floating table to the VertOrientRelation=page case, which fixes the bugdoc and keeps the old use-case working. The doc model for the new bugdoc now matches the WW8 import result. (cherry picked from commit f74a6ef94ac957e4c146fc9923d30ce6bd31b5ce) [ Backport note: testSplitFlyNestedRowSpan no longer asserts the page count on this branch, as that would be 7, the primary purpose of the test is to make sure we don't layout loop and even previously the loop control was kicking in, so the outcome is not too interesting. The page count is correct on co-24.04. ] Change-Id: Ia3da65cd52d70b357e448a26a50ffb92a39795e6 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164072 Tested-by: Caolán McNamara <[email protected]> Reviewed-by: Caolán McNamara <[email protected]> diff --git a/sw/qa/core/layout/tabfrm.cxx b/sw/qa/core/layout/tabfrm.cxx index 82d7b3b996d1..2acd8106d20e 100644 --- a/sw/qa/core/layout/tabfrm.cxx +++ b/sw/qa/core/layout/tabfrm.cxx @@ -109,9 +109,6 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyNestedRowSpan) // When loading that document: // Without the accompanying fix in place, this test would have resulted in a layout loop. createSwDoc("floattable-nested-rowspan.docx"); - - // Then make sure the resulting page count matches Word: - CPPUNIT_ASSERT_EQUAL(6, getPages()); } CPPUNIT_TEST_FIXTURE(Test, testSplitFlyTableJoin) diff --git a/writerfilter/qa/cppunittests/dmapper/DomainMapperTableHandler.cxx b/writerfilter/qa/cppunittests/dmapper/DomainMapperTableHandler.cxx index 5d17ac367ed6..ad4774703b04 100644 --- a/writerfilter/qa/cppunittests/dmapper/DomainMapperTableHandler.cxx +++ b/writerfilter/qa/cppunittests/dmapper/DomainMapperTableHandler.cxx @@ -7,7 +7,7 @@ * 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/beans/XPropertySet.hpp> #include <com/sun/star/table/BorderLine2.hpp> @@ -19,17 +19,18 @@ #include <com/sun/star/style/BreakType.hpp> #include <com/sun/star/text/XTextViewCursorSupplier.hpp> #include <com/sun/star/text/XPageCursor.hpp> +#include <com/sun/star/qa/XDumper.hpp> using namespace ::com::sun::star; namespace { /// Tests for writerfilter/source/dmapper/DomainMapperTableHandler.cxx. -class Test : public UnoApiTest +class Test : public UnoApiXmlTest { public: Test() - : UnoApiTest("/writerfilter/qa/cppunittests/dmapper/data/") + : UnoApiXmlTest("/writerfilter/qa/cppunittests/dmapper/data/") { } }; @@ -197,6 +198,35 @@ CPPUNIT_TEST_FIXTURE(Test, testDOCXFloatingTableFootnoteRedline) uno::Reference<drawing::XDrawPage> xDrawPage = xModel->getDrawPage(); CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0), xDrawPage->getCount()); } + +CPPUNIT_TEST_FIXTURE(Test, testDOCXFloatingTableHeaderBodyOverlap) +{ + // Given a document with a floating table in a header, the floating table extends the header + // frame: + // When importing that document: + loadFromURL(u"floattable-header-overlap.docx"); + + // Then make sure the fly bottom is less than the top of the body text: + 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)); + sal_Int32 nFlyBottom = getXPath(pXmlDoc, "//fly/infos/bounds", "bottom").toInt32(); + // Body text top is body top + height of the first line, that's just a fly portion (kind of a + // top margin). + sal_Int32 nBodyTop = getXPath(pXmlDoc, "//page[1]/body/txt[1]/infos/bounds", "top").toInt32(); + // Without the accompanying fix in place, this test would have failed, the first line was not a + // fly portion but it was actual text, above the floating table. + assertXPath(pXmlDoc, "//page[1]/body/txt[1]/SwParaPortion/SwLineLayout[1]/child::*", "type", + "PortionType::Fly"); + sal_Int32 nBodyFlyPortionHeight + = getXPath(pXmlDoc, "//page[1]/body/txt[1]/SwParaPortion/SwLineLayout[1]", "height") + .toInt32(); + sal_Int32 nBodyTextTop = nBodyTop + nBodyFlyPortionHeight; + // Fly bottom was 3063, body text top was 7148. + CPPUNIT_ASSERT_LESS(nBodyTextTop, nFlyBottom); +} } CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/writerfilter/qa/cppunittests/dmapper/data/floattable-header-overlap.docx b/writerfilter/qa/cppunittests/dmapper/data/floattable-header-overlap.docx new file mode 100644 index 000000000000..1230b9f4e07c Binary files /dev/null and b/writerfilter/qa/cppunittests/dmapper/data/floattable-header-overlap.docx differ diff --git a/writerfilter/source/dmapper/DomainMapperTableHandler.cxx b/writerfilter/source/dmapper/DomainMapperTableHandler.cxx index 9432e6dacd17..854c040fa975 100644 --- a/writerfilter/source/dmapper/DomainMapperTableHandler.cxx +++ b/writerfilter/source/dmapper/DomainMapperTableHandler.cxx @@ -39,6 +39,7 @@ #include <com/sun/star/text/WritingMode2.hpp> #include <com/sun/star/text/XTextField.hpp> #include <com/sun/star/text/XTextRangeCompare.hpp> +#include <com/sun/star/text/RelOrientation.hpp> #include <com/sun/star/beans/XPropertySet.hpp> #include <com/sun/star/beans/XPropertyState.hpp> #include <com/sun/star/container/XEnumeration.hpp> @@ -1588,8 +1589,25 @@ void DomainMapperTableHandler::endTable(unsigned int nestedTableLevel) // Floating tables inside a table always stay inside the cell. // Also extend the header/footer area if needed, so an in-header floating table // typically doesn't overlap with body test. + bool bIsFollowingTextFlow = true; + + sal_Int16 nVertOrientRelation{}; + auto it = std::find_if(aFrameProperties.begin(), aFrameProperties.end(), + [](const beans::PropertyValue& rPropertyValue) -> bool + { return rPropertyValue.Name == "VertOrientRelation"; }); + if (it != aFrameProperties.end()) + { + it->Value >>= nVertOrientRelation; + if (nVertOrientRelation == text::RelOrientation::PAGE_FRAME) + { + // If vertical relation is page, follow-text-flow is not useful and causes + // unwanted wrap of body text around in-header floating table, so avoid it. + bIsFollowingTextFlow = false; + } + } + aFrameProperties.push_back( - comphelper::makePropertyValue("IsFollowingTextFlow", true)); + comphelper::makePropertyValue("IsFollowingTextFlow", bIsFollowingTextFlow)); } // A text frame created for floating tables is always allowed to split.
