sw/qa/extras/layout/data/tdf170846_1.docx |binary sw/qa/extras/layout/data/tdf170846_2.docx |binary sw/qa/extras/layout/layout6.cxx | 20 ++++++++++ sw/source/core/layout/tabfrm.cxx | 57 ++++++++++++++---------------- 4 files changed, 47 insertions(+), 30 deletions(-)
New commits: commit 4c53ed28749d408ac961ea2d747c257d72d93e3b Author: Mike Kaganski <[email protected]> AuthorDate: Tue Feb 17 11:34:00 2026 +0500 Commit: Xisco Fauli <[email protected]> CommitDate: Fri Feb 20 12:09:34 2026 +0100 tdf#170846: Use InsertMovedFwdFrame to move floating table forward Partially revert logic of commit 26aa5db06221ab940c65ecc27a2a52bf1ea0eb42 (tdf#170477 follow-up: reimplement the fix by cleaning up code, 2026-01-26). It turned out problematic to completely avoid InsertMovedFwdFrame. But this change does not call InsertMovedFwdFrame after MoveFwd, as in the old code; it uses InsertMovedFwdFrame instead of MoveFwd. Change-Id: If54f6cf18320d76843bb3bfad888b729f50a7a7c Reviewed-on: https://gerrit.libreoffice.org/c/core/+/199513 Reviewed-by: Mike Kaganski <[email protected]> Tested-by: Jenkins Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/199803 diff --git a/sw/qa/extras/layout/data/tdf170846_1.docx b/sw/qa/extras/layout/data/tdf170846_1.docx new file mode 100644 index 000000000000..b13782ba5514 Binary files /dev/null and b/sw/qa/extras/layout/data/tdf170846_1.docx differ diff --git a/sw/qa/extras/layout/data/tdf170846_2.docx b/sw/qa/extras/layout/data/tdf170846_2.docx new file mode 100644 index 000000000000..e7af0910978e Binary files /dev/null and b/sw/qa/extras/layout/data/tdf170846_2.docx differ diff --git a/sw/qa/extras/layout/layout6.cxx b/sw/qa/extras/layout/layout6.cxx index 75abbc4fc7d5..367802968171 100644 --- a/sw/qa/extras/layout/layout6.cxx +++ b/sw/qa/extras/layout/layout6.cxx @@ -2230,6 +2230,26 @@ CPPUNIT_TEST_FIXTURE(SwLayoutWriter6, testTdf169999) } } +CPPUNIT_TEST_FIXTURE(SwLayoutWriter6, testTdf170846_1) +{ + // In this document, the whole floating table must move to page 2 + createSwDoc("tdf170846_1.docx"); + xmlDocUniquePtr pXmlDoc = parseLayoutDump(); + // Without the fix, the next test failed: + assertXPath(pXmlDoc, "//page[1]//tab", 0); // No tables on page 1 + assertXPath(pXmlDoc, "//page[2]//tab", 1); // One table on page 2 +} + +CPPUNIT_TEST_FIXTURE(SwLayoutWriter6, testTdf170846_2) +{ + // In this document, the whole floating table must move to page 2 + createSwDoc("tdf170846_2.docx"); + xmlDocUniquePtr pXmlDoc = parseLayoutDump(); + // Without the fix, the next test failed: + assertXPath(pXmlDoc, "//page[1]//tab", 0); // No tables on page 1 + assertXPath(pXmlDoc, "//page[2]//tab", 3); // Three tables on page 2 +} + } // end of anonymous namespace CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/sw/source/core/layout/tabfrm.cxx b/sw/source/core/layout/tabfrm.cxx index 1a5d0c191aa5..b42d599dd165 100644 --- a/sw/source/core/layout/tabfrm.cxx +++ b/sw/source/core/layout/tabfrm.cxx @@ -3290,6 +3290,27 @@ void SwTabFrame::MakeAll(vcl::RenderContext* pRenderContext) { // Don't try to move floating table forward. It's done elsewhere moving its fly. bSkipMoveFwd = true; + // Only limit the InsertMovedFwdFrame hack to non-split floating tables for now: + // it seems that split tables don't need it. + if (!IsFollow() && !HasFollow()) + { + // This floating table doesn't fit on this page. Instead of the complex + // MoveFwd sequence (split anchor, create follow fly, move table, destroy + // old fly, merge anchor - which may oscillate), signal the anchor to move + // forward. The fly will follow. + SwTextFrame* pAnchor = pFlyFrame->FindAnchorCharFrame(); + if (pAnchor) + { + SwPageFrame* pPage = pAnchor->FindPageFrame(); + if (pPage) + { + SwLayouter::InsertMovedFwdFrame(pPage->GetFormat()->GetDoc(), + *pAnchor, + pPage->GetPhyPageNum() + 1); + pAnchor->InvalidatePos(); + } + } + } } } // don't make the effort to move fwd if its known commit 6aec3c203e936bad60f565a73a365dfab573aa5b Author: Mike Kaganski <[email protected]> AuthorDate: Mon Jan 26 15:29:24 2026 +0500 Commit: Xisco Fauli <[email protected]> CommitDate: Fri Feb 20 12:09:23 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]> Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/199802 diff --git a/sw/source/core/layout/tabfrm.cxx b/sw/source/core/layout/tabfrm.cxx index ea41e8e5f62b..1a5d0c191aa5 100644 --- a/sw/source/core/layout/tabfrm.cxx +++ b/sw/source/core/layout/tabfrm.cxx @@ -3277,58 +3277,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);
