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.)

Change-Id: I3a3f1efe6d7ed28e05dc159a86abc3d702cc272b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/105810
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@cib.de>

sw/qa/extras/layout/layout2.cxx

@@ -1937,6 +1937,27 @@ CPPUNIT_TEST_FIXTURE(SwLayoutWriter2, testTdf138039)
     assertXPath(pXmlDoc, "/root/page[3]/body/txt[1]/anchored", 0);
 }
 
+CPPUNIT_TEST_FIXTURE(SwLayoutWriter2, 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_TEST_FIXTURE(SwLayoutWriter2, testShapeAllowOverlap)
 {
 // Need to find out why this fails on macOS and why this is unstable on Windows.

sw/source/core/inc/pagefrm.hxx

@@ -439,6 +439,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: */

sw/source/core/layout/pagechg.cxx

@@ -987,11 +987,66 @@ 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 - consider that drawing objects in
+        // header/footer are supported now.
+        bool bOnlySuperfluousObjs = true;
+        SwSortedObjs const& rObjs = *rPage.GetSortedObjs();
+        for (size_t i = 0; bOnlySuperfluousObjs && i < rObjs.size(); ++i)
+        {
+            // #i28701#
+            SwAnchoredObject* pAnchoredObj = rObjs[i];
+            // do not consider hidden objects
+            if ( rPage.GetFormat()->GetDoc()->getIDocumentDrawModelAccess().IsVisibleLayerId(
+                                pAnchoredObj->GetDrawObj()->GetLayer() ) &&
+                 !pAnchoredObj->GetAnchorFrame()->FindFooterOrHeader() )
+            {
+                bOnlySuperfluousObjs = false;
+            }
+        }
+        bExistEssentialObjs = !bOnlySuperfluousObjs;
+    }
+
+    // optimization: check first if essential objects exist.
+    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
@@ -1029,7 +1084,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();
@@ -1126,6 +1184,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.
             {
@@ -1169,16 +1228,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)
@@ -1438,44 +1502,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 (!sw::IsPageFrameEmpty(*pPage))
-        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 bOnlySuperfluousObjs = true;
-            SwSortedObjs &rObjs = *pPage->GetSortedObjs();
-            for ( size_t i = 0; bOnlySuperfluousObjs && 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() )
-                {
-                    bOnlySuperfluousObjs = false;
-                }
-            }
-            bExistEssentialObjs = !bOnlySuperfluousObjs;
-        }
-
-        // 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 ( pPage->IsFootnotePage() )
             {

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>
 
@@ -1207,6 +1208,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 );
             }