sw/qa/core/layout/data/floattable-header.docx |binary sw/qa/core/layout/tabfrm.cxx | 21 +++++++++++++++++++++ sw/qa/extras/uiwriter/uiwriter5.cxx | 2 -- sw/source/core/inc/tabfrm.hxx | 1 - sw/source/core/layout/tabfrm.cxx | 19 +++++++++++++++++-- sw/source/core/text/xmldump.cxx | 10 ---------- sw/source/core/undo/untbl.cxx | 15 ++++++++++++++- 7 files changed, 52 insertions(+), 16 deletions(-)
New commits: commit 9e3adbc98bba6211d03b30752136fa17aa1a91f3 Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Thu Feb 29 08:17:41 2024 +0100 Commit: Michael Stahl <michael.st...@allotropia.de> CommitDate: Thu Mar 21 13:16:12 2024 +0100 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. Change-Id: I52b51f6d86ab4e0bab560f892e9cceb183aecd5f Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164136 Reviewed-by: Miklos Vajna <vmik...@collabora.com> Tested-by: Jenkins (cherry picked from commit 186de7178c6065e1de13fd216b46ac9b716e44c5) diff --git a/sw/qa/core/layout/data/floattable-header.docx b/sw/qa/core/layout/data/floattable-header.docx new file mode 100644 index 000000000000..baddd365ce37 Binary files /dev/null and b/sw/qa/core/layout/data/floattable-header.docx differ diff --git a/sw/qa/core/layout/tabfrm.cxx b/sw/qa/core/layout/tabfrm.cxx index 1659cf0df6fd..b6219bc2966b 100644 --- a/sw/qa/core/layout/tabfrm.cxx +++ b/sw/qa/core/layout/tabfrm.cxx @@ -13,6 +13,8 @@ #include <rootfrm.hxx> #include <pagefrm.hxx> #include <tabfrm.hxx> +#include <docsh.hxx> +#include <wrtsh.hxx> namespace { @@ -109,6 +111,25 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyNestedRowSpan) // Then make sure the resulting page count matches Word: CPPUNIT_ASSERT_EQUAL(6, getPages()); } + +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 df23a69dce71..597dd053e727 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 @@ -6230,6 +6236,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() ); commit e12be6921f5421527ffdfa5021701a9822526b4d Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Fri Aug 11 08:05:11 2023 +0200 Commit: Michael Stahl <michael.st...@allotropia.de> CommitDate: Thu Mar 21 13:14:37 2024 +0100 sw: fold SwTabFrame::dumpAsXmlAttributes() into dumpAsXml() One dumpAsXml() per SwFrame subclass is enough, no need to have a separate function that dumps just the attributes. Change-Id: I7341e6a2c71c9a3596f70397c4450206222b76f2 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/155572 Reviewed-by: Miklos Vajna <vmik...@collabora.com> Tested-by: Jenkins (cherry picked from commit af4341a0262352efecd772a7390fb2cff544564b) diff --git a/sw/source/core/inc/tabfrm.hxx b/sw/source/core/inc/tabfrm.hxx index 4eba886c2385..e53474d325d9 100644 --- a/sw/source/core/inc/tabfrm.hxx +++ b/sw/source/core/inc/tabfrm.hxx @@ -239,7 +239,6 @@ public: sal_uInt16 GetBottomLineSize() const; - virtual void dumpAsXmlAttributes(xmlTextWriterPtr writer) const override; void dumpAsXml(xmlTextWriterPtr writer = nullptr) const override; }; diff --git a/sw/source/core/layout/tabfrm.cxx b/sw/source/core/layout/tabfrm.cxx index f730d85f7db5..df23a69dce71 100644 --- a/sw/source/core/layout/tabfrm.cxx +++ b/sw/source/core/layout/tabfrm.cxx @@ -6229,7 +6229,12 @@ bool SwTabFrame::IsCollapsingBorders() const void SwTabFrame::dumpAsXml(xmlTextWriterPtr writer) const { (void)xmlTextWriterStartElement(writer, reinterpret_cast<const xmlChar*>("tab")); - dumpAsXmlAttributes(writer); + SwFrame::dumpAsXmlAttributes( writer ); + if ( HasFollow() ) + (void)xmlTextWriterWriteFormatAttribute( writer, BAD_CAST( "follow" ), "%" SAL_PRIuUINT32, GetFollow()->GetFrameId() ); + + if (m_pPrecede != nullptr) + (void)xmlTextWriterWriteFormatAttribute( writer, BAD_CAST( "precede" ), "%" SAL_PRIuUINT32, static_cast<SwTabFrame*>( m_pPrecede )->GetFrameId() ); (void)xmlTextWriterStartElement(writer, BAD_CAST("infos")); dumpInfosAsXml(writer); diff --git a/sw/source/core/text/xmldump.cxx b/sw/source/core/text/xmldump.cxx index 5c85d5c601fb..103eb170fd07 100644 --- a/sw/source/core/text/xmldump.cxx +++ b/sw/source/core/text/xmldump.cxx @@ -444,14 +444,4 @@ void SwSectionFrame::dumpAsXmlAttributes( xmlTextWriterPtr writer ) const (void)xmlTextWriterWriteFormatAttribute( writer, BAD_CAST( "precede" ), "%" SAL_PRIuUINT32, static_cast<SwSectionFrame*>( m_pPrecede )->GetFrameId() ); } -void SwTabFrame::dumpAsXmlAttributes( xmlTextWriterPtr writer ) const -{ - SwFrame::dumpAsXmlAttributes( writer ); - if ( HasFollow() ) - (void)xmlTextWriterWriteFormatAttribute( writer, BAD_CAST( "follow" ), "%" SAL_PRIuUINT32, GetFollow()->GetFrameId() ); - - if (m_pPrecede != nullptr) - (void)xmlTextWriterWriteFormatAttribute( writer, BAD_CAST( "precede" ), "%" SAL_PRIuUINT32, static_cast<SwTabFrame*>( m_pPrecede )->GetFrameId() ); -} - /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ commit f4ef889614308e51d741db2e31ba8477f09d5b6b Author: Michael Stahl <michael.st...@allotropia.de> AuthorDate: Thu Mar 14 12:14:28 2024 +0100 Commit: Michael Stahl <michael.st...@allotropia.de> CommitDate: Thu Mar 21 11:44:33 2024 +0100 sw: fix ~SwIndexReg assert in testTdf149498 The problem is that a SwNavigationMgr thingy has a cursor in one of the table cells, and the text node is moved to the undo nodes array in SwUndoTableCpyTable::AddBoxBefore() and deleted in SwUndoTableCpyTable::UndoImpl(). SwUndoTableCpyTable needs to move the cursors out of the way because SwUndoDelete doesn't do it. Change-Id: I75e271c84a6624ffb0df151b171acb1e1f743928 diff --git a/sw/qa/extras/uiwriter/uiwriter5.cxx b/sw/qa/extras/uiwriter/uiwriter5.cxx index 50e19edd273d..db3357446012 100644 --- a/sw/qa/extras/uiwriter/uiwriter5.cxx +++ b/sw/qa/extras/uiwriter/uiwriter5.cxx @@ -2807,7 +2807,6 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest5, testTdf156487) assertXPath(pXmlDoc, "/metafile/push/push/push/textarray/text", 1); } -#ifndef DBG_UTIL CPPUNIT_TEST_FIXTURE(SwUiWriterTest5, testTdf149498) { // load a table, and delete the first column with enabled change tracking: @@ -2823,7 +2822,6 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest5, testTdf149498) // this would crash due to bookmark over cell boundary dispatchCommand(mxComponent, ".uno:Undo", {}); } -#endif CPPUNIT_TEST_FIXTURE(SwUiWriterTest5, testTdf150673_RedlineTableColumnDeletionWithExport) { diff --git a/sw/source/core/undo/untbl.cxx b/sw/source/core/undo/untbl.cxx index 8da85452c7d9..c4060e053e92 100644 --- a/sw/source/core/undo/untbl.cxx +++ b/sw/source/core/undo/untbl.cxx @@ -2599,11 +2599,17 @@ void SwUndoTableCpyTable::AddBoxBefore( const SwTableBox& rBox, bool bDelContent if( bDelContent ) { SwNodeIndex aInsIdx( *rBox.GetSttNd(), 1 ); - pDoc->GetNodes().MakeTextNode( aInsIdx.GetNode(), pDoc->GetDfltTextFormatColl() ); + SwTextNode *const pNewNode(pDoc->GetNodes().MakeTextNode(aInsIdx.GetNode(), pDoc->GetDfltTextFormatColl())); SwPaM aPam( aInsIdx.GetNode(), *rBox.GetSttNd()->EndOfSectionNode() ); if( !pDoc->getIDocumentRedlineAccess().IsRedlineOn() ) + { + { // move cursors to new node which precedes aPam + SwPosition const pos(*pNewNode, 0); + ::PaMCorrAbs(aPam, pos); + } pEntry->pUndo = std::make_unique<SwUndoDelete>(aPam, SwDeleteFlags::Default, true); + } } pEntry->pBoxNumAttr = std::make_unique<SfxItemSetFixed< @@ -2627,6 +2633,13 @@ void SwUndoTableCpyTable::AddBoxAfter( const SwTableBox& rBox, const SwNodeIndex SwDoc* pDoc = rBox.GetFrameFormat()->GetDoc(); DEBUG_REDLINE( pDoc ) + { // move cursors to first node which was inserted + SwPaM pam(SwNodeIndex(*rBox.GetSttNd(), 1)); + assert(pam.GetPoint()->GetNode().IsTextNode()); + pam.SetMark(); + pam.Move(fnMoveForward, GoInContent); + ::PaMCorrAbs(pam, *pam.GetPoint()); + } if( pDoc->getIDocumentRedlineAccess().IsRedlineOn() ) { SwPosition aTmpPos( rIdx );