sw/qa/core/text/data/full-page-shape-wrap.docx |binary sw/qa/core/text/frmform.cxx | 40 ++++++++++++++++ sw/source/core/text/frmform.cxx | 60 +++++++++++++++++++++++++ 3 files changed, 100 insertions(+)
New commits: commit aa291933b41c90e93e40063c59f39e275e78829a Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Wed Aug 28 11:49:26 2024 +0200 Commit: Michael Stahl <michael.st...@allotropia.de> CommitDate: Thu Aug 29 11:07:33 2024 +0200 tdf#162662 sw: fix layout loop with full-page anchored shape that wants to wrap The bugdoc has a full-page group shape on page 2, with wrap type set to "above and below", which is Writer's "no wrap" type. Open this document in Writer, the layout loops before showing the first page. What seems to happen is that SwTextFrame::FormatAdjust() splits the anchor of the group shape at position 0, splits, then SwFlowFrame::MoveFwd() moves it to the next page. Now page 2 is empty, so later SwFlowFrame::MoveBwd() moves the content back from page 3 -> loop. Fix the problem by checking for this "we would split at para start due to full-page shape" case and in case wrapping is requested, but at the same time to size is so large that it can't be provided, then don't try to split as we'll join later anyway, so it's pointless. This matches Word behavior, which also doesn't try to move the body text of page 2 to a 3rd page in this conflicting case. An alternative would be to not tweak the "should we split" logic, but instead change the layout to assume "wrap through" instead of "no wrap" in this case. Fixing this in SwTextFrame::FormatAdjust() has the advantage that the i#84870 problem (similar, but for as-char images) was also fixed here, so this looks more consistent. Change-Id: Icfb58183c89a32b5f97c901d17cfae0fcc4e711f Reviewed-on: https://gerrit.libreoffice.org/c/core/+/172523 Reviewed-by: Miklos Vajna <vmik...@collabora.com> Tested-by: Jenkins (cherry picked from commit 5bd78c7dc4d17378c31207e6640e4d141d2ef5c3) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/172498 Reviewed-by: Michael Stahl <michael.st...@allotropia.de> diff --git a/sw/qa/core/text/data/full-page-shape-wrap.docx b/sw/qa/core/text/data/full-page-shape-wrap.docx new file mode 100644 index 000000000000..edb585d9bbd7 Binary files /dev/null and b/sw/qa/core/text/data/full-page-shape-wrap.docx differ diff --git a/sw/qa/core/text/frmform.cxx b/sw/qa/core/text/frmform.cxx index 4a81c1937cec..b73497f42b33 100644 --- a/sw/qa/core/text/frmform.cxx +++ b/sw/qa/core/text/frmform.cxx @@ -149,6 +149,46 @@ CPPUNIT_TEST_FIXTURE(Test, testFloattableBadFlyPos) CPPUNIT_ASSERT(pPage4->GetSortedObjs()); CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), pPage4->GetSortedObjs()->size()); } + +CPPUNIT_TEST_FIXTURE(Test, testFullPageShapeWrap) +{ + // Given a conflicting doc model: full page shape wants to wrap but leaves no space for the body + // text: + // When loading this document: + // Without the accompanying fix in place, this test would have resulted in a layout loop: + // content on page 2 did not fit the space, so move to page 3, then page 2 was empty, so we + // moved back -> loop. + createSwDoc("full-page-shape-wrap.docx"); + + // Check the import result: + SwDoc* pDoc = getSwDoc(); + SwRootFrame* pLayout = pDoc->getIDocumentLayoutAccess().GetCurrentLayout(); + // No anchored objects on page 1: + auto pPage1 = pLayout->Lower()->DynCastPageFrame(); + CPPUNIT_ASSERT(pPage1); + CPPUNIT_ASSERT(!pPage1->GetSortedObjs()); + // One group shape on page 2, and several fly frames (visually) inside that: + auto pPage2 = pPage1->GetNext()->DynCastPageFrame(); + CPPUNIT_ASSERT(pPage2); + CPPUNIT_ASSERT(pPage2->GetSortedObjs()); + int nFlyCount = 0; + int nDrawCount = 0; + for (const auto& pAnchoredObject : *pPage2->GetSortedObjs()) + { + if (pAnchoredObject->GetFrameFormat()->Which() == RES_FLYFRMFMT) + { + ++nFlyCount; + } + else + { + ++nDrawCount; + } + } + CPPUNIT_ASSERT_EQUAL(1, nDrawCount); + CPPUNIT_ASSERT_GREATER(1, nFlyCount); + // No page 3. + CPPUNIT_ASSERT(!pPage2->GetNext()); +} } /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/source/core/text/frmform.cxx b/sw/source/core/text/frmform.cxx index 9ce1475a3b1b..82a0f8c7dc14 100644 --- a/sw/source/core/text/frmform.cxx +++ b/sw/source/core/text/frmform.cxx @@ -52,6 +52,7 @@ #include <flyfrms.hxx> #include <frmtool.hxx> #include <layouter.hxx> +#include <fmtsrnd.hxx> // Tolerance in formatting and text output #define SLOPPY_TWIPS 5 @@ -1134,6 +1135,57 @@ static bool isReallyEmptyMaster(const SwTextFrame* pFrame) return pFrame->IsEmptyMaster() && (!pFrame->GetDrawObjs() || !pFrame->GetDrawObjs()->size()); } +namespace +{ +/// Determines if pFrame has at least one anchored object which is positioned against the page frame +/// and uses all space available for body text. +bool HasFullPageFly(const SwTextFrame* pFrame) +{ + const SwFrame* pBodyFrame = pFrame->FindBodyFrame(); + if (!pBodyFrame) + { + // Inside a fly frame, not interesting. + return false; + } + + const SwRect& rBodyFrameArea = pBodyFrame->getFrameArea(); + const SwSortedObjs* pDrawObjs = pFrame->GetDrawObjs(); + if (!pDrawObjs) + { + return false; + } + + for (SwAnchoredObject* pDrawObj : *pDrawObjs) + { + SwFrameFormat* pFrameFormat = pDrawObj->GetFrameFormat(); + if (pFrameFormat->GetHoriOrient().GetRelationOrient() != text::RelOrientation::PAGE_FRAME) + { + continue; + } + + if (pFrameFormat->GetVertOrient().GetRelationOrient() != text::RelOrientation::PAGE_FRAME) + { + continue; + } + + if (pFrameFormat->GetSurround().GetValue() != text::WrapTextMode::WrapTextMode_NONE) + { + // Not a case where the request is to wrap the content around the object, ignore. + continue; + } + + if (pDrawObj->GetObjRectWithSpaces().Contains(rBodyFrameArea)) + { + // Wrap is requested, but the object uses all available space: this is a full page + // object. + return true; + } + } + + return false; +} +} + void SwTextFrame::FormatAdjust( SwTextFormatter &rLine, WidowsAndOrphans &rFrameBreak, TextFrameIndex const nStrLen, @@ -1214,6 +1266,14 @@ void SwTextFrame::FormatAdjust( SwTextFormatter &rLine, } } + if (nNew && HasFullPageFly(this) && nEnd == TextFrameIndex(0) && !bEmptyWithSplitFly) + { + // We intended to split at start, due to an anchored object which would use all space on the + // current page. It makes no sense to split & move all text of the frame forward: the + // current page would be empty and we would move back later anyway. + nNew = 0; + } + if ( nNew ) { SplitFrame( nEnd );