dev/null                                                |binary
 sw/qa/core/layout/data/floattable-no-footer-overlap.doc |binary
 sw/qa/core/layout/flycnt.cxx                            |    9 ++
 sw/source/core/layout/flowfrm.cxx                       |   13 ++++
 sw/source/core/layout/tabfrm.cxx                        |   50 +++++++++++++++-
 5 files changed, 70 insertions(+), 2 deletions(-)

New commits:
commit 45574624ff05673d44f11cdbbbb49e1af599133e
Author:     Miklos Vajna <vmik...@collabora.com>
AuthorDate: Wed Jul 19 08:29:01 2023 +0200
Commit:     Miklos Vajna <vmik...@collabora.com>
CommitDate: Wed Jul 19 12:17:50 2023 +0200

    tdf#120262 sw floattable: no split when none of first row fits the vert 
space
    
    The final problem with the bugdoc is that half row of the second
    floating table was still on page 1, while it should be fully on page 2.
    
    The reason for this was that first we thought we can't move forward,
    since GetIndPrev() returns nullptr for a table that's inside a fly
    frame. Then we thought it's a good idea to split, even if the split
    would move the entire first row to the next page.
    
    Fix the problem by:
    
    - In SwTabFrame::MakeAll(), handle split flys after calling
      GetIndPrev(), an indirect prev of the anchor has the same meaning in
      this context.
    
    - In SwTabFrame::Split(), fail for split flys in case only half of the first
      row's first line would fit, which gives an opporunity to call
      MoveFwd().
    
    - In SwTabFrame::MakeAll(), call MoveFwd() in a way that it's not a
      problem that the GetIndPrev() call in MoveFwd() gives us a nullptr.
    
    - At this point we don't split the table, we move it forward, but an
      empty master remains on page 1. Fix that by improving
      SwFlowFrame::MoveSubTree() to mark empty flys for deletion, similar to
      how it does the same for sections.
    
    - Finally avoid a layout warning in SwTabFrame::MakeAll(), it's OK to
      try to split in case our split fly has a follow, we can move there on
      split failure.
    
    Also bring the test document closed to the original bugdoc, so we can
    assert the content on both pages.
    
    Change-Id: I2d3f88342d91b3e256bc41416a9afb274a9309d1
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/154633
    Reviewed-by: Miklos Vajna <vmik...@collabora.com>
    Tested-by: Jenkins

diff --git a/sw/qa/core/layout/data/floattable-no-footer-overlap.doc 
b/sw/qa/core/layout/data/floattable-no-footer-overlap.doc
new file mode 100644
index 000000000000..87e301189df5
Binary files /dev/null and 
b/sw/qa/core/layout/data/floattable-no-footer-overlap.doc differ
diff --git a/sw/qa/core/layout/data/floattable-no-footer-overlap.docx 
b/sw/qa/core/layout/data/floattable-no-footer-overlap.docx
deleted file mode 100644
index ca2f0d6d7244..000000000000
Binary files a/sw/qa/core/layout/data/floattable-no-footer-overlap.docx and 
/dev/null differ
diff --git a/sw/qa/core/layout/flycnt.cxx b/sw/qa/core/layout/flycnt.cxx
index 106be77ceac2..a1e4c05c6c1d 100644
--- a/sw/qa/core/layout/flycnt.cxx
+++ b/sw/qa/core/layout/flycnt.cxx
@@ -950,7 +950,7 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyAnchorKeepWithNext)
 CPPUNIT_TEST_FIXTURE(Test, testSplitFlyNoFooterOverlap)
 {
     // Given a document with 2 pages, a floating table on both pages:
-    createSwDoc("floattable-no-footer-overlap.docx");
+    createSwDoc("floattable-no-footer-overlap.doc");
 
     // When calculating the layout:
     calcLayout();
@@ -960,6 +960,13 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyNoFooterOverlap)
     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: 1
+    // - Actual  : 2
+    // i.e. part of the second table was on page 1.
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), rPage1Objs.size());
     auto pPage2 = dynamic_cast<SwPageFrame*>(pPage1->GetNext());
     // Without the accompanying fix in place, this test would have failed, 
there was no page 2, both
     // floating tables were on page 1.
diff --git a/sw/source/core/layout/flowfrm.cxx 
b/sw/source/core/layout/flowfrm.cxx
index fbcd6e14d159..f30c34938c20 100644
--- a/sw/source/core/layout/flowfrm.cxx
+++ b/sw/source/core/layout/flowfrm.cxx
@@ -693,6 +693,7 @@ void SwFlowFrame::MoveSubTree( SwLayoutFrame* pParent, 
SwFrame* pSibling )
     // disappear automatically.
     SwSectionFrame *pSct;
 
