sw/CppunitTest_sw_core_text.mk | 1
sw/qa/core/text/data/floattable-negative-vert-offset-empty.docx |binary
sw/qa/core/text/itratr.cxx | 58
++++++++++
sw/source/core/inc/txtfrm.hxx | 4
sw/source/core/text/frmform.cxx | 9 +
sw/source/core/text/itratr.cxx | 41 +++++++
sw/source/core/text/porrst.cxx | 7 +
sw/source/core/text/widorp.cxx | 25 ----
8 files changed, 124 insertions(+), 21 deletions(-)
New commits:
commit 16b5b21d36da87be9b50235acbbb8008ed23b8bb
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Mon Jul 3 08:30:21 2023 +0200
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Mon Jul 3 10:12:49 2023 +0200
sw floattable: fix negative vert offset with on page boundary without anch
text
The bugdoc has 3 floating tables on page 1, and the anchor of the last
floating table goes to page 2. All anchors are empty (no text), apart
from serving as an anchor for the floating tables. The last floating
table goes to page 2 in Writer, but not in Word.
The problem is quite similar to what commit
2d0a4ef1d83b8de6cb133970c2c35ae706fb058e (sw floattable: fix negative
vertical offset handling on page boundary, 2023-06-20) fixed already,
but here the anchor of the floating table is empty, and there it had
some text, which is a different codepath.
Fix the problem by first requesting a frame split in
SwTextFrame::FormatAdjust() even for empty paragraphs with flys (so the
fly part can go to page 1 and the empty paragraph can go to page 2) and
second disabling the SwTextFrame::FormatEmpty() optimization that would
assume no split is needed, while a split is required for a correct
layout here.
With this, the DOCX version of the original bnc#816603 document finally
renders without overlaps (4 pages, 11 floating tables).
Change-Id: Ie64ce92bf19b3dee8059fa14294d7599b41cc53f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/153866
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
Tested-by: Jenkins
diff --git a/sw/CppunitTest_sw_core_text.mk b/sw/CppunitTest_sw_core_text.mk
index 79ac4a0320bf..a04824e08e07 100644
--- a/sw/CppunitTest_sw_core_text.mk
+++ b/sw/CppunitTest_sw_core_text.mk
@@ -15,6 +15,7 @@ $(eval $(call
gb_CppunitTest_use_common_precompiled_header,sw_core_text))
$(eval $(call gb_CppunitTest_add_exception_objects,sw_core_text, \
sw/qa/core/text/frmform \
+ sw/qa/core/text/itratr \
sw/qa/core/text/text \
))
diff --git a/sw/qa/core/text/data/floattable-negative-vert-offset-empty.docx
b/sw/qa/core/text/data/floattable-negative-vert-offset-empty.docx
new file mode 100644
index 000000000000..132fde553825
Binary files /dev/null and
b/sw/qa/core/text/data/floattable-negative-vert-offset-empty.docx differ
diff --git a/sw/qa/core/text/itratr.cxx b/sw/qa/core/text/itratr.cxx
new file mode 100644
index 000000000000..6913d0037dbd
--- /dev/null
+++ b/sw/qa/core/text/itratr.cxx
@@ -0,0 +1,58 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <swmodeltestbase.hxx>
+
+#include <memory>
+
+#include <IDocumentLayoutAccess.hxx>
+#include <doc.hxx>
+#include <frame.hxx>
+#include <pagefrm.hxx>
+#include <rootfrm.hxx>
+#include <sortedobjs.hxx>
+
+namespace
+{
+/// Covers sw/source/core/text/itratr.cxx fixes.
+class Test : public SwModelTestBase
+{
+public:
+ Test()
+ : SwModelTestBase("/sw/qa/core/text/data/")
+ {
+ }
+};
+
+CPPUNIT_TEST_FIXTURE(Test, testFloattableNegativeVertOffsetEmptyAnchor)
+{
+ // Given a document with 3 floating tables, all of them on page 1, but the
anchor of the last
+ // floating table is on page 2, but that anchor has no own text:
+ createSwDoc("floattable-negative-vert-offset-empty.docx");
+
+ // When laying out that document:
+ calcLayout();
+
+ // Then make sure the that 3rd floating table is not shifted to page 2:
+ 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 shifted to page 2, which lead to
overlapping text in the
+ // original document.
+ CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(3), rPage1Objs.size());
+}
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/inc/txtfrm.hxx b/sw/source/core/inc/txtfrm.hxx
index 090f0bc0c340..ca1b34239055 100644
--- a/sw/source/core/inc/txtfrm.hxx
+++ b/sw/source/core/inc/txtfrm.hxx
@@ -803,6 +803,10 @@ public:
/// fly is anchored to the text frame.
bool IsEmptyMasterWithSplitFly() const;
+ /// This text frame is not split, doesn't fit the upper, has a single
split fly anchored to it
+ /// with a negative vertical offset. Such frames may need explicit
splitting.
+ bool IsEmptyWithSplitFly() 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 9398f28ad0ae..c2a280211a8f 100644
--- a/sw/source/core/text/frmform.cxx
+++ b/sw/source/core/text/frmform.cxx
@@ -1084,6 +1084,15 @@ void SwTextFrame::FormatAdjust( SwTextFormatter &rLine,
!rFrameBreak.IsInside( rLine ) )
: rFrameBreak.IsBreakNow( rLine ) ) ) )
? 1 : 0;
+
+ SwTextFormatInfo& rInf = rLine.GetInfo();
+ if (nNew == 0 && !nStrLen && !rInf.GetTextFly().IsOn() &&
IsEmptyWithSplitFly())
+ {
+ // Empty paragraph, so IsBreakNow() is not called, but we should split
the fly portion and
+ // the paragraph marker.
+ nNew = 1;
+ }
+
// i#84870
// no split of text frame, which only contains an as-character anchored
object
bool bOnlyContainsAsCharAnchoredObj =
diff --git a/sw/source/core/text/itratr.cxx b/sw/source/core/text/itratr.cxx
index 176e06c4490e..97425515b0a0 100644
--- a/sw/source/core/text/itratr.cxx
+++ b/sw/source/core/text/itratr.cxx
@@ -1546,6 +1546,47 @@ bool SwTextFrame::IsEmptyMasterWithSplitFly() const
return true;
}
+bool SwTextFrame::IsEmptyWithSplitFly() const
+{
+ if (IsFollow())
+ {
+ return false;
+ }
+
+ if (GetTextNodeFirst()->GetSwAttrSet().HasItem(RES_PAGEDESC))
+ {
+ return false;
+ }
+
+ if (getFrameArea().Bottom() <= GetUpper()->getFramePrintArea().Bottom())
+ {
+ return false;
+ }
+
+ // This is a master that doesn't fit the current parent.
+ if (!m_pDrawObjs || m_pDrawObjs->size() != 1)
+ {
+ return false;
+ }
+
+ SwFlyFrame* pFlyFrame = (*m_pDrawObjs)[0]->DynCastFlyFrame();
+ if (!pFlyFrame || !pFlyFrame->IsFlySplitAllowed())
+ {
+ return false;
+ }
+
+ // It has a split fly anchored to it.
+ if (pFlyFrame->GetFrameFormat().GetVertOrient().GetPos() >= 0)
+ {
+ return false;
+ }
+
+ // Negative vertical offset means that visually it already may have a
first line.
+ // Consider that, we may need to split the frame, so the fly frame is on
one page and the empty
+ // paragraph's frame is on a next page.
+ return true;
+}
+
SwTwips SwTextNode::GetWidthOfLeadingTabs() const
{
SwTwips nRet = 0;
diff --git a/sw/source/core/text/porrst.cxx b/sw/source/core/text/porrst.cxx
index 5c04b60b925e..255318119f87 100644
--- a/sw/source/core/text/porrst.cxx
+++ b/sw/source/core/text/porrst.cxx
@@ -439,6 +439,13 @@ bool SwTextFrame::FormatEmpty()
aTextFly.IsOn() && aTextFly.IsAnyObj( aRect ) &&
!bHasNonLastSplitFlyDrawObj )
return false;
+ if (IsEmptyWithSplitFly())
+ {
+ // We don't want this optimization in case the paragraph is not really
empty, because it has
+ // a fly frame and it also needs space for the empty paragraph in a
next line.
+ return false;
+ }
+
// only need to check one node because of early return on GetMerged()
for (SwContentIndex const* pIndex = GetTextNodeFirst()->GetFirstIndex();
pIndex; pIndex = pIndex->GetNext())
diff --git a/sw/source/core/text/widorp.cxx b/sw/source/core/text/widorp.cxx
index 72903502749e..8a66e40f8234 100644
--- a/sw/source/core/text/widorp.cxx
+++ b/sw/source/core/text/widorp.cxx
@@ -241,28 +241,11 @@ bool SwTextFrameBreak::IsBreakNow( SwTextMargin &rLine )
bool bFirstLine = 1 == rLine.GetLineNr() && !rLine.GetPrev();
m_bBreak = true;
- if (bFirstLine)
+ if (bFirstLine && m_pFrame->IsEmptyWithSplitFly())
{
- 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;
- }
- }
- }
- }
+ // Not really the first line, visually we may have a previous line
(including the fly
+ // frame) already.
+ bFirstLine = false;
}
if( ( bFirstLine && m_pFrame->GetIndPrev() )
