writerfilter/qa/cppunittests/dmapper/DomainMapperTableHandler.cxx | 38 +++++++++- writerfilter/qa/cppunittests/dmapper/data/floattable-header-overlap.docx |binary writerfilter/source/dmapper/DomainMapperTableHandler.cxx | 20 +++++ 3 files changed, 54 insertions(+), 4 deletions(-)
New commits: commit c50b5b648d157e940b17e36de7c529c4ceeafd84 Author: Miklos Vajna <[email protected]> AuthorDate: Tue Feb 13 08:09:23 2024 +0100 Commit: Xisco Fauli <[email protected]> CommitDate: Tue Feb 13 12:23:37 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. Change-Id: Ia3da65cd52d70b357e448a26a50ffb92a39795e6 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163290 Reviewed-by: Miklos Vajna <[email protected]> Tested-by: Jenkins (cherry picked from commit f74a6ef94ac957e4c146fc9923d30ce6bd31b5ce) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163286 Reviewed-by: Xisco Fauli <[email protected]> diff --git a/writerfilter/qa/cppunittests/dmapper/DomainMapperTableHandler.cxx b/writerfilter/qa/cppunittests/dmapper/DomainMapperTableHandler.cxx index b72b81913978..5924fb90bc2a 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,37 @@ 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: + loadFromFile(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"_ostr, "bottom"_ostr).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"_ostr, "top"_ostr).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::*"_ostr, + "type"_ostr, "PortionType::Fly"); + sal_Int32 nBodyFlyPortionHeight + = getXPath(pXmlDoc, "//page[1]/body/txt[1]/SwParaPortion/SwLineLayout[1]"_ostr, + "height"_ostr) + .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 e63d1cf9615f..0ecc6701551a 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> @@ -1585,8 +1586,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.
