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 e7a8b8a154e359c8d04dbe2554d05af495e0ceb0
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 11:09:44 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.
(cherry picked from commit 2d0a4ef1d83b8de6cb133970c2c35ae706fb058e)
Conflicts:
sw/qa/core/text/text.cxx
sw/source/core/inc/txtfrm.hxx
Change-Id: I007b57b369f5c1e98ccad77111958dfd9335f079
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/153374
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
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 138db21fdc35..f5b8c9253ea4 100644
--- a/sw/qa/core/text/text.cxx
+++ b/sw/qa/core/text/text.cxx
@@ -964,6 +964,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 3c6dae427815..c1be26567abf 100644
--- a/sw/source/core/inc/txtfrm.hxx
+++ b/sw/source/core/inc/txtfrm.hxx
@@ -794,6 +794,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;
+
virtual void dumpAsXmlAttributes(xmlTextWriterPtr writer) const override;
};
diff --git a/sw/source/core/text/frmform.cxx b/sw/source/core/text/frmform.cxx
index 50b3ca91b20c..34405f99aee0 100644
--- a/sw/source/core/text/frmform.cxx
+++ b/sw/source/core/text/frmform.cxx
@@ -650,6 +650,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() )
{
@@ -2038,6 +2044,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 794d1f590967..1d0bea9d8fbe 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 596307e61384..d4ee29e86df0 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() ) )
{
