sw/source/core/layout/tabfrm.cxx | 36 ++++++------------------------------ 1 file changed, 6 insertions(+), 30 deletions(-)
New commits: commit 26aa5db06221ab940c65ecc27a2a52bf1ea0eb42 Author: Mike Kaganski <[email protected]> AuthorDate: Mon Jan 26 15:29:24 2026 +0500 Commit: Mike Kaganski <[email protected]> CommitDate: Mon Jan 26 16:30:56 2026 +0100 tdf#170477 follow-up: reimplement the fix by cleaning up code This is an alternative fix to the bug. The idea is, that we shouldn't call MoveFwd in SwTabFrame::MakeAll in case of floating tables. In commit 45574624ff05673d44f11cdbbbb49e1af599133e (tdf#120262 sw floattable: no split when none of first row fits the vert space, 2023-07-19) a code was added, that made sure that for floating tables MoveFwd was called with bMoveAlways = true. Obviously, for that bug, this code path was used at that time. However, the unit test for that but doesn't pass the code path these days. What happens inside MoveFwd for a floating table is: 1. MoveFwd calls GetLeaf, which in turn calls GetNextFlyLeaf; 2. GetNextFlyLeaf splits the anchor (the new frame isn't yet moved), and creates a follow fly, which is still on the same page; 3. MoveFwd moves the whole table to the new fly, making the old fly empty; 4. The old fly calls its DelEmpty, breaking precede/follow chain and making the new fly standalone. In the end, the situation after the call is fundamentally the same as before, with exception of new garbage (empty fly) to clean later, and the split anchor that needs a join. This change avoids calling MoveFwd for floating tables in this case. It keeps all unit tests working, so hopefully this cleanup could be an improvement by simplifying code, and by avoiding a call to SwLayouter::InsertMovedFwdFrame, which itself is manual override of the normal layout process (IMO, the less it's used, the better). If it turns out problematic, it may be simply reverted (that's the idea of merging it only after the more conservative fix). Change-Id: I606976de629bb5514edafd1430704901241876cc Reviewed-on: https://gerrit.libreoffice.org/c/core/+/198135 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> diff --git a/sw/source/core/layout/tabfrm.cxx b/sw/source/core/layout/tabfrm.cxx index f6cd00b56e7a..2e702f736a74 100644 --- a/sw/source/core/layout/tabfrm.cxx +++ b/sw/source/core/layout/tabfrm.cxx @@ -3274,58 +3274,34 @@ void SwTabFrame::MakeAll(vcl::RenderContext* pRenderContext) // #i29771# Reset bTryToSplit flag on change of upper const SwFrame* pOldUpper = GetUpper(); - const SwPageFrame* pOldUpperPage = nullptr; //Let's see if we find some place anywhere... if (!bMovedFwd) { - bool bMoveAlways = false; + bool bSkipMoveFwd = 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; - pOldUpperPage = pFlyFrame->FindPageFrame(); + // Don't try to move floating table forward. It's done elsewhere moving its fly. + bSkipMoveFwd = 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, bMoveAlways)) + else if (bSkipMoveFwd || !MoveFwd(bMakePage, false)) bMakePage = false; } // #i29771# Reset bSplitError flag on change of upper if ( GetUpper() != pOldUpper ) { - bool bResetSplit = true; - if (GetUpper() && GetUpper()->IsFlyFrame() && pOldUpperPage - && GetUpper()->FindPageFrame() == pOldUpperPage - && static_cast<const SwFlyFrame*>(GetUpper())->IsFlySplitAllowed()) - { - // MoveFwd created a split (follow) fly on the same page, that was intended to move - // to the next page together with its anchor. Then the content was moved to the new - // fly, and the old fly became empty; it called its DelEmpty(). The new fly is now - // basically a copy of the old one; move to the next page and prevent oscillation. - SwTextFrame* pAnchor = static_cast<SwFlyFrame*>(GetUpper())->FindAnchorCharFrame(); - if (pAnchor) - { - bResetSplit = false; - SwPageFrame* pPage = pAnchor->FindPageFrame(); - SwLayouter::InsertMovedFwdFrame(pPage->GetFormat()->GetDoc(), *pAnchor, - pPage->GetPhyPageNum() + 1); - } - } - if (bResetSplit) - { - bTryToSplit = true; - nUnSplitted = 5; - } + bTryToSplit = true; + nUnSplitted = 5; } aRectFnSet.Refresh(*this);
