ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

tdf#170477 follow-up: reimplement the fix by cleaning up code

Browse Source 
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 45574624ff (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 <mike.kaganski@collabora.com>
This commit is contained in:
Mike Kaganski
2026-01-26 16:30:56 +01:00
parent 705dcfe362
commit 26aa5db062
1 changed files with 6 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sw/source/core/layout/tabfrm.cxx
+6 -30
View File
@@ -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);