sw/qa/core/layout/data/floattable-anchor-keep-with-next.docx |binary sw/qa/core/layout/flycnt.cxx | 25 ++++++++++ sw/source/core/layout/flowfrm.cxx | 11 ++++ writerfilter/qa/cppunittests/dmapper/DomainMapper_Impl.cxx | 17 ++++++ writerfilter/qa/cppunittests/dmapper/data/floattable-sectend.docx |binary writerfilter/source/dmapper/DomainMapper_Impl.cxx | 8 +++ 6 files changed, 61 insertions(+)
New commits: commit 677761a10059c33ba25df7daeee4e72e199248e4 Author: Miklos Vajna <[email protected]> AuthorDate: Fri Jun 2 11:02:46 2023 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Tue Jun 6 08:27:25 2023 +0200 sw floattable: ignore keep-with-next for anchors of non-last split flys The bugdoc has a floating table that could split between page 1 and page 2, but we used to put the entire table to page 2. What happens is that the anchor of the floating table had <w:keepNext>, which means the layout tried to keep the paragraph on a single page, and also with the next paragraph (but in this case there is no next paragraph, it's the last one in the document). Fix the problem ignoring the "keep with next" request, in case the paragraph is an anchor for a split floating table; unless we're the last anchor in the chain. This is probably consistent with the intent that "keep with next" is meant to control the anchor text and not the anchored floating tables. (cherry picked from commit 920e76f15b78398de62002e30002f4f8e0fee7c1) Change-Id: I387a2db5f5db013da7055686ae6b9d032b467266 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/152602 Tested-by: Jenkins CollaboraOffice <[email protected]> Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sw/qa/core/layout/data/floattable-anchor-keep-with-next.docx b/sw/qa/core/layout/data/floattable-anchor-keep-with-next.docx new file mode 100644 index 000000000000..a0930900d884 Binary files /dev/null and b/sw/qa/core/layout/data/floattable-anchor-keep-with-next.docx differ diff --git a/sw/qa/core/layout/flycnt.cxx b/sw/qa/core/layout/flycnt.cxx index 55ac7382a062..edd079b6a819 100644 --- a/sw/qa/core/layout/flycnt.cxx +++ b/sw/qa/core/layout/flycnt.cxx @@ -921,6 +921,31 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyNextLeafInSection) // Then this never returned, the loop in SwFrame::GetNextFlyLeaf() never finished. calcLayout(); } + +CPPUNIT_TEST_FIXTURE(Test, testSplitFlyAnchorKeepWithNext) +{ + // Given a document with 2 pages, a split floating table on both pages: + createSwDoc("floattable-anchor-keep-with-next.docx"); + + // When calculating the layout: + calcLayout(); + + // Then make sure the pages have the expected amount of anchored objects: + SwDoc* pDoc = getSwDoc(); + SwRootFrame* pLayout = pDoc->getIDocumentLayoutAccess().GetCurrentLayout(); + auto pPage1 = dynamic_cast<SwPageFrame*>(pLayout->Lower()); + CPPUNIT_ASSERT(pPage1); + // Without the accompanying fix in place, this test would have failed, page 1 had no floating + // table, it was entirely on page 2. + CPPUNIT_ASSERT(pPage1->GetSortedObjs()); + const SwSortedObjs& rPage1Objs = *pPage1->GetSortedObjs(); + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), rPage1Objs.size()); + auto pPage2 = dynamic_cast<SwPageFrame*>(pPage1->GetNext()); + CPPUNIT_ASSERT(pPage2); + CPPUNIT_ASSERT(pPage2->GetSortedObjs()); + const SwSortedObjs& rPage2Objs = *pPage2->GetSortedObjs(); + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), rPage2Objs.size()); +} } /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/source/core/layout/flowfrm.cxx b/sw/source/core/layout/flowfrm.cxx index b499257bb001..68bde26d59b7 100644 --- a/sw/source/core/layout/flowfrm.cxx +++ b/sw/source/core/layout/flowfrm.cxx @@ -248,6 +248,17 @@ bool SwFlowFrame::IsKeep(SvxFormatKeepItem const& rKeep, ( !m_rThis.IsInTab() || m_rThis.IsTabFrame() ) && rKeep.GetValue() && !IsNextContentFullPage(m_rThis)); + if (bKeep && m_rThis.IsTextFrame()) + { + auto& rTextFrame = static_cast<SwTextFrame&>(m_rThis); + if (rTextFrame.HasNonLastSplitFlyDrawObj()) + { + // Allow split for the non-last anchors of a split fly, even if rKeep.GetValue() is + // true. + bKeep = false; + } + } + OSL_ENSURE( !bCheckIfLastRowShouldKeep || m_rThis.IsTabFrame(), "IsKeep with bCheckIfLastRowShouldKeep should only be used for tabfrms" ); commit 1c18eed7c1f80e5b72c8d29e717cc59daaa337dd Author: Miklos Vajna <[email protected]> AuthorDate: Thu Jun 1 08:31:34 2023 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Tue Jun 6 08:27:20 2023 +0200 tdf#103869 sw floattable: fix lost table right before a section break from DOCX The bugdoc has 2 pages and 2 tables, one table on each page. The table on the first page was missing in Writer. What happened is that the floating table is anchored in the next paragraph, but that is removed (since it's the last one in the section, so no need for a "next" paragraph), which shifted the table to the next section, which was already a wrong anchor point. Then that next section (next page) started with a (floating) table, so a dummy text node was inserted at the start, which means once it's disposed at the end of the 2nd section, we lost the first floating table with its bad anchor. Fixing the problem was a bit tricky, because DomainMapper_Impl::RemoveLastParagraph() is called before the text frame conversion would be invoked in DomainMapperTableHandler::endTable(), so we can't check if this last paragraph has something anchored in it, as it's too early. Instead, detect that a floating table will be created, and don't remove the last paragraph in this case, since we know that we're at the end of the section (that's why we remove the last paragraph). The export result looks OK, we keep the paragraph-table-paragraph-table-paragraph model correctly. (cherry picked from commit e2f90d1d0e51c68dd01c9968cdb7a3bbb5658613) Change-Id: I4612a15d0d1ad3fe527593a21a4120096790a33f Reviewed-on: https://gerrit.libreoffice.org/c/core/+/152527 Tested-by: Jenkins CollaboraOffice <[email protected]> Reviewed-by: Miklos Vajna <[email protected]> diff --git a/writerfilter/qa/cppunittests/dmapper/DomainMapper_Impl.cxx b/writerfilter/qa/cppunittests/dmapper/DomainMapper_Impl.cxx index 858faf709fd4..733fef6b1db6 100644 --- a/writerfilter/qa/cppunittests/dmapper/DomainMapper_Impl.cxx +++ b/writerfilter/qa/cppunittests/dmapper/DomainMapper_Impl.cxx @@ -348,6 +348,23 @@ CPPUNIT_TEST_FIXTURE(Test, testFloatingTableSectionBreak) // i.e. the document was of 1 page, the section break was lost. CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int16>(2), xCursor->getPage()); } + +CPPUNIT_TEST_FIXTURE(Test, testFloattableSectend) +{ + // Given a document with 2 tables, table 1 on page 1, table 2 on page 2: + loadFromURL(u"floattable-sectend.docx"); + + // When importing that document and listing the tables: + uno::Reference<text::XTextTablesSupplier> xTextDocument(mxComponent, uno::UNO_QUERY); + uno::Reference<container::XIndexAccess> xTables(xTextDocument->getTextTables(), uno::UNO_QUERY); + + // Then make sure that we have two tables: + // Without the accompanying fix in place, this test would have failed with: + // - Expected: 2 + // - Actual : 1 + // i.e. the first table was lost. + CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(2), xTables->getCount()); +} } /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/writerfilter/qa/cppunittests/dmapper/data/floattable-sectend.docx b/writerfilter/qa/cppunittests/dmapper/data/floattable-sectend.docx new file mode 100644 index 000000000000..50a121412d10 Binary files /dev/null and b/writerfilter/qa/cppunittests/dmapper/data/floattable-sectend.docx differ diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx index 3936dfcd147f..745a4ca355e5 100644 --- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx +++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx @@ -758,6 +758,14 @@ void DomainMapper_Impl::RemoveLastParagraph( ) uno::Reference< text::XTextAppend > xTextAppend = m_aTextAppendStack.top().xTextAppend; if (!xTextAppend.is()) return; + + if (hasTableManager() && getTableManager().getCurrentTablePosition().getLength() != 0) + { + // If we have an open floating table, then don't remove this paragraph, since that'll be the + // anchor of the floating table. Otherwise we would lose the table. + return; + } + try { uno::Reference< text::XTextCursor > xCursor;
