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 );
