sw/qa/extras/layout/data/tdf134298.ott |binary
sw/qa/extras/layout/data/tdf138039.odt |binary
sw/qa/extras/layout/layout.cxx | 45 +++++++++++++
sw/source/core/inc/pagefrm.hxx | 3
sw/source/core/layout/layact.cxx | 20 +++---
sw/source/core/layout/pagechg.cxx | 108 ++++++++++++++++++++-------------
sw/source/core/layout/tabfrm.cxx | 36 ++++++++++-
sw/source/core/layout/wsfrm.cxx | 16 ++++
8 files changed, 177 insertions(+), 51 deletions(-)
New commits:
commit 72669b98cf2c71fb0006b2d100248e8234b1b61b
Author: Michael Stahl <michael.st...@cib.de>
AuthorDate: Fri Nov 13 20:52:28 2020 +0100
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Fri Oct 8 15:03:04 2021 +0200
tdf#134298 sw: layout: remove left-over page frame without content
Once tdf#138039 is fixed, this bugdoc has an additional empty page 3.
This is because it first goes to 3 pages, and then the SwTextFrame
on page does a MoveBwd, leaving behind a page frame with just a body
frame and nothing else.
It turns out that SwRootFrame::RemoveSuperfluous() only removes
empty frames at the end of the document, but here there's a non-empty
frame following it. Also, this function doesn't handle cases like
right/left page styles so it can't delete pages in the middle.
SwFrame::CheckPageDescs() doesn't remove page frames that don't have
content, it only removes those that have the intentionally-empty flag set.
Extend CheckPageDescs() to also remove page frames that don't have
content, and make sure it is called when SwContentFrame::Cut()
removes the last content from a page frame (it will be called after
all pages are valid in SwLayAction::InternalAction()).
(Alternatively it might be possible to prevent the problem from
occurring in SwTextFly::ForEach() by ignoring the fly so that the first
paragraph never leaves page 1, but we didn't explore that.)
(cherry picked from commit b9ef71476fd70bc13f50ebe80390e0730d1b7afb)
Conflicts:
sw/qa/extras/layout/layout2.cxx
sw/source/core/layout/pagechg.cxx
Change-Id: I3a3f1efe6d7ed28e05dc159a86abc3d702cc272b
diff --git a/sw/qa/extras/layout/data/tdf134298.ott
b/sw/qa/extras/layout/data/tdf134298.ott
new file mode 100644
index 000000000000..effb595eb328
Binary files /dev/null and b/sw/qa/extras/layout/data/tdf134298.ott differ
diff --git a/sw/qa/extras/layout/layout.cxx b/sw/qa/extras/layout/layout.cxx
index b8dd065967c4..cb873bbf59b6 100644
--- a/sw/qa/extras/layout/layout.cxx
+++ b/sw/qa/extras/layout/layout.cxx
@@ -4259,6 +4259,27 @@ CPPUNIT_TEST_FIXTURE(SwLayoutWriter, testTdf138039)
assertXPath(pXmlDoc, "/root/page[3]/body/txt[1]/anchored", 0);
}
+CPPUNIT_TEST_FIXTURE(SwLayoutWriter, testTdf134298)
+{
+ createDoc("tdf134298.ott");
+
+ xmlDocUniquePtr pXmlDoc = parseLayoutDump();
+
+ // there are 2 pages
+ assertXPath(pXmlDoc, "/root/page", 2);
+ // table and first para on first page
+ assertXPath(pXmlDoc, "/root/page[1]/body/tab", 1);
+ assertXPath(pXmlDoc, "/root/page[1]/body/txt", 1);
+ assertXPath(pXmlDoc, "/root/page[1]/body/txt[1]/anchored", 0);
+ // paragraph with large fly on second page
+ assertXPath(pXmlDoc, "/root/page[2]/body/tab", 0);
+ assertXPath(pXmlDoc, "/root/page[2]/body/txt", 1);
+ assertXPath(pXmlDoc, "/root/page[2]/body/txt[1]/anchored/fly", 1);
+ assertXPath(pXmlDoc,
"/root/page[2]/body/txt[1]/anchored/fly[1]/infos/bounds", "top", "17897");
+ assertXPath(pXmlDoc,
"/root/page[2]/body/txt[1]/anchored/fly[1]/infos/bounds", "height",
+ "15819");
+}
+
CPPUNIT_PLUGIN_IMPLEMENT();
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/inc/pagefrm.hxx b/sw/source/core/inc/pagefrm.hxx
index 3c7f29f52ad0..c8101ef3157b 100644
--- a/sw/source/core/inc/pagefrm.hxx
+++ b/sw/source/core/inc/pagefrm.hxx
@@ -438,6 +438,9 @@ SwTextGridItem const* GetGridItem(SwPageFrame const*const);
sal_uInt16 GetGridWidth(SwTextGridItem const&, SwDoc const&);
+namespace sw { bool IsPageFrameEmpty(SwPageFrame const& rPage); }
+
+
#endif // INCLUDED_SW_SOURCE_CORE_INC_PAGEFRM_HXX
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/layout/pagechg.cxx
b/sw/source/core/layout/pagechg.cxx
index a3188eb2a5ca..e119ee4dca08 100644
--- a/sw/source/core/layout/pagechg.cxx
+++ b/sw/source/core/layout/pagechg.cxx
@@ -991,11 +991,67 @@ void SwPageFrame::PrepareRegisterChg()
}
}
+namespace sw {
+
+/// check if there's content on the page that requires it to exist
+bool IsPageFrameEmpty(SwPageFrame const& rPage)
+{
+ bool bExistEssentialObjs = ( nullptr != rPage.GetSortedObjs() );
+ if ( bExistEssentialObjs )
+ {
+ // Only because the page has Flys does not mean that it is needed. If
all Flys are
+ // attached to generic content it is also superfluous (checking
DocBody should be enough)
+ // OD 19.06.2003 #108784# - consider that drawing objects in
+ // header/footer are supported now.
+ bool bOnlySuperfluosObjs = true;
+ const SwSortedObjs &rObjs = *rPage.GetSortedObjs();
+ for ( size_t i = 0; bOnlySuperfluosObjs && i < rObjs.size(); ++i )
+ {
+ // #i28701#
+ SwAnchoredObject* pAnchoredObj = rObjs[i];
+ // OD 2004-01-19 #110582# - do not consider hidden objects
+ if (
rPage.GetFormat()->GetDoc()->getIDocumentDrawModelAccess().IsVisibleLayerId(
+ pAnchoredObj->GetDrawObj()->GetLayer() ) &&
+ !pAnchoredObj->GetAnchorFrame()->FindFooterOrHeader() )
+ {
+ bOnlySuperfluosObjs = false;
+ }
+ }
+ bExistEssentialObjs = !bOnlySuperfluosObjs;
+ }
+
+ // OD 19.06.2003 #108784# - optimization: check first, if essential objects
+ // exists.
+ const SwLayoutFrame* pBody = nullptr;
+ if ( bExistEssentialObjs ||
+ rPage.FindFootnoteCont() ||
+ ( nullptr != ( pBody = rPage.FindBodyCont() ) &&
+ ( pBody->ContainsContent() ||
+ // #i47580#
+ // Do not delete page if there's an empty tabframe
+ // left. I think it might be correct to use ContainsAny()
+ // instead of ContainsContent() to cover the empty-table-case,
+ // but I'm not fully sure, since ContainsAny() also returns
+ // SectionFrames. Therefore I prefer to do it the safe way:
+ ( pBody->Lower() && pBody->Lower()->IsTabFrame() ) ) ) )
+ {
+ return false;
+ }
+ else
+ {
+ return true;
+ }
+}
+
+} // namespace sw
+
//FIXME: provide missing documentation
/** Check all pages (starting from the given one) if they use the appropriate
frame format.
*
* If "wrong" pages are found, try to fix this as simple as possible.
*
+ * Also delete pages that don't have content on them.
+ *
* @param pStart the page from where to start searching
* @param bNotifyFields
* @param ppPrev
@@ -1033,7 +1089,10 @@ void SwFrame::CheckPageDescs( SwPageFrame *pStart, bool
bNotifyFields, SwPageFra
SwPageFrame *pNextPage = static_cast<SwPageFrame*>(pPage->GetNext());
SwPageDesc *pDesc = pPage->FindPageDesc();
+ /// page is intentionally empty page
bool bIsEmpty = pPage->IsEmptyPage();
+ // false for intentionally empty pages, they need additional check
+ bool isPageFrameEmpty(!bIsEmpty && sw::IsPageFrameEmpty(*pPage));
bool bIsOdd = pPage->OnRightPage();
bool bWantOdd = pPage->WannaRightPage();
bool bFirst = pPage->OnFirstPage();
@@ -1130,6 +1189,7 @@ void SwFrame::CheckPageDescs( SwPageFrame *pStart, bool
bNotifyFields, SwPageFra
pTmp->Paste( pRoot, pPage );
pTmp->PreparePage( false );
pPage = pTmp;
+ isPageFrameEmpty = false; // don't delete it right away!
}
else if ( pPage->GetPageDesc() != pDesc ) //4.
{
@@ -1173,16 +1233,21 @@ void SwFrame::CheckPageDescs( SwPageFrame *pStart, bool
bNotifyFields, SwPageFra
}
#endif
}
- if ( bIsEmpty )
+ assert(!bIsEmpty || !isPageFrameEmpty);
+ if (bIsEmpty || isPageFrameEmpty)
{
// It also might be that an empty page is not needed at all.
// However, the algorithm above cannot determine that. It is not
needed if the following
// page can live without it. Do obtain that information, we need
to dig deeper...
SwPageFrame *pPg = static_cast<SwPageFrame*>(pPage->GetNext());
- if( !pPg || pPage->OnRightPage() == pPg->WannaRightPage() )
+ if (isPageFrameEmpty || !pPg || pPage->OnRightPage() ==
pPg->WannaRightPage())
{
// The following page can find a FrameFormat or has no
successor -> empty page not needed
SwPageFrame *pTmp =
static_cast<SwPageFrame*>(pPage->GetNext());
+ if (isPageFrameEmpty && pPage->GetPrev())
+ { // check previous *again* vs. its new next! see
"ooo321_stylepagenumber.odt"
+ pTmp = static_cast<SwPageFrame*>(pPage->GetPrev());
+ }
pPage->Cut();
bool bUpdatePrev = false;
if (ppPrev && *ppPrev == pPage)
@@ -1442,44 +1507,7 @@ void SwRootFrame::RemoveSuperfluous()
// Check the corresponding last page if it is empty and stop loop at the
last non-empty page.
do
{
- bool bExistEssentialObjs = ( nullptr != pPage->GetSortedObjs() );
- if ( bExistEssentialObjs )
- {
- // Only because the page has Flys does not mean that it is needed.
If all Flys are
- // attached to generic content it is also superfluous (checking
DocBody should be enough)
- // OD 19.06.2003 #108784# - consider that drawing objects in
- // header/footer are supported now.
- bool bOnlySuperfluosObjs = true;
- SwSortedObjs &rObjs = *pPage->GetSortedObjs();
- for ( size_t i = 0; bOnlySuperfluosObjs && i < rObjs.size(); ++i )
- {
- // #i28701#
- SwAnchoredObject* pAnchoredObj = rObjs[i];
- // OD 2004-01-19 #110582# - do not consider hidden objects
- if (
pPage->GetFormat()->GetDoc()->getIDocumentDrawModelAccess().IsVisibleLayerId(
- pAnchoredObj->GetDrawObj()->GetLayer() ) &&
- !pAnchoredObj->GetAnchorFrame()->FindFooterOrHeader() )
- {
- bOnlySuperfluosObjs = false;
- }
- }
- bExistEssentialObjs = !bOnlySuperfluosObjs;
- }
-
- // OD 19.06.2003 #108784# - optimization: check first, if essential
objects
- // exists.
- const SwLayoutFrame* pBody = nullptr;
- if ( bExistEssentialObjs ||
- pPage->FindFootnoteCont() ||
- ( nullptr != ( pBody = pPage->FindBodyCont() ) &&
- ( pBody->ContainsContent() ||
- // #i47580#
- // Do not delete page if there's an empty tabframe
- // left. I think it might be correct to use ContainsAny()
- // instead of ContainsContent() to cover the
empty-table-case,
- // but I'm not fully sure, since ContainsAny() also returns
- // SectionFrames. Therefore I prefer to do it the safe way:
- ( pBody->Lower() && pBody->Lower()->IsTabFrame() ) ) ) )
+ if (!sw::IsPageFrameEmpty(*pPage))
{
if ( pPage->IsFootnotePage() )
{
diff --git a/sw/source/core/layout/wsfrm.cxx b/sw/source/core/layout/wsfrm.cxx
index 57db1547a1c6..11d20243baf4 100644
--- a/sw/source/core/layout/wsfrm.cxx
+++ b/sw/source/core/layout/wsfrm.cxx
@@ -56,6 +56,7 @@
#include <sortedobjs.hxx>
#include <frmatr.hxx>
#include <frmtool.hxx>
+#include <layact.hxx>
#include <ndtxt.hxx>
#include <swtable.hxx>
@@ -1200,6 +1201,21 @@ void SwContentFrame::Cut()
if ( pRoot )
{
pRoot->SetSuperfluous();
+ // RemoveSuperfluous can only remove empty pages at the end;
+ // find if there are pages without content following pPage
+ // and if so request a call to CheckPageDescs()
+ SwPageFrame const* pNext(pPage);
+ if (pRoot->GetCurrShell()->Imp()->IsAction())
+ {
+ while ((pNext = static_cast<SwPageFrame
const*>(pNext->GetNext())))
+ {
+ if (!sw::IsPageFrameEmpty(*pNext) &&
!pNext->IsFootnotePage())
+ {
+
pRoot->GetCurrShell()->Imp()->GetLayAction().SetCheckPageNum(pPage->GetPhyPageNum());
+ break;
+ }
+ }
+ }
GetUpper()->SetCompletePaint();
GetUpper()->InvalidatePage( pPage );
}
commit a543433c730eac8f91b49c6a7993ae26e67d309b
Author: Michael Stahl <michael.st...@cib.de>
AuthorDate: Mon Nov 16 13:08:48 2020 +0100
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Fri Oct 8 15:03:02 2021 +0200
(related tdf#134298) sw: layout: avoid infinite loop in InternalAction()
The condition IsInterrupt() && pPage && (m_nCheckPageNum != USHRT_MAX)
isn't handled properly and the while loop will never terminate with
the fix for tdf#134298 in several UITest_writer_tests*.
If m_nCheckPageNum is set, then it must result in a call to
CheckPageDescs() here; it's a member of SwLayAction so won't survive
until the next idle layout invocation.
There is a funny history of these loop conditions with
commit 9eff9e699e17cc5a8a25895bd28dc8e4ceb8071e
and cee296066ab780217395201ab84c2150c8840d25 so we can only hope
this time we got it right...
(cherry picked from commit 094ee3955ee81e1bc631d50cc216cbb17a777839)
Change-Id: I91b63540bf4280296d747cb8e841594f8dd3b140
diff --git a/sw/source/core/layout/layact.cxx b/sw/source/core/layout/layact.cxx
index 7f913c8a02ff..d3dd8b62027d 100644
--- a/sw/source/core/layout/layact.cxx
+++ b/sw/source/core/layout/layact.cxx
@@ -447,15 +447,19 @@ void SwLayAction::InternalAction(OutputDevice*
pRenderContext)
sal_uInt16 nPercentPageNum = 0;
while ((!IsInterrupt() && pPage) || (m_nCheckPageNum != USHRT_MAX))
{
- if (!pPage && m_nCheckPageNum != USHRT_MAX)
+ // note: this is the only place that consumes and resets
m_nCheckPageNum
+ if ((IsInterrupt() || !pPage) && m_nCheckPageNum != USHRT_MAX)
{
- SwPageFrame *pPg = static_cast<SwPageFrame*>(m_pRoot->Lower());
- while (pPg && pPg->GetPhyPageNum() < m_nCheckPageNum)
- pPg = static_cast<SwPageFrame*>(pPg->GetNext());
- if (pPg)
- pPage = pPg;
- if (!pPage)
- break;
+ if (!pPage || m_nCheckPageNum < pPage->GetPhyPageNum())
+ {
+ SwPageFrame *pPg = static_cast<SwPageFrame*>(m_pRoot->Lower());
+ while (pPg && pPg->GetPhyPageNum() < m_nCheckPageNum)
+ pPg = static_cast<SwPageFrame*>(pPg->GetNext());
+ if (pPg)
+ pPage = pPg;
+ if (!pPage)
+ break;
+ }
SwPageFrame *pTmp = pPage->GetPrev() ?
static_cast<SwPageFrame*>(pPage->GetPrev()) : pPage;
commit f6405603547435442ad4b11a37c8cdc68b063f3e
Author: Michael Stahl <michael.st...@cib.de>
AuthorDate: Fri Nov 13 20:51:42 2020 +0100
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Fri Oct 8 15:03:00 2021 +0200
tdf#138039 tdf#134298 sw: layout: fix overlap of fly and table
The layout is horribly borked, the fly anchored in the body-level
paragraph messed with the preceding table:
page id="1" top="284" width="11905" height="16837" bottom="17120"
tab id="3" top="794"
row id="4" top="17121"
fly id="8" top="16725"
txt id="7" top="1394"
fly ptr="0x6ce5510" id="10" top="1302"
SwTabFrame::CalcFlyOffsets() detects an overlap with the large fly, and
since it has wrap NONE it resizes to below the large image.
Then the SwTabFrame doesn't fit on the page, so it is split, but the split
fails because nDistanceToUpperPrtBottom is -720 (negative); hence it is
joined again.
Meanwhile the fly was invalidated, so now CalcFlyOffsets() ignores it and
the table shrinks again.
Once the fly is positioned again, the process repeats from the start.
Fix this in SwTabFrame::CalcFlyOffsets() by ignoring flys with wrap NONE
that
extend below the body of the document and are anchored in a frame in the
next-chain of the table frame: these must move to the next page with their
anchor frame.
For the bugdoc this gives the same layout as LO 5.2.
Reportedly this problem started to happen since commit
6f5024de2e1a5cc533527e45b33d9a415467c48d, but it's not obvious why.
(cherry picked from commit 6b92d2e8522ecc98d2c5532f5076c20ae295168e)
Conflicts:
sw/qa/extras/layout/layout2.cxx
sw/source/core/layout/tabfrm.cxx
Change-Id: Iafb8a6afcba634f11c5db73869313ded0fe13bbd
diff --git a/sw/qa/extras/layout/data/tdf138039.odt
b/sw/qa/extras/layout/data/tdf138039.odt
new file mode 100644
index 000000000000..f355fd1349a6
Binary files /dev/null and b/sw/qa/extras/layout/data/tdf138039.odt differ
diff --git a/sw/qa/extras/layout/layout.cxx b/sw/qa/extras/layout/layout.cxx
index 44512e2ce8a2..b8dd065967c4 100644
--- a/sw/qa/extras/layout/layout.cxx
+++ b/sw/qa/extras/layout/layout.cxx
@@ -4235,6 +4235,30 @@ CPPUNIT_TEST_FIXTURE(SwLayoutWriter, testTdf134548)
}
}
+CPPUNIT_TEST_FIXTURE(SwLayoutWriter, testTdf138039)
+{
+ createDoc("tdf138039.odt");
+
+ xmlDocUniquePtr pXmlDoc = parseLayoutDump();
+
+ // there are 3 pages
+ assertXPath(pXmlDoc, "/root/page", 3);
+ // table on first page
+ assertXPath(pXmlDoc, "/root/page[1]/body/tab", 1);
+ assertXPath(pXmlDoc, "/root/page[1]/body/txt", 0);
+ // paragraph with large fly on second page
+ assertXPath(pXmlDoc, "/root/page[2]/body/tab", 0);
+ assertXPath(pXmlDoc, "/root/page[2]/body/txt", 1);
+ assertXPath(pXmlDoc, "/root/page[2]/body/txt[1]/anchored/fly", 1);
+ assertXPath(pXmlDoc,
"/root/page[2]/body/txt[1]/anchored/fly[1]/infos/bounds", "top", "17915");
+ assertXPath(pXmlDoc,
"/root/page[2]/body/txt[1]/anchored/fly[1]/infos/bounds", "height",
+ "15819");
+ // paragraph on third page
+ assertXPath(pXmlDoc, "/root/page[3]/body/tab", 0);
+ assertXPath(pXmlDoc, "/root/page[3]/body/txt", 1);
+ assertXPath(pXmlDoc, "/root/page[3]/body/txt[1]/anchored", 0);
+}
+
CPPUNIT_PLUGIN_IMPLEMENT();
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/layout/tabfrm.cxx b/sw/source/core/layout/tabfrm.cxx
index b89b21b9205a..009099250954 100644
--- a/sw/source/core/layout/tabfrm.cxx
+++ b/sw/source/core/layout/tabfrm.cxx
@@ -2706,6 +2706,21 @@ void SwTabFrame::MakeAll(vcl::RenderContext*
pRenderContext)
aNotify.SetInvaKeep();
}
+static bool IsNextOnSamePage(SwPageFrame const& rPage,
+ SwTabFrame const& rTabFrame, SwTextFrame const& rAnchorFrame)
+{
+ for (SwContentFrame const* pContentFrame = rTabFrame.FindNextCnt();
+ pContentFrame && pContentFrame->FindPageFrame() == &rPage;
+ pContentFrame = pContentFrame->FindNextCnt())
+ {
+ if (pContentFrame == &rAnchorFrame)
+ {
+ return true;
+ }
+ }
+ return false;
+}
+
/// Calculate the offsets arising because of FlyFrames
bool SwTabFrame::CalcFlyOffsets( SwTwips& rUpper,
long& rLeftOffset,
@@ -2835,10 +2850,25 @@ bool SwTabFrame::CalcFlyOffsets( SwTwips& rUpper,
if (bShiftDown)
{
+ // possible cases:
+ // both in body
+ // both in same fly
+ // any comb. of body, footnote, header/footer
+ // to keep it safe, check only in doc body vs page
margin for now
long nBottom = aRectFnSet.GetBottom(aFlyRect);
- if( aRectFnSet.YDiff( nPrtPos, nBottom ) < 0 )
- nPrtPos = nBottom;
- bInvalidatePrtArea = true;
+ // tdf#138039 don't grow beyond the page body
+ // if the fly is anchored below the table; the fly
+ // must move with its anchor frame to the next page
+ SwRectFnSet fnPage(pPage);
+ if (!IsInDocBody() // TODO
+ || fnPage.YDiff(fnPage.GetBottom(aFlyRect),
fnPage.GetPrtBottom(*pPage)) <= 0
+ || !IsNextOnSamePage(*pPage, *this,
+
*static_cast<SwTextFrame*>(pFly->GetAnchorFrameContainingAnchPos())))
+ {
+ if (aRectFnSet.YDiff( nPrtPos, nBottom ) < 0)
+ nPrtPos = nBottom;
+ bInvalidatePrtArea = true;
+ }
}
if ( (css::text::WrapTextMode_RIGHT ==
rSur.GetSurround() ||
css::text::WrapTextMode_PARALLEL ==
rSur.GetSurround())&&