+    SwFlyFrame* pFly = nullptr;
     if ( pOldParent && !pOldParent->Lower() &&
          ( pOldParent->IsInSct() &&
            !(pSct = pOldParent->FindSctFrame())->ContainsContent() &&
@@ -700,6 +701,18 @@ void SwFlowFrame::MoveSubTree( SwLayoutFrame* pParent, 
SwFrame* pSibling )
     {
             pSct->DelEmpty( false );
     }
+    else if (pOldParent && !pOldParent->Lower()
+             && (pOldParent->IsInFly() && !(pFly = 
pOldParent->FindFlyFrame())->ContainsContent()
+                 && !pFly->ContainsAny()))
+    {
+        if (pFly->IsFlySplitAllowed())
+        {
+            // Master fly is empty now that we pasted the content to the 
follow, mark it for
+            // deletion.
+            auto pFlyAtContent = static_cast<SwFlyAtContentFrame*>(pFly);
+            pFlyAtContent->DelEmpty();
+        }
+    }
 
     // If we're in a column section, we'd rather not call Calc "from below"
     if( !m_rThis.IsInSct() &&
diff --git a/sw/source/core/layout/tabfrm.cxx b/sw/source/core/layout/tabfrm.cxx
index a7e767a00e3a..7ce35fd62512 100644
--- a/sw/source/core/layout/tabfrm.cxx
+++ b/sw/source/core/layout/tabfrm.cxx
@@ -1099,6 +1099,13 @@ bool SwTabFrame::Split( const SwTwips nCutPos, bool 
bTryToSplit, bool bTableRowK
             if (nMinHeight > nRemainingSpaceForLastRow)
             {
                 bSplitRowAllowed = false;
+
+                if (!pRow->GetPrev() && 
aRectFnSet.GetHeight(pRow->getFrameArea()) > nRemainingSpaceForLastRow)
+                {
+                    // Split of pRow is not allowed, no previous row, the 
current row doesn't fit:
+                    // that's a failure, we'll have to move forward instead.
+                    return false;
+                }
             }
         }
     }
@@ -2498,6 +2505,19 @@ void SwTabFrame::MakeAll(vcl::RenderContext* 
pRenderContext)
         // 2. If this row wants to keep, we need an additional row
         // 3. The table is allowed to split or we do not have a pIndPrev:
         SwFrame* pIndPrev = GetIndPrev();
+
+        SwFlyFrame* pFly = FindFlyFrame();
+        if (!pIndPrev && pFly && pFly->IsFlySplitAllowed())
+        {
+            auto pFlyAtContent = static_cast<SwFlyAtContentFrame*>(pFly);
+            SwFrame* pAnchor = pFlyAtContent->FindAnchorCharFrame();
+            if (pAnchor)
+            {
+                // If the anchor of the split has a previous frame, we're 
allowed to move forward.
+                pIndPrev = pAnchor->GetIndPrev();
+            }
+        }
+
         const SwRowFrame* pFirstNonHeadlineRow = GetFirstNonHeadlineRow();
         // #i120016# if this row wants to keep, allow split in case that all 
rows want to keep with next,
         // the table can not move forward as it is the first one and a split 
is in general allowed.
@@ -2752,11 +2772,23 @@ void SwTabFrame::MakeAll(vcl::RenderContext* 
pRenderContext)
         //Let's see if we find some place anywhere...
         if (!bMovedFwd)
         {
+            bool bMoveAlways = false;
+            SwFrame* pUpper = GetUpper();
+            if (pUpper && pUpper->IsFlyFrame())
+            {
+                auto pFlyFrame = static_cast<SwFlyFrame*>(pUpper);
+                if (pFlyFrame->IsFlySplitAllowed())
+                {
+                    // If the anchor of the split has a previous frame, 
MoveFwd() is allowed to move
+                    // forward.
+                    bMoveAlways = true;
+                }
+            }
             // don't make the effort to move fwd if its known
             // conditions that are known not to work
             if (IsInFootnote() && ForbiddenForFootnoteCntFwd())
                 bMakePage = false;
-            else if (!MoveFwd(bMakePage, false))
+            else if (!MoveFwd(bMakePage, false, bMoveAlways))
                 bMakePage = false;
         }
 
@@ -2804,6 +2836,22 @@ void SwTabFrame::MakeAll(vcl::RenderContext* 
pRenderContext)
             // allowed to split.
             SwTwips nDistToUpperPrtBottom =
                 aRectFnSet.BottomDist( getFrameArea(), 
aRectFnSet.GetPrtBottom(*GetUpper()));
+
+            if (GetUpper()->IsFlyFrame())
+            {
+                SwFlyFrame* pFlyFrame = GetUpper()->FindFlyFrame();
+                if (pFlyFrame->IsFlySplitAllowed())
+                {
+                    SwTextFrame* pAnchor = pFlyFrame->FindAnchorCharFrame();
+                    if (pAnchor && pAnchor->HasFollow())
+                    {
+                        // The split fly's anchor has a follow frame, we can 
move there & try to
+                        // split again.
+                        bTryToSplit = true;
+                    }
+                }
+            }
+
             if ( nDistToUpperPrtBottom >= 0 || bTryToSplit )
             {
                 lcl_RecalcTable( *this, nullptr, aNotify );

Reply via email to