sw/qa/core/layout/data/floattable-nested.odt |binary sw/qa/core/layout/flycnt.cxx | 41 ++++++++++ sw/source/core/layout/flycnt.cxx | 21 ++++- sw/source/core/layout/tabfrm.cxx | 19 +++- sw/source/core/objectpositioning/tocntntanchoredobjectposition.cxx | 4 5 files changed, 79 insertions(+), 6 deletions(-)
New commits: commit d29c1a90ae77dde7c87c51f21e859fa254f23e01 Author: Miklos Vajna <[email protected]> AuthorDate: Wed Sep 6 08:30:10 2023 +0200 Commit: Miklos Vajna <[email protected]> CommitDate: Wed Sep 6 14:11:37 2023 +0200 sw floattable, nesting: fix layout crash The manually created ODT bugdoc has a nested floating table. The outer table is just 1 cell, the inner one has 2 rows: it's meant to span over 2 pages. This bugdoc currently crashes the layout. In practice what happens is that the inner fly would split + move the follow to the next page, so we hit a case where the inner fly is marked to be "in table", but it won't have a table parent anymore, so the layout crashes. As a first step, fix the crash: 1) SwFrame::GetNextFlyLeaf() should only split the inner fly and its anchor (not move it), the move will happen with the split of the outer fly. 2) Fix SwToContentAnchoredObjectPosition::CalcPosition() to move the inner, follow fly down (it should not overlap with its master). This was not visible previously, as we manually moved the follow fly to the next page, but now we need a vertical cut position that is between the inner master and follow flys. 3) lcl_ArrangeLowers(), called by SwCellFrame::Format() now updates the position of follow flys, so exactly two master flys are on page 1 and two follow flys are on page 2. This is related to tdf#55160, but it's not yet focusing on DOCX, where nested floating tables are currently disabled at a filter level. Also, this fixes the crash, but the position of the inner follow table still needs fixing. Change-Id: Ife42b64cda53946a8262ddabfd000803620c4ff1 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/156587 Reviewed-by: Miklos Vajna <[email protected]> Tested-by: Jenkins diff --git a/sw/qa/core/layout/data/floattable-nested.odt b/sw/qa/core/layout/data/floattable-nested.odt new file mode 100644 index 000000000000..3d21314f5871 Binary files /dev/null and b/sw/qa/core/layout/data/floattable-nested.odt differ diff --git a/sw/qa/core/layout/flycnt.cxx b/sw/qa/core/layout/flycnt.cxx index 539d379d97c2..0839e6a34b36 100644 --- a/sw/qa/core/layout/flycnt.cxx +++ b/sw/qa/core/layout/flycnt.cxx @@ -1024,6 +1024,47 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyFromAsCharAnchor) // frame+table inside a footnote. dispatchCommand(mxComponent, ".uno:SetAnchorToPara", {}); } + +CPPUNIT_TEST_FIXTURE(Test, testSplitFlyNested) +{ + // Given a document with a nested, multi-page floating table: + // When calculating the layout: + createSwDoc("floattable-nested.odt"); + calcLayout(); + + // Then make sure we don't crash: + // Without the accompanying fix in place, this test would have crashed. + // Check that we have exactly 4 fly frames, all of them on the expected pages: master outer, + // follow outer, master inner and follow inner. + SwDoc* pDoc = getSwDoc(); + SwRootFrame* pLayout = pDoc->getIDocumentLayoutAccess().GetCurrentLayout(); + auto pPage1 = pLayout->Lower()->DynCastPageFrame(); + CPPUNIT_ASSERT(pPage1); + CPPUNIT_ASSERT(pPage1->GetSortedObjs()); + SwSortedObjs& rPage1Objs = *pPage1->GetSortedObjs(); + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), rPage1Objs.size()); + auto pPage1Fly1 = rPage1Objs[0]->DynCastFlyFrame()->DynCastFlyAtContentFrame(); + CPPUNIT_ASSERT(pPage1Fly1); + CPPUNIT_ASSERT(pPage1Fly1->GetAnchorFrameContainingAnchPos()->IsInFly()); + CPPUNIT_ASSERT(pPage1Fly1->GetFollow()); + auto pPage1Fly2 = rPage1Objs[1]->DynCastFlyFrame()->DynCastFlyAtContentFrame(); + CPPUNIT_ASSERT(pPage1Fly2); + CPPUNIT_ASSERT(!pPage1Fly2->GetAnchorFrameContainingAnchPos()->IsInFly()); + CPPUNIT_ASSERT(pPage1Fly2->GetFollow()); + auto pPage2 = pPage1->GetNext()->DynCastPageFrame(); + CPPUNIT_ASSERT(pPage2); + CPPUNIT_ASSERT(pPage2->GetSortedObjs()); + SwSortedObjs& rPage2Objs = *pPage2->GetSortedObjs(); + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), rPage2Objs.size()); + auto pPage2Fly1 = rPage2Objs[0]->DynCastFlyFrame()->DynCastFlyAtContentFrame(); + CPPUNIT_ASSERT(pPage2Fly1); + CPPUNIT_ASSERT(pPage2Fly1->GetAnchorFrameContainingAnchPos()->IsInFly()); + CPPUNIT_ASSERT(pPage2Fly1->GetPrecede()); + auto pPage2Fly2 = rPage2Objs[1]->DynCastFlyFrame()->DynCastFlyAtContentFrame(); + CPPUNIT_ASSERT(pPage2Fly2); + CPPUNIT_ASSERT(!pPage2Fly2->GetAnchorFrameContainingAnchPos()->IsInFly()); + CPPUNIT_ASSERT(pPage2Fly2->GetPrecede()); +} } /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/source/core/layout/flycnt.cxx b/sw/source/core/layout/flycnt.cxx index 5eabc5ccaef2..d6aa1b6b2321 100644 --- a/sw/source/core/layout/flycnt.cxx +++ b/sw/source/core/layout/flycnt.cxx @@ -1591,6 +1591,7 @@ SwLayoutFrame *SwFrame::GetNextFlyLeaf( MakePageType eMakePage ) } SwLayoutFrame* pOldLayLeaf = nullptr; + bool bNesting = false; while (true) { if (pLayLeaf) @@ -1609,6 +1610,18 @@ SwLayoutFrame *SwFrame::GetNextFlyLeaf( MakePageType eMakePage ) bSameBody = true; } } + + if (bLeftFly && pFlyAnchor && pFlyAnchor->IsInFly() + && FindFlyFrame() == pLayLeaf->FindFlyFrame()) + { + // This is an inner fly, then the follow anchor will be just next to us. + SwLayoutFrame* pFlyAnchorUpper = pFlyAnchor->GetUpper(); + pOldLayLeaf = pLayLeaf; + pLayLeaf = pFlyAnchorUpper; + bLeftFly = false; + bNesting = true; + } + if (bLeftBody || bLeftFly || bSameBody) { // The above conditions are not held, reject. @@ -1650,8 +1663,12 @@ SwLayoutFrame *SwFrame::GetNextFlyLeaf( MakePageType eMakePage ) // Split the anchor at char 0: all the content goes to the follow of the anchor. pFlyAnchor->SplitFrame(TextFrameIndex(0)); auto pNext = static_cast<SwTextFrame*>(pFlyAnchor->GetNext()); - // Move the new anchor frame, before the first child of pLayLeaf. - pNext->MoveSubTree(pLayLeaf, pLayLeaf->Lower()); + // The nesting case just splits the inner fly; the outer fly will split and move. + if (!bNesting) + { + // Move the new anchor frame, before the first child of pLayLeaf. + pNext->MoveSubTree(pLayLeaf, pLayLeaf->Lower()); + } // Now create the follow of the fly and anchor it in the master of the anchor. pNew = new SwFlyAtContentFrame(*pFly); diff --git a/sw/source/core/layout/tabfrm.cxx b/sw/source/core/layout/tabfrm.cxx index 68bb19b7f39d..5b14113825e4 100644 --- a/sw/source/core/layout/tabfrm.cxx +++ b/sw/source/core/layout/tabfrm.cxx @@ -5287,11 +5287,24 @@ static bool lcl_ArrangeLowers( SwLayoutFrame *pLay, tools::Long lYStart, bool bI lcl_ArrangeLowers( static_cast<SwLayoutFrame*>(pFrame), aRectFnSet.GetTop(static_cast<SwLayoutFrame*>(pFrame)->Lower()->getFrameArea()) + lDiffX, bInva ); - if ( pFrame->GetDrawObjs() ) + SwSortedObjs* pDrawObjs = pFrame->GetDrawObjs(); + auto pTextFrame = pFrame->DynCastTextFrame(); + if (pTextFrame && pTextFrame->IsInFly()) { - for ( size_t i = 0; i < pFrame->GetDrawObjs()->size(); ++i ) + // See if this is a follow anchor. If so, we want the flys anchored in the master + // which are also lowers of pFrame. + SwTextFrame* pMaster = pTextFrame; + while (pMaster->IsFollow()) { - SwAnchoredObject* pAnchoredObj = (*pFrame->GetDrawObjs())[i]; + pMaster = pMaster->FindMaster(); + } + pDrawObjs = pMaster->GetDrawObjs(); + } + if (pDrawObjs) + { + for (size_t i = 0; i < pDrawObjs->size(); ++i) + { + SwAnchoredObject* pAnchoredObj = (*pDrawObjs)[i]; // #i26945# - check, if anchored object // is lower of layout frame by checking, if the anchor // frame, which contains the anchor position, is a lower diff --git a/sw/source/core/objectpositioning/tocntntanchoredobjectposition.cxx b/sw/source/core/objectpositioning/tocntntanchoredobjectposition.cxx index 73b8b088d1f7..9c1c491b24c9 100644 --- a/sw/source/core/objectpositioning/tocntntanchoredobjectposition.cxx +++ b/sw/source/core/objectpositioning/tocntntanchoredobjectposition.cxx @@ -492,11 +492,13 @@ void SwToContentAnchoredObjectPosition::CalcPosition() // same page as <pOrientFrame> and the vertical position isn't aligned // automatic at the anchor character or the top of the line of the // anchor character, the anchor frame determines the vertical position. + // Split fly follows: always let the anchor char frame determine the vertical position. + // This gives us a vertical cut position between the master and the follow. if ( &rAnchorTextFrame == pOrientFrame || ( rAnchorTextFrame.FindPageFrame() == pOrientFrame->FindPageFrame() && aVert.GetVertOrient() == text::VertOrientation::NONE && aVert.GetRelationOrient() != text::RelOrientation::CHAR && - aVert.GetRelationOrient() != text::RelOrientation::TEXT_LINE ) ) + aVert.GetRelationOrient() != text::RelOrientation::TEXT_LINE && !bFollowSplitFly ) ) { pUpperOfOrientFrame = rAnchorTextFrame.GetUpper(); pAnchorFrameForVertPos = &rAnchorTextFrame;
