diff options
author | Miklos Vajna <vmiklos@collabora.com> | 2024-02-29 08:17:41 +0100 |
---|---|---|
committer | Caolán McNamara <caolan.mcnamara@collabora.com> | 2024-03-05 09:23:21 +0100 |
commit | 2d6a0e2a354ffeba10f69634835a48d65e601846 (patch) | |
tree | 7f998ed2c7affa2fee594cf7e25c5a4b5bb161ca /sw | |
parent | 571a1ab88a41bfc4f8024bd1531c9f34b1023e43 (diff) |
tdf#158801 sw floattable: fix crash with headers and interactive editing
Regression from commit ce2fc5eb29b4e252993b549dee002fa8948c8386
(tdf#158341 sw floattable: fix layout loop when fly is below the body
frame, 2023-11-29), open the bugdoc, add an empty paragraph at the
start, Writer layout crashes. The immediate problem is that
SwTabFrame::MakeAll() assumes that in case HasFollowFlowLine() is true,
then GetFollow()->GetFirstNonHeadlineRow() is always non-nullptr,
similar to the situation in commit
223d2fac61e061478721a7a4a89b1362f5037d8f (sw floattable: fix crash by
trying harder to split tables, 2023-11-22).
The deeper problem is that the bugdoc has a repeated table header row,
the fly frame temporarily gets shifted down, so nominally the header
doesn't fit anymore, and this leads to a modification of the doc model,
which creates inconsistency: the model now says we have no header rows
but the layout still contains table row frames where the header bit is
true. This is problematic in theory, but in practice caused no problem
so far.
Fix the problem by disabling this mechanism for floating tables: trying
to have a table header that doesn't fit the table is asking for trouble
anyway, and this way at least we have a layout that is consistent with
the model, which also avoids the crash, now that nobody violates the
"HasFollowFlowLine -> follow's FirstNonHeadlineRow != nullptr"
invariant.
Also extend the layout dump, so it's easier to see when the master's
flag and the follow's row list gets out of sync.
(cherry picked from commit 186de7178c6065e1de13fd216b46ac9b716e44c5)
Change-Id: I52b51f6d86ab4e0bab560f892e9cceb183aecd5f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164366
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Diffstat (limited to 'sw')
-rw-r--r-- | sw/qa/core/layout/data/floattable-header.docx | bin | 0 -> 31604 bytes | |||
-rw-r--r-- | sw/qa/core/layout/tabfrm.cxx | 21 | ||||
-rw-r--r-- | sw/source/core/layout/tabfrm.cxx | 12 |
3 files changed, 32 insertions, 1 deletions
diff --git a/sw/qa/core/layout/data/floattable-header.docx b/sw/qa/core/layout/data/floattable-header.docx Binary files differnew file mode 100644 index 000000000000..baddd365ce37 --- /dev/null +++ b/sw/qa/core/layout/data/floattable-header.docx diff --git a/sw/qa/core/layout/tabfrm.cxx b/sw/qa/core/layout/tabfrm.cxx index e0d099c77102..9bfbd1b82e2a 100644 --- a/sw/qa/core/layout/tabfrm.cxx +++ b/sw/qa/core/layout/tabfrm.cxx @@ -17,6 +17,8 @@ #include <anchoredobject.hxx> #include <flyfrm.hxx> #include <flyfrms.hxx> +#include <docsh.hxx> +#include <wrtsh.hxx> namespace { @@ -235,6 +237,25 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyWrappedByTableNested) CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(3), pDoc->GetTableFrameFormats()->GetFormatCount()); CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), pDoc->GetSpzFrameFormats()->GetFormatCount()); } + +CPPUNIT_TEST_FIXTURE(Test, testSplitFlyHeader) +{ + // Given a document with 8 pages: a first page ending in a manual page break, then a multi-page + // floating table on pages 2..8: + createSwDoc("floattable-header.docx"); + CPPUNIT_ASSERT_EQUAL(8, getPages()); + + // When creating a new paragraph at doc start: + SwDocShell* pDocShell = getSwDocShell(); + SwWrtShell* pWrtShell = pDocShell->GetWrtShell(); + pWrtShell->SttEndDoc(/*bStt=*/true); + pWrtShell->SplitNode(); + // Without the accompanying fix in place, this test would have crashed here. + pWrtShell->CalcLayout(); + + // Then make sure we get one more page, since the first page is now 2 pages: + CPPUNIT_ASSERT_EQUAL(9, getPages()); +} } /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/source/core/layout/tabfrm.cxx b/sw/source/core/layout/tabfrm.cxx index 8c49106364dc..43b633934156 100644 --- a/sw/source/core/layout/tabfrm.cxx +++ b/sw/source/core/layout/tabfrm.cxx @@ -1165,7 +1165,13 @@ bool SwTabFrame::Split(const SwTwips nCutPos, bool bTryToSplit, OSL_ENSURE( !GetIndPrev(), "Table is supposed to be at beginning" ); if ( !IsInSct() ) { - m_pTable->SetRowsToRepeat(0); + // This would mean the layout modifies the doc model, so RowsToRepeat drops to 0 while + // there are existing row frames with RepeatedHeadline == true. Avoid this at least + // inside split flys, it would lead to a crash in SwTabFrame::MakeAll(). + if (!pFly || !pFly->IsFlySplitAllowed()) + { + m_pTable->SetRowsToRepeat(0); + } return false; } else @@ -6371,6 +6377,10 @@ void SwTabFrame::dumpAsXml(xmlTextWriterPtr writer) const { (void)xmlTextWriterStartElement(writer, reinterpret_cast<const xmlChar*>("tab")); SwFrame::dumpAsXmlAttributes( writer ); + + (void)xmlTextWriterWriteAttribute(writer, BAD_CAST("has-follow-flow-line"), + BAD_CAST(OString::boolean(m_bHasFollowFlowLine).getStr())); + if ( HasFollow() ) (void)xmlTextWriterWriteFormatAttribute( writer, BAD_CAST( "follow" ), "%" SAL_PRIuUINT32, GetFollow()->GetFrameId() ); |