sw/qa/core/text/data/floattable-anchor-next-page.docx |binary
sw/qa/core/text/text.cxx | 23 ++++++++++++++
sw/source/core/inc/txtfrm.hxx | 4 ++
sw/source/core/text/frmform.cxx | 18 +++++++++++
sw/source/core/text/itratr.cxx | 26 ++++++++++++++++
sw/source/core/text/widorp.cxx | 28 ++++++++++++++++++
6 files changed, 99 insertions(+)
New commits:
commit 3ba4bbcd13dda662832de8cb603b725a66cb53f0
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Tue Jun 20 08:50:57 2023 +0200
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Wed Jun 21 08:25:04 2023 +0200
sw floattable: fix negative vertical offset handling on page boundary
The bugdoc has 3 floating tables, the last one was on page 1 in Word,
but it was on page 2 in Writer.
It seems what happens is that the vertical offset of the last table is
negative, so it is moved above the paragraph before the last floating
table, but once the anchor frame (last paragraph) is moved to a new page
(because it doesn't fit), then its fly frame is also moved to page 2,
which leads to overlapping text in the original bugdoc. Interesting this
works already with 0 vertical offset, and in that case we split the last
paragraph, fill the page 1 part with a fly portion and fill the page 2
part with the anchor text.
Fix the problem by:
- triggering a split of the frame in SwTextFrameBreak::IsBreakNow(): if
the anchor frame doesn't fit (has to be moved to a next page), then
split it, so only the anchor text is moved, the fly is not (so its
position matches Word)
- preventing the manipulation of the frame offset in
SwTextFrame::AdjustFollow_(), so no content is moved from the follow
to the parent, because that would mean later we move the joined frame to
the next page
- finally minimizing the frame height at the end of
SwTextFrame::Format(), so the master still fits the current page
An alternative approach would be to extend
SwAnchoredObject::FindAnchorCharFrame(), which already has code to
handle the case when the text frame master and the anchored object is
not on the same page, but that operates on existing anchor frames, and
here the original problem is that the entire anchor frame is moved to
page 2, so we don't have anything left on page 1. Note that this is all
specific to floating tables, I could not reproduce the same behavior
with an anchored shape in Word.
Change-Id: I007b57b369f5c1e98ccad77111958dfd9335f079
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/153309
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
Tested-by: Jenkins
Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/153366
diff --git a/sw/qa/core/text/data/floattable-anchor-next-page.docx
b/sw/qa/core/text/data/floattable-anchor-next-page.docx
new file mode 100644
index 000000000000..898c5514c587
Binary files /dev/null and
b/sw/qa/core/text/data/floattable-anchor-next-page.docx differ
diff --git a/sw/qa/core/text/text.cxx b/sw/qa/core/text/text.cxx
index 4a08b0683e3c..c638e0f4be0c 100644
--- a/sw/qa/core/text/text.cxx
+++ b/sw/qa/core/text/text.cxx
@@ -1309,6 +1309,29 @@ CPPUNIT_TEST_FIXTURE(SwCoreTextTest,
testFloattableOverlap)
CPPUNIT_ASSERT(!rRect1.Overlaps(rRect2));
}
+CPPUNIT_TEST_FIXTURE(SwCoreTextTest, testFloattableAnchorNextPage)
+{
+ // Given a document with 3 floating tables, the last one has a negative
vertical offset, so the
+ // floating table is on page 1, but its anchor frame is effectively on
page 2:
+ createSwDoc("floattable-anchor-next-page.docx");
+
+ // When laying out that document:
+ calcLayout();
+
+ // Then make sure all 3 floating tables are on page 1:
+ SwDoc* pDoc = getSwDoc();
+ SwRootFrame* pLayout = pDoc->getIDocumentLayoutAccess().GetCurrentLayout();
+ auto pPage1 = dynamic_cast<SwPageFrame*>(pLayout->Lower());
+ CPPUNIT_ASSERT(pPage1);
+ CPPUNIT_ASSERT(pPage1->GetSortedObjs());
+ const SwSortedObjs& rPage1Objs = *pPage1->GetSortedObjs();
+ // Without the accompanying fix in place, this test would have failed with:
+ // - Expected: 3
+ // - Actual : 2
+ // i.e. the last floating table was on the wrong page (page 2).
+ CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(3), rPage1Objs.size());
+}
+
CPPUNIT_TEST_FIXTURE(SwCoreTextTest, testParaUpperMarginFlyIntersect)
{
// Given a document with 2 paragraphs, the paragraphs have both upper and
lower spacing of 567
diff --git a/sw/source/core/inc/txtfrm.hxx b/sw/source/core/inc/txtfrm.hxx
index 40cc0ef0bfac..512579c2d083 100644
--- a/sw/source/core/inc/txtfrm.hxx
+++ b/sw/source/core/inc/txtfrm.hxx
@@ -799,6 +799,10 @@ public:
/// a follow, i.e. not the last in a master -> follow 1 -> ... -> last
follow chain?
bool HasNonLastSplitFlyDrawObj() const;
+ /// This text frame has a follow and the text frame don't contain text.
Additionally one split
+ /// fly is anchored to the text frame.
+ bool IsEmptyMasterWithSplitFly() const;
+
static SwView* GetView();
virtual void dumpAsXmlAttributes(xmlTextWriterPtr writer) const override;
diff --git a/sw/source/core/text/frmform.cxx b/sw/source/core/text/frmform.cxx
index c90a12a2d011..36a45f1b13b6 100644
--- a/sw/source/core/text/frmform.cxx
+++ b/sw/source/core/text/frmform.cxx
@@ -652,6 +652,12 @@ void SwTextFrame::AdjustFollow_( SwTextFormatter &rLine,
}
}
+ if (IsEmptyMasterWithSplitFly())
+ {
+ // A split fly is anchored to us, don't move content from the follow
frame to this one.
+ return;
+ }
+
// The Offset moved
if( GetFollow() )
{
@@ -2116,6 +2122,18 @@ void SwTextFrame::Format( vcl::RenderContext*
pRenderContext, const SwBorderAttr
{
pPre->InvalidatePos();
}
+
+ if (IsEmptyMasterWithSplitFly())
+ {
+ // A fly is anchored to us, reduce size, so we definitely
still fit the current
+ // page.
+ SwFrameAreaDefinition::FrameAreaWriteAccess aFrm(*this);
+ aRectFnSet.SetHeight(aFrm, 0);
+
+ SwFrameAreaDefinition::FramePrintAreaWriteAccess aPrt(*this);
+ aRectFnSet.SetTop(aPrt, 0);
+ aRectFnSet.SetHeight(aPrt, 0);
+ }
}
}
diff --git a/sw/source/core/text/itratr.cxx b/sw/source/core/text/itratr.cxx
index 61fb23368fbe..176e06c4490e 100644
--- a/sw/source/core/text/itratr.cxx
+++ b/sw/source/core/text/itratr.cxx
@@ -1520,6 +1520,32 @@ bool SwTextFrame::HasNonLastSplitFlyDrawObj() const
return false;
}
+bool SwTextFrame::IsEmptyMasterWithSplitFly() const
+{
+ if (!IsEmptyMaster())
+ {
+ return false;
+ }
+
+ if (!m_pDrawObjs || m_pDrawObjs->size() != 1)
+ {
+ return false;
+ }
+
+ SwFlyFrame* pFlyFrame = (*m_pDrawObjs)[0]->DynCastFlyFrame();
+ if (!pFlyFrame || !pFlyFrame->IsFlySplitAllowed())
+ {
+ return false;
+ }
+
+ if (mnOffset != GetFollow()->GetOffset())
+ {
+ return false;
+ }
+
+ return true;
+}
+
SwTwips SwTextNode::GetWidthOfLeadingTabs() const
{
SwTwips nRet = 0;
diff --git a/sw/source/core/text/widorp.cxx b/sw/source/core/text/widorp.cxx
index 553cc2fa2de9..72903502749e 100644
--- a/sw/source/core/text/widorp.cxx
+++ b/sw/source/core/text/widorp.cxx
@@ -37,6 +37,9 @@
#include <ftnfrm.hxx>
#include <pagefrm.hxx>
#include <IDocumentSettingAccess.hxx>
+#include <sortedobjs.hxx>
+#include <anchoredobject.hxx>
+#include <flyfrm.hxx>
#undef WIDOWTWIPS
@@ -237,6 +240,31 @@ bool SwTextFrameBreak::IsBreakNow( SwTextMargin &rLine )
bool bFirstLine = 1 == rLine.GetLineNr() && !rLine.GetPrev();
m_bBreak = true;
+
+ if (bFirstLine)
+ {
+ bool bFits = m_pFrame->getFrameArea().Bottom() <=
m_pFrame->GetUpper()->getFramePrintArea().Bottom();
+ if (!m_pFrame->IsFollow() && !bFits)
+ {
+ // This is a master that doesn't fit the current parent.
+ if (m_pFrame->GetDrawObjs() && m_pFrame->GetDrawObjs()->size()
== 1)
+ {
+ SwFlyFrame* pFlyFrame =
(*m_pFrame->GetDrawObjs())[0]->DynCastFlyFrame();
+ if (pFlyFrame && pFlyFrame->IsFlySplitAllowed())
+ {
+ // It has a split fly anchored to it.
+ if
(pFlyFrame->GetFrameFormat().GetVertOrient().GetPos() < 0)
+ {
+ // Negative vertical offset means that visually it
already has a
+ // previous line. Consider that, otherwise the
split fly would move to
+ // the next page as well, which is not wanted.
+ bFirstLine = false;
+ }
+ }
+ }
+ }
+ }
+
if( ( bFirstLine && m_pFrame->GetIndPrev() )
|| ( rLine.GetLineNr() <= rLine.GetDropLines() ) )
{
