diff options
author | Miklos Vajna <vmiklos@collabora.com> | 2023-07-19 08:29:01 +0200 |
---|---|---|
committer | Miklos Vajna <vmiklos@collabora.com> | 2023-07-19 12:17:50 +0200 |
commit | 45574624ff05673d44f11cdbbbb49e1af599133e (patch) | |
tree | 7b78d05023d8a448bdbab63d54ba4aaf50c0ebc7 | |
parent | a85cb997bc84a26196d8852bd0d546db1de3b761 (diff) |
tdf#120262 sw floattable: no split when none of first row fits the vert space
The final problem with the bugdoc is that half row of the second
floating table was still on page 1, while it should be fully on page 2.
The reason for this was that first we thought we can't move forward,
since GetIndPrev() returns nullptr for a table that's inside a fly
frame. Then we thought it's a good idea to split, even if the split
would move the entire first row to the next page.
Fix the problem by:
- In SwTabFrame::MakeAll(), handle split flys after calling
GetIndPrev(), an indirect prev of the anchor has the same meaning in
this context.
- In SwTabFrame::Split(), fail for split flys in case only half of the first
row's first line would fit, which gives an opporunity to call
MoveFwd().
- In SwTabFrame::MakeAll(), call MoveFwd() in a way that it's not a
problem that the GetIndPrev() call in MoveFwd() gives us a nullptr.
- At this point we don't split the table, we move it forward, but an
empty master remains on page 1. Fix that by improving
SwFlowFrame::MoveSubTree() to mark empty flys for deletion, similar to
how it does the same for sections.
- Finally avoid a layout warning in SwTabFrame::MakeAll(), it's OK to
try to split in case our split fly has a follow, we can move there on
split failure.
Also bring the test document closed to the original bugdoc, so we can
assert the content on both pages.
Change-Id: I2d3f88342d91b3e256bc41416a9afb274a9309d1
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/154633
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins
-rw-r--r-- | sw/qa/core/layout/data/floattable-no-footer-overlap.doc | bin | 0 -> 48640 bytes | |||
-rw-r--r-- | sw/qa/core/layout/data/floattable-no-footer-overlap.docx | bin | 20043 -> 0 bytes | |||
-rw-r--r-- | sw/qa/core/layout/flycnt.cxx | 9 | ||||
-rw-r--r-- | sw/source/core/layout/flowfrm.cxx | 13 | ||||
-rw-r--r-- | sw/source/core/layout/tabfrm.cxx | 50 |
5 files changed, 70 insertions, 2 deletions
diff --git a/sw/qa/core/layout/data/floattable-no-footer-overlap.doc b/sw/qa/core/layout/data/floattable-no-footer-overlap.doc Binary files differnew file mode 100644 index 000000000000..87e301189df5 --- /dev/null +++ b/sw/qa/core/layout/data/floattable-no-footer-overlap.doc diff --git a/sw/qa/core/layout/data/floattable-no-footer-overlap.docx b/sw/qa/core/layout/data/floattable-no-footer-overlap.docx Binary files differdeleted file mode 100644 index ca2f0d6d7244..000000000000 --- a/sw/qa/core/layout/data/floattable-no-footer-overlap.docx +++ /dev/null diff --git a/sw/qa/core/layout/flycnt.cxx b/sw/qa/core/layout/flycnt.cxx index 106be77ceac2..a1e4c05c6c1d 100644 --- a/sw/qa/core/layout/flycnt.cxx +++ b/sw/qa/core/layout/flycnt.cxx @@ -950,7 +950,7 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyAnchorKeepWithNext) CPPUNIT_TEST_FIXTURE(Test, testSplitFlyNoFooterOverlap) { // Given a document with 2 pages, a floating table on both pages: - createSwDoc("floattable-no-footer-overlap.docx"); + createSwDoc("floattable-no-footer-overlap.doc"); // When calculating the layout: calcLayout(); @@ -960,6 +960,13 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyNoFooterOverlap) SwRootFrame* pLayout = pDoc->getIDocumentLayoutAccess().GetCurrentLayout(); auto pPage1 = dynamic_cast<SwPageFrame*>(pLayout->Lower()); CPPUNIT_ASSERT(pPage1); + CPPUNIT_ASSERT(pPage1->GetSortedObjs()); + const SwSortedObjs& rPage1Objs = *pPage1->GetSortedObjs(); + // Without the accompanying fix in place, this test would have failed with: + // - Expected: 1 + // - Actual : 2 + // i.e. part of the second table was on page 1. + CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), rPage1Objs.size()); auto pPage2 = dynamic_cast<SwPageFrame*>(pPage1->GetNext()); // Without the accompanying fix in place, this test would have failed, there was no page 2, both // floating tables were on page 1. diff --git a/sw/source/core/layout/flowfrm.cxx b/sw/source/core/layout/flowfrm.cxx index fbcd6e14d159..f30c34938c20 100644 --- a/sw/source/core/layout/flowfrm.cxx +++ b/sw/source/core/layout/flowfrm.cxx @@ -693,6 +693,7 @@ void SwFlowFrame::MoveSubTree( SwLayoutFrame* pParent, SwFrame* pSibling ) // disappear automatically. SwSectionFrame *pSct; + SwFlyFrame* pFly = nullptr; if ( pOldParent && !pOldParent->Lower() && ( pOldParent->IsInSct() && !(pSct = pOldParent->FindSctFrame())->ContainsContent() && @@ -700,6 +701,18 @@ void SwFlowFrame::MoveSubTree( SwLayoutFrame* pParent, SwFrame* pSibling ) { pSct->DelEmpty( false ); } + else if (pOldParent && !pOldParent->Lower() + && (pOldParent->IsInFly() && !(pFly = pOldParent->FindFlyFrame())->ContainsContent() + && !pFly->ContainsAny())) + { + if (pFly->IsFlySplitAllowed()) + { + // Master fly is empty now that we pasted the content to the follow, mark it for + // deletion. + auto pFlyAtContent = static_cast<SwFlyAtContentFrame*>(pFly); + pFlyAtContent->DelEmpty(); + } + } // If we're in a column section, we'd rather not call Calc "from below" if( !m_rThis.IsInSct() && diff --git a/sw/source/core/layout/tabfrm.cxx b/sw/source/core/layout/tabfrm.cxx index a7e767a00e3a..7ce35fd62512 100644 --- a/sw/source/core/layout/tabfrm.cxx +++ b/sw/source/core/layout/tabfrm.cxx @@ -1099,6 +1099,13 @@ bool SwTabFrame::Split( const SwTwips nCutPos, bool bTryToSplit, bool bTableRowK if (nMinHeight > nRemainingSpaceForLastRow) { bSplitRowAllowed = false; + + if (!pRow->GetPrev() && aRectFnSet.GetHeight(pRow->getFrameArea()) > nRemainingSpaceForLastRow) + { + // Split of pRow is not allowed, no previous row, the current row doesn't fit: + // that's a failure, we'll have to move forward instead. + return false; + } } } } @@ -2498,6 +2505,19 @@ void SwTabFrame::MakeAll(vcl::RenderContext* pRenderContext) // 2. If this row wants to keep, we need an additional row // 3. The table is allowed to split or we do not have a pIndPrev: SwFrame* pIndPrev = GetIndPrev(); + + SwFlyFrame* pFly = FindFlyFrame(); + if (!pIndPrev && pFly && pFly->IsFlySplitAllowed()) + { + auto pFlyAtContent = static_cast<SwFlyAtContentFrame*>(pFly); + SwFrame* pAnchor = pFlyAtContent->FindAnchorCharFrame(); + if (pAnchor) + { + // If the anchor of the split has a previous frame, we're allowed to move forward. + pIndPrev = pAnchor->GetIndPrev(); + } + } + const SwRowFrame* pFirstNonHeadlineRow = GetFirstNonHeadlineRow(); // #i120016# if this row wants to keep, allow split in case that all rows want to keep with next, // the table can not move forward as it is the first one and a split is in general allowed. @@ -2752,11 +2772,23 @@ void SwTabFrame::MakeAll(vcl::RenderContext* pRenderContext) //Let's see if we find some place anywhere... if (!bMovedFwd) { + bool bMoveAlways = false; + SwFrame* pUpper = GetUpper(); + if (pUpper && pUpper->IsFlyFrame()) + { + auto pFlyFrame = static_cast<SwFlyFrame*>(pUpper); + if (pFlyFrame->IsFlySplitAllowed()) + { + // If the anchor of the split has a previous frame, MoveFwd() is allowed to move + // forward. + bMoveAlways = true; + } + } // don't make the effort to move fwd if its known // conditions that are known not to work if (IsInFootnote() && ForbiddenForFootnoteCntFwd()) bMakePage = false; - else if (!MoveFwd(bMakePage, false)) + else if (!MoveFwd(bMakePage, false, bMoveAlways)) bMakePage = false; } @@ -2804,6 +2836,22 @@ void SwTabFrame::MakeAll(vcl::RenderContext* pRenderContext) // allowed to split. SwTwips nDistToUpperPrtBottom = aRectFnSet.BottomDist( getFrameArea(), aRectFnSet.GetPrtBottom(*GetUpper())); + + if (GetUpper()->IsFlyFrame()) + { + SwFlyFrame* pFlyFrame = GetUpper()->FindFlyFrame(); + if (pFlyFrame->IsFlySplitAllowed()) + { + SwTextFrame* pAnchor = pFlyFrame->FindAnchorCharFrame(); + if (pAnchor && pAnchor->HasFollow()) + { + // The split fly's anchor has a follow frame, we can move there & try to + // split again. + bTryToSplit = true; + } + } + } + if ( nDistToUpperPrtBottom >= 0 || bTryToSplit ) { lcl_RecalcTable( *this, nullptr, aNotify ); |