writerfilter/qa/cppunittests/dmapper/DomainMapperTableHandler.cxx | 25 ++++++++++ writerfilter/qa/cppunittests/dmapper/data/floattable-break-before.docx |binary writerfilter/source/dmapper/DomainMapperTableHandler.cxx | 20 +++++++- 3 files changed, 44 insertions(+), 1 deletion(-)
New commits: commit 4b6b9411e4ac912817dd804782ad2054bc0d1660 Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Thu Apr 27 08:15:51 2023 +0200 Commit: Miklos Vajna <vmik...@collabora.com> CommitDate: Thu Apr 27 12:21:30 2023 +0200 sw floattable, crashtesting: fix PDF export of fdo72790-1.docx, part 4 Converting the bugdoc to PDF crashed Writer layout since commit ce3308a926f036b87515b8cd97d2b197063dc77a (tdf#61594 sw floattable: import floating tables as split flys by default, 2023-04-12). Part 1 already fixed the crash and parts 2-3 already improved the layout partially, towards avoiding a layout loop. The top problem now seems to be that page breaks before floating tables are ignored, which leads to a layout situation that loops. This problem was hidden before, since page breaks were not ignored. Fix the problem at DOCX import time: if there is a "break before" on the table, then transfer that to the anchor paragraph, which gives the correct layout, and also side-steps the above described layout loop. We should probably never call SwTextFrame::JoinFrame() when creating the initial layout for a document, that part is still unfixed, but that looks like a pre-existing problem. Change-Id: I1e2ecdbf0a3d4e2477cd4768a9b4a35a155e815b Reviewed-on: https://gerrit.libreoffice.org/c/core/+/151082 Reviewed-by: Miklos Vajna <vmik...@collabora.com> Tested-by: Jenkins diff --git a/writerfilter/qa/cppunittests/dmapper/DomainMapperTableHandler.cxx b/writerfilter/qa/cppunittests/dmapper/DomainMapperTableHandler.cxx index 7f1a1db064d8..0650e48fe607 100644 --- a/writerfilter/qa/cppunittests/dmapper/DomainMapperTableHandler.cxx +++ b/writerfilter/qa/cppunittests/dmapper/DomainMapperTableHandler.cxx @@ -14,6 +14,8 @@ #include <com/sun/star/text/XTextTable.hpp> #include <com/sun/star/text/XTextTablesSupplier.hpp> #include <com/sun/star/drawing/XDrawPageSupplier.hpp> +#include <com/sun/star/text/XTextDocument.hpp> +#include <com/sun/star/style/BreakType.hpp> using namespace ::com::sun::star; @@ -57,6 +59,29 @@ CPPUNIT_TEST_FIXTURE(Test, testNestedFloatingTable) // was partly positioned outside the table cell, leading to overlapping text. CPPUNIT_ASSERT(bIsFollowingTextFlow); } + +CPPUNIT_TEST_FIXTURE(Test, testFloatingTableBreakBefore) +{ + // Given a 3 pages document: page break, then a multi-page floating table on pages 2 and 3: + // When laying out that document: + loadFromURL(u"floattable-break-before.docx"); + + // Then make sure the page break property is on the anchor of the floating table, otherwise it + // has no effect: + uno::Reference<text::XTextDocument> xTextDocument(mxComponent, uno::UNO_QUERY); + uno::Reference<container::XEnumerationAccess> xText(xTextDocument->getText(), uno::UNO_QUERY); + uno::Reference<container::XEnumeration> xParagraphs = xText->createEnumeration(); + xParagraphs->nextElement(); + xParagraphs->nextElement(); + uno::Reference<beans::XPropertySet> xParagraph(xParagraphs->nextElement(), uno::UNO_QUERY); + style::BreakType eBreakType{}; + xParagraph->getPropertyValue("BreakType") >>= eBreakType; + // Without the accompanying fix in place, this test would have failed with: + // - Expected: 4 (style::BreakType_PAGE_BEFORE) + // - Actual : 0 (style::BreakType_NONE) + // i.e. the page break was lost. + CPPUNIT_ASSERT_EQUAL(style::BreakType_PAGE_BEFORE, eBreakType); +} } CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/writerfilter/qa/cppunittests/dmapper/data/floattable-break-before.docx b/writerfilter/qa/cppunittests/dmapper/data/floattable-break-before.docx new file mode 100644 index 000000000000..7fcfed4a637d Binary files /dev/null and b/writerfilter/qa/cppunittests/dmapper/data/floattable-break-before.docx differ diff --git a/writerfilter/source/dmapper/DomainMapperTableHandler.cxx b/writerfilter/source/dmapper/DomainMapperTableHandler.cxx index eaddae615817..ac2f00a7d5a5 100644 --- a/writerfilter/source/dmapper/DomainMapperTableHandler.cxx +++ b/writerfilter/source/dmapper/DomainMapperTableHandler.cxx @@ -50,6 +50,7 @@ #include <comphelper/sequence.hxx> #include <comphelper/propertyvalue.hxx> #include <com/sun/star/lang/IndexOutOfBoundsException.hpp> +#include <com/sun/star/style/BreakType.hpp> #include <boost/lexical_cast.hpp> #include <officecfg/Office/Writer.hxx> @@ -1541,6 +1542,15 @@ void DomainMapperTableHandler::endTable(unsigned int nestedTableLevel, bool bTab // A non-zero left margin would move the table out of the frame, move the frame itself instead. xTableProperties->setPropertyValue("LeftMargin", uno::Any(sal_Int32(0))); + style::BreakType eBreakType{}; + xTableProperties->getPropertyValue("BreakType") >>= eBreakType; + if (eBreakType != style::BreakType_NONE) + { + // A break before the table was requested. Reset that break here, since the table + // will be at the start of the fly frame, not in the body frame. + xTableProperties->setPropertyValue("BreakType", uno::Any(style::BreakType_NONE)); + } + if (nestedTableLevel >= 2) { // Floating tables inside a table always stay inside the cell. @@ -1572,6 +1582,7 @@ void DomainMapperTableHandler::endTable(unsigned int nestedTableLevel, bool bTab // Multi-page floating tables works if an outer/toplevel table is floating, but not // when an inner table would float. bool bToplevelSplitFly = nestedTableLevel <= 1; + uno::Reference<beans::XPropertySet> xFrameAnchor; if (xTextAppendAndConvert.is() && (!bTableStartsAtCellStart || bToplevelSplitFly)) { std::deque<css::uno::Any> aFramedRedlines = m_rDMapper_Impl.m_aStoredRedlines[StoredRedlines::FRAME]; @@ -1580,10 +1591,17 @@ void DomainMapperTableHandler::endTable(unsigned int nestedTableLevel, bool bTab std::vector<OUString> redTable; BeforeConvertToTextFrame(aFramedRedlines, redPos, redLen, redCell, redTable); - xTextAppendAndConvert->convertToTextFrame(xStart, xEnd, comphelper::containerToSequence(aFrameProperties)); + uno::Reference<text::XTextContent> xContent = xTextAppendAndConvert->convertToTextFrame(xStart, xEnd, comphelper::containerToSequence(aFrameProperties)); + xFrameAnchor.set(xContent->getAnchor(), uno::UNO_QUERY); AfterConvertToTextFrame(m_rDMapper_Impl, aFramedRedlines, redPos, redLen, redCell, redTable); } + + if (xFrameAnchor.is() && eBreakType != style::BreakType_NONE) + { + // A break before the table was requested. Restore that on the anchor. + xFrameAnchor->setPropertyValue("BreakType", uno::Any(eBreakType)); + } } }