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 ecdeed57693617263a88bdff87f11622b94e6889 Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Mon Jul 3 08:30:21 2023 +0200 Commit: Caolán McNamara <caolan.mcnam...@collabora.com> CommitDate: Wed Jul 5 15:15:41 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). (cherry picked from commit 16b5b21d36da87be9b50235acbbb8008ed23b8bb) Conflicts: sw/source/core/inc/txtfrm.hxx Change-Id: Ie64ce92bf19b3dee8059fa14294d7599b41cc53f Reviewed-on: https://gerrit.libreoffice.org/c/core/+/154018 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Tested-by: Caolán McNamara <caolan.mcnam...@collabora.com> Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com> 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 c1be26567abf..9fd305c5562d 100644 --- a/sw/source/core/inc/txtfrm.hxx +++ b/sw/source/core/inc/txtfrm.hxx @@ -798,6 +798,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; + virtual void dumpAsXmlAttributes(xmlTextWriterPtr writer) const override; }; diff --git a/sw/source/core/text/frmform.cxx b/sw/source/core/text/frmform.cxx index 34405f99aee0..231825ff89e6 100644 --- a/sw/source/core/text/frmform.cxx +++ b/sw/source/core/text/frmform.cxx @@ -1082,6 +1082,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 1d0bea9d8fbe..5de5467f3075 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 42b40cbe5a84..4bdc0fa40f77 100644 --- a/sw/source/core/text/porrst.cxx +++ b/sw/source/core/text/porrst.cxx @@ -437,6 +437,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 d4ee29e86df0..0c978b091c5c 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() )