sw/qa/extras/odfexport/data/tdf169882.odt |binary sw/qa/extras/odfexport/odfexport4.cxx | 7 + sw/qa/inc/swmodeltestbase.hxx | 1 sw/qa/unit/swmodeltestbase.cxx | 7 + sw/source/core/layout/flycnt.cxx | 20 --- sw/source/core/layout/objectformattertxtfrm.cxx | 138 +----------------------- sw/source/core/layout/objectformattertxtfrm.hxx | 6 - 7 files changed, 31 insertions(+), 148 deletions(-)
New commits: commit 0a5f8141a5aa37bfdb7c639b18166b1ae547a3b6 Author: Mike Kaganski <[email protected]> AuthorDate: Wed Dec 10 21:13:55 2025 +0500 Commit: Xisco Fauli <[email protected]> CommitDate: Mon Jan 12 10:12:06 2026 +0100 tdf#169882: revert commit c799de145f7e289f31e3669646e5bd12814e6c5e (tdf#138518 sw: layout: avoid moving flys forward prematurely, 2021-04-22). That change made sure that if a fly tries to move its anchor forward, and there are more fly frames on this page anchored after this fly's anchor, then the anchor was not added to "moved forward frames" list that is used to prevent frames from moving backward. (Note that the problem is similar to the problem fixed by commit 02a1edc64bde14401899d0f026d49dbf125e3ffc - see its commit message for details.) Bugdoc for tdf#169882 has a table with a row having two cells, each with an image; the images require the row to move forward. For the first cell, layout moved the image forward (which caused the whole row to move), but didn't mark its anchor, because there was another fly "below this". Then the text succeeded moving back, creating a layout loop. Before the fix of tdf#166871 (which caused the regression addressed here), the optimization allowed the layout to skip the problem (but it was actually possible to trigger it in a different way). It could be possible to modify the code to special-case table rows. The code in SwObjectFormatterTextFrame::CheckMovedFwdCondition contained a similar condition for "on same page" case. But it seems that the whole workaround is now not needed. Reverting it keeps bugdoc from tdf#138518 fixed, so avoid the fragile workaround. This change also reverts commit d9e38d5830ac278d7180b96fb6cefe3ee8ef6d76 (tdf#135220 sw: fix layout after SwUndoDelete), which keeps its steps behave correctly. Change-Id: I299e8f78b83c1d73402e548ea7f6cce1a2e5af0d Reviewed-on: https://gerrit.libreoffice.org/c/core/+/195395 Reviewed-by: Mike Kaganski <[email protected]> Tested-by: Jenkins Signed-off-by: Xisco Fauli <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/195489 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/195534 diff --git a/sw/qa/extras/odfexport/data/tdf169882.odt b/sw/qa/extras/odfexport/data/tdf169882.odt new file mode 100644 index 000000000000..9f5ff59ec5f1 Binary files /dev/null and b/sw/qa/extras/odfexport/data/tdf169882.odt differ diff --git a/sw/qa/extras/odfexport/odfexport4.cxx b/sw/qa/extras/odfexport/odfexport4.cxx index 28994104f392..730777bac8ea 100644 --- a/sw/qa/extras/odfexport/odfexport4.cxx +++ b/sw/qa/extras/odfexport/odfexport4.cxx @@ -1644,6 +1644,13 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf168980) saveAndReload(u"OpenDocument Text Flat XML"_ustr); } +CPPUNIT_TEST_FIXTURE(Test, testTdf169882) +{ + // The document must not hang on layout + createSwDoc("tdf169882.odt"); + saveAndReload(TestFilter::ODT); +} + } // end of anonymous namespace CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/sw/qa/inc/swmodeltestbase.hxx b/sw/qa/inc/swmodeltestbase.hxx index c77a0d212141..3ca6563eb757 100644 --- a/sw/qa/inc/swmodeltestbase.hxx +++ b/sw/qa/inc/swmodeltestbase.hxx @@ -197,6 +197,7 @@ protected: void header(); void saveAndReload(const OUString& pFilter, const char* pPassword = nullptr); + void saveAndReload(TestFilter eFilter, const char* pPassword = nullptr); /// Combines load() and save(). void loadAndSave(const char* pName, const char* pPassword = nullptr); diff --git a/sw/qa/unit/swmodeltestbase.cxx b/sw/qa/unit/swmodeltestbase.cxx index 523c5186d358..61a754104361 100644 --- a/sw/qa/unit/swmodeltestbase.cxx +++ b/sw/qa/unit/swmodeltestbase.cxx @@ -397,6 +397,13 @@ void SwModelTestBase::saveAndReload(const OUString& pFilter, const char* pPasswo loadURL(maTempFile.GetURL(), pPassword); } +void SwModelTestBase::saveAndReload(TestFilter eFilter, const char* pPassword) +{ + save(eFilter, pPassword); + + loadURL(maTempFile.GetURL(), pPassword); +} + void SwModelTestBase::loadAndSave(const char* pName, const char* pPassword) { loadURL(createFileURL(OUString::createFromAscii(pName)), pPassword); diff --git a/sw/source/core/layout/flycnt.cxx b/sw/source/core/layout/flycnt.cxx index 67081a263cb1..2eb148edb6d7 100644 --- a/sw/source/core/layout/flycnt.cxx +++ b/sw/source/core/layout/flycnt.cxx @@ -444,17 +444,12 @@ void SwFlyAtContentFrame::MakeAll(vcl::RenderContext* pRenderContext) // <SwObjectFormatterTextFrame::CheckMovedFwdCondition(..)> sal_uInt32 nToPageNum( 0 ); bool bDummy( false ); - bool bPageHasFlysAnchoredBelowThis(false); if ( SwObjectFormatterTextFrame::CheckMovedFwdCondition( // TODO: what if this fly moved bc it's in table? does sth prevent that? *this, *GetPageFrame(), - bAnchoredAtMaster, nToPageNum, bDummy, - bPageHasFlysAnchoredBelowThis) ) + bAnchoredAtMaster, nToPageNum, bDummy)) { - if (!bPageHasFlysAnchoredBelowThis) - { - bConsiderWrapInfluenceDueToMovedFwdAnchor = true; - } + bConsiderWrapInfluenceDueToMovedFwdAnchor = true; // mark anchor text frame // directly, that it is moved forward by object positioning. SwTextFrame* pAnchorTextFrame( static_cast<SwTextFrame*>(AnchorFrame()) ); @@ -465,21 +460,14 @@ void SwFlyAtContentFrame::MakeAll(vcl::RenderContext* pRenderContext) { if ( nAnchorFrameToPageNum < nToPageNum ) { - if (!bPageHasFlysAnchoredBelowThis) - { - SwLayouter::RemoveMovedFwdFrame(rDoc, *pAnchorTextFrame); - } + SwLayouter::RemoveMovedFwdFrame(rDoc, *pAnchorTextFrame); } else bInsert = false; } if ( bInsert ) { - if (!bPageHasFlysAnchoredBelowThis) - { - SwLayouter::InsertMovedFwdFrame(rDoc, *pAnchorTextFrame, - nToPageNum); - } + SwLayouter::InsertMovedFwdFrame(rDoc, *pAnchorTextFrame, nToPageNum); } } } diff --git a/sw/source/core/layout/objectformattertxtfrm.cxx b/sw/source/core/layout/objectformattertxtfrm.cxx index 00f215b9ecfc..19f544302954 100644 --- a/sw/source/core/layout/objectformattertxtfrm.cxx +++ b/sw/source/core/layout/objectformattertxtfrm.cxx @@ -29,7 +29,6 @@ #include <fmtwrapinfluenceonobjpos.hxx> #include <fmtfollowtextflow.hxx> #include <layact.hxx> -#include <flyfrm.hxx> #include <ftnfrm.hxx> #include <osl/diagnose.h> @@ -230,7 +229,6 @@ bool SwObjectFormatterTextFrame::DoFormatObj( SwAnchoredObject& _rAnchoredObj, sal_uInt32 nToPageNum( 0 ); // #i43913# bool bDummy( false ); - bool bPageHasFlysAnchoredBelowThis(false); // see how SwObjectFormatter::FormatObjsAtFrame_() checks // "pPageFrameOfAnchor == &mrPageFrame" - only caller relevant for // this subclass @@ -238,8 +236,7 @@ bool SwObjectFormatterTextFrame::DoFormatObj( SwAnchoredObject& _rAnchoredObj, if ( SwObjectFormatterTextFrame::CheckMovedFwdCondition( *GetCollectedObj( nIdx ), GetPageFrame(), IsCollectedAnchoredAtMaster( nIdx ), - nToPageNum, bDummy, - bPageHasFlysAnchoredBelowThis)) + nToPageNum, bDummy)) { // #i49987# - consider, that anchor frame // could already been marked to move forward. @@ -251,10 +248,7 @@ bool SwObjectFormatterTextFrame::DoFormatObj( SwAnchoredObject& _rAnchoredObj, { if ( nMovedFwdToPageNum < nToPageNum ) { - if (!bPageHasFlysAnchoredBelowThis) - { - SwLayouter::RemoveMovedFwdFrame(rDoc, mrAnchorTextFrame); - } + SwLayouter::RemoveMovedFwdFrame(rDoc, mrAnchorTextFrame); } else bInsert = false; @@ -263,11 +257,7 @@ bool SwObjectFormatterTextFrame::DoFormatObj( SwAnchoredObject& _rAnchoredObj, { // Indicate that anchor text frame has to move forward and // invalidate its position to force a re-format. - if (!bPageHasFlysAnchoredBelowThis) - { - SwLayouter::InsertMovedFwdFrame(rDoc, - mrAnchorTextFrame, nToPageNum); - } + SwLayouter::InsertMovedFwdFrame(rDoc, mrAnchorTextFrame, nToPageNum); mrAnchorTextFrame.InvalidatePos(); // Indicate restart of the layout process @@ -369,14 +359,13 @@ bool SwObjectFormatterTextFrame::DoFormatObjs() sal_uInt32 nToPageNum( 0 ); // #i43913# bool bInFollow( false ); - bool bPageHasFlysAnchoredBelowThis(false); SwAnchoredObject* pObj = nullptr; if ( !mrAnchorTextFrame.IsFollow() ) { pObj = GetFirstObjWithMovedFwdAnchor( // #i35017# - constant name has changed text::WrapInfluenceOnPosition::ONCE_CONCURRENT, - nToPageNum, bInFollow, bPageHasFlysAnchoredBelowThis ); + nToPageNum, bInFollow ); } // #i35911# if ( pObj && pObj->HasClearedEnvironment() ) @@ -398,21 +387,15 @@ bool SwObjectFormatterTextFrame::DoFormatObjs() { if ( nTmpToPageNum < pAnchorPageFrame->GetPhyPageNum() ) { - if (!bPageHasFlysAnchoredBelowThis) - { - SwLayouter::RemoveMovedFwdFrame(rDoc, mrAnchorTextFrame); - } + SwLayouter::RemoveMovedFwdFrame(rDoc, mrAnchorTextFrame); } else bInsert = false; } if ( bInsert ) { - if (!bPageHasFlysAnchoredBelowThis) - { - SwLayouter::InsertMovedFwdFrame(rDoc, mrAnchorTextFrame, - pAnchorPageFrame->GetPhyPageNum()); - } + SwLayouter::InsertMovedFwdFrame(rDoc, mrAnchorTextFrame, + pAnchorPageFrame->GetPhyPageNum()); mrAnchorTextFrame.InvalidatePos(); bSuccess = false; InvalidatePrevObjs( *pObj ); @@ -532,8 +515,7 @@ void SwObjectFormatterTextFrame::InvalidateFollowObjs( SwAnchoredObject& _rAncho SwAnchoredObject* SwObjectFormatterTextFrame::GetFirstObjWithMovedFwdAnchor( const sal_Int16 _nWrapInfluenceOnPosition, sal_uInt32& _noToPageNum, - bool& _boInFollow, - bool& o_rbPageHasFlysAnchoredBelowThis) + bool& _boInFollow) { // #i35017# - constant names have changed OSL_ENSURE( _nWrapInfluenceOnPosition == text::WrapInfluenceOnPosition::ONCE_SUCCESSIVE || @@ -560,8 +542,7 @@ SwAnchoredObject* SwObjectFormatterTextFrame::GetFirstObjWithMovedFwdAnchor( if ( SwObjectFormatterTextFrame::CheckMovedFwdCondition( *GetCollectedObj( i ), GetPageFrame(), IsCollectedAnchoredAtMaster( i ), - _noToPageNum, _boInFollow, - o_rbPageHasFlysAnchoredBelowThis) ) + _noToPageNum, _boInFollow ) ) { pRetAnchoredObj = pAnchoredObj; break; @@ -572,40 +553,6 @@ SwAnchoredObject* SwObjectFormatterTextFrame::GetFirstObjWithMovedFwdAnchor( return pRetAnchoredObj; } -static SwRowFrame const* FindTopLevelRowFrame(SwFrame const*const pFrame) -{ - SwRowFrame * pRow = const_cast<SwFrame*>(pFrame)->FindRowFrame(); - if (!pRow) - return nullptr; - // looks like SwTabFrame has mbInfTab = true so go up 2 levels - while (pRow->GetUpper()->GetUpper()->IsInTab()) - { - pRow = pRow->GetUpper()->GetUpper()->FindRowFrame(); - } - return pRow; -} - -static SwContentFrame const* FindFrameInBody(SwAnchoredObject const& rAnchored) -{ - SwFrame const*const pAnchor(rAnchored.GetAnchorFrame()); - assert(pAnchor); - if (pAnchor->IsPageFrame() || pAnchor->FindFooterOrHeader()) - { - return nullptr; - } - if (pAnchor->IsInFly()) - { - return FindFrameInBody(*pAnchor->FindFlyFrame()); - } - if (pAnchor->IsInFootnote()) - { - return pAnchor->FindFootnoteFrame()->GetRef(); - } - assert(pAnchor->IsInDocBody()); - assert(pAnchor->IsContentFrame()); - return static_cast<SwContentFrame const*>(pAnchor); -} - // #i58182# // - replace private method by corresponding static public method bool SwObjectFormatterTextFrame::CheckMovedFwdCondition( @@ -613,8 +560,7 @@ bool SwObjectFormatterTextFrame::CheckMovedFwdCondition( SwPageFrame const& rFromPageFrame, const bool _bAnchoredAtMasterBeforeFormatAnchor, sal_uInt32& _noToPageNum, - bool& _boInFollow, - bool& o_rbPageHasFlysAnchoredBelowThis) + bool& _boInFollow) { const sal_uInt32 _nFromPageNum(rFromPageFrame.GetPhyPageNum()); bool bAnchorIsMovedForward( false ); @@ -691,70 +637,6 @@ bool SwObjectFormatterTextFrame::CheckMovedFwdCondition( } } - if (bAnchorIsMovedForward) - { - // tdf#138518 try to determine if there is a fly on page rFromPageFrame - // which is anchored in a frame that is "below" the anchor frame - // of _rAnchoredObj, such that it should move to the next page before - // _rAnchoredObj does - if (auto * pObjs = rFromPageFrame.GetSortedObjs()) - { - for (SwAnchoredObject *const pObj : *pObjs) - { - SwPageFrame const*const pObjAnchorPage(pObj->FindPageFrameOfAnchor()); - assert(pObjAnchorPage); - if ((pObjAnchorPage == &rFromPageFrame - ? _boInFollow // same-page but will move forward - : rFromPageFrame.GetPhyPageNum() < pObjAnchorPage->GetPhyPageNum()) - && pObj->GetFrameFormat()->GetAnchor().GetAnchorId() - != RndStdIds::FLY_AS_CHAR) - { - assert(pPageFrameOfAnchor); - if (pPageFrameOfAnchor->GetPhyPageNum() < pObjAnchorPage->GetPhyPageNum()) - { - SAL_INFO("sw.layout", "SwObjectFormatterTextFrame::CheckMovedFwdCondition(): o_rbPageHasFlysAnchoredBelowThis because next page"); - o_rbPageHasFlysAnchoredBelowThis = true; - break; - } - // on same page: check if it's in next-chain in the document body - // (in case both are in the same fly the flag must not be - // set because the whole fly moves at once) - SwContentFrame const*const pInBodyFrameObj(FindFrameInBody(*pObj)); - SwContentFrame const*const pInBodyFrameAnchoredObj(FindFrameInBody(_rAnchoredObj)); - if (pInBodyFrameObj && pInBodyFrameAnchoredObj) - { - bool isBreakMore(false); - // currently this ignores index of at-char flys - for (SwContentFrame const* pContentFrame = pInBodyFrameAnchoredObj->FindNextCnt(); - pContentFrame; - pContentFrame = pContentFrame->FindNextCnt()) - { - if (pInBodyFrameObj == pContentFrame) - { - // subsequent cells in a row are not automatically - // "below" and the row could potentially be split - // TODO refine check if needed - if (!pInBodyFrameAnchoredObj->IsInTab() - || FindTopLevelRowFrame(pInBodyFrameObj) - != FindTopLevelRowFrame(pInBodyFrameAnchoredObj)) - { // anchored in next chain on same page - SAL_INFO("sw.layout", "SwObjectFormatterTextFrame::CheckMovedFwdCondition(): o_rbPageHasFlysAnchoredBelowThis because next chain on same page"); - o_rbPageHasFlysAnchoredBelowThis = true; - isBreakMore = true; - } - break; - } - } - if (isBreakMore) - { - break; - } - } - } - } - } - } - return bAnchorIsMovedForward; } diff --git a/sw/source/core/layout/objectformattertxtfrm.hxx b/sw/source/core/layout/objectformattertxtfrm.hxx index 25a7a7e92b77..1785ba25a46f 100644 --- a/sw/source/core/layout/objectformattertxtfrm.hxx +++ b/sw/source/core/layout/objectformattertxtfrm.hxx @@ -96,8 +96,7 @@ class SwObjectFormatterTextFrame : public SwObjectFormatter SwAnchoredObject* GetFirstObjWithMovedFwdAnchor( const sal_Int16 _nWrapInfluenceOnPosition, sal_uInt32& _noToPageNum, - bool& _boInFollow, - bool& o_rbPageHasFlysAnchoredBelowThis); + bool& _boInFollow); /** method to format the anchor frame for checking of the move forward condition @@ -181,8 +180,7 @@ class SwObjectFormatterTextFrame : public SwObjectFormatter SwPageFrame const& rFromPageFrame, const bool _bAnchoredAtMasterBeforeFormatAnchor, sal_uInt32& _noToPageNum, - bool& _boInFollow, - bool& o_rbPageHasFlysAnchoredBelowThis); + bool& _boInFollow); }; #endif
