sw floattable: fix crash by trying harder to split tables

Regression from commit 60e2fdf1d7e8346e5a3835369c47e582c737ce20 (sw
floattable: maintain the invariant that fly height is at least MINFLY,
2023-09-28), the bugdoc crashed on load in SwTabFrame::MakeAll(),
because the tab frame's HasFollowFlowLine() was true, but
GetFollow()->GetFirstNonHeadlineRow() was nullptr and the invarint is
that these are always in sync.

Digging deeper, what happens is that the master table has a split row at
the end, so the follow table has a "follow flow line". We remove that
when we try to split the master table (split either moves rows to the
follow or creates a new follow), so the follow table only has a
"headline row" remaining. Then Split() is called with bTryToSplit set to
true, this fails (because only a single line would fit the master, but
orphan/widow control rejects that) and then we join the follow table
(because it only has headline rows), so a split with bTryToSplit set to
false (don't split the row itself) never happens. This at the end leads
to a strange table frame with only headline rows and gets deleted, which
is odd to happen during the initial layout.

Fix the problem by remembering if we just removed the follow flow line,
and in case we tried to split the rows itself and table split failed,
then don't join the follow table, so a next split can be invoked with
bTryToSplit set to false, which leads to the correct layout. This means
not only the crash is fixed, but also no layout loop happens and result
matches Word.

Limit this to tables in split flys, at least for this bugdoc the inline
table case would not have this problem as widow/orphan control is
disabled inside inline tables.

Change-Id: I172e38be11baf6f73df722a4c6c035a6a283d727
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159802
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins

3 files changed, 73 insertions, 1 deletions

diff --git a/sw/qa/core/layout/data/floattable-table-join.docx b/sw/qa/core/layout/data/floattable-table-join.docx
new file mode 100644
index 000000000000..807b4cfa8c11
--- /dev/null
+++ b/sw/qa/core/layout/data/floattable-table-join.docx
diff --git a/sw/qa/core/layout/tabfrm.cxx b/sw/qa/core/layout/tabfrm.cxx
index cfad007cebcd..e83767992aa6 100644
--- a/sw/qa/core/layout/tabfrm.cxx
+++ b/sw/qa/core/layout/tabfrm.cxx
@@ -13,6 +13,10 @@
 #include <rootfrm.hxx>
 #include <pagefrm.hxx>
 #include <tabfrm.hxx>
+#include <sortedobjs.hxx>
+#include <anchoredobject.hxx>
+#include <flyfrm.hxx>
+#include <flyfrms.hxx>
 
 namespace
 {
@@ -109,6 +113,62 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyNestedRowSpan)
     // Then make sure the resulting page count matches Word:
     CPPUNIT_ASSERT_EQUAL(6, getPages());
 }
+
+CPPUNIT_TEST_FIXTURE(Test, testSplitFlyTableJoin)
+{
+    // Given a document with a multi-page floating table:
+    // When loading this document:
+    createSwDoc("floattable-table-join.docx");
+
+    // Then make sure this document doesn't crash the layout and has a floating table split on 4
+    // pages:
+    SwDoc* pDoc = getSwDoc();
+    SwRootFrame* pLayout = pDoc->getIDocumentLayoutAccess().GetCurrentLayout();
+    auto pPage1 = pLayout->Lower()->DynCastPageFrame();
+    CPPUNIT_ASSERT(pPage1);
+    CPPUNIT_ASSERT(pPage1->GetSortedObjs());
+    {
+        SwSortedObjs& rPageObjs = *pPage1->GetSortedObjs();
+        CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), rPageObjs.size());
+        auto pFly = rPageObjs[0]->DynCastFlyFrame()->DynCastFlyAtContentFrame();
+        CPPUNIT_ASSERT(pFly);
+        // Start of the chain.
+        CPPUNIT_ASSERT(!pFly->GetPrecede());
+        CPPUNIT_ASSERT(pFly->HasFollow());
+    }
+    auto pPage2 = pPage1->GetNext()->DynCastPageFrame();
+    CPPUNIT_ASSERT(pPage2);
+    CPPUNIT_ASSERT(pPage2->GetSortedObjs());
+    {
+        SwSortedObjs& rPageObjs = *pPage2->GetSortedObjs();
+        CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), rPageObjs.size());
+        auto pFly = rPageObjs[0]->DynCastFlyFrame()->DynCastFlyAtContentFrame();
+        CPPUNIT_ASSERT(pFly);
+        CPPUNIT_ASSERT(pFly->GetPrecede());
+        CPPUNIT_ASSERT(pFly->HasFollow());
+    }
+    auto pPage3 = pPage2->GetNext()->DynCastPageFrame();
+    CPPUNIT_ASSERT(pPage3);
+    CPPUNIT_ASSERT(pPage3->GetSortedObjs());
+    {
+        SwSortedObjs& rPageObjs = *pPage3->GetSortedObjs();
+        CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), rPageObjs.size());
+        auto pFly = rPageObjs[0]->DynCastFlyFrame()->DynCastFlyAtContentFrame();
+        CPPUNIT_ASSERT(pFly);
+        CPPUNIT_ASSERT(pFly->GetPrecede());
+        CPPUNIT_ASSERT(pFly->HasFollow());
+    }
+    auto pPage4 = pPage3->GetNext()->DynCastPageFrame();
+    CPPUNIT_ASSERT(pPage4);
+    CPPUNIT_ASSERT(pPage4->GetSortedObjs());
+    SwSortedObjs& rPageObjs = *pPage4->GetSortedObjs();
+    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), rPageObjs.size());
+    auto pFly = rPageObjs[0]->DynCastFlyFrame()->DynCastFlyAtContentFrame();
+    CPPUNIT_ASSERT(pFly);
+    // End of the chain.
+    CPPUNIT_ASSERT(pFly->GetPrecede());
+    CPPUNIT_ASSERT(!pFly->HasFollow());
+}
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/layout/tabfrm.cxx b/sw/source/core/layout/tabfrm.cxx
index b671e2248550..4367d86255a2 100644
--- a/sw/source/core/layout/tabfrm.cxx
+++ b/sw/source/core/layout/tabfrm.cxx
@@ -2739,6 +2739,7 @@ void SwTabFrame::MakeAll(vcl::RenderContext* pRenderContext)
                         aRectFnSet.GetTopMargin(*this) +
                          lcl_GetHeightOfRows( GetLower(), nMinNumOfLines ) );
 
+                bool bHadFollowFlowLineBeforeSplit = false;
                 // Some more checks if we want to call the split algorithm or not:
                 // The repeating lines / keeping lines still fit into the upper or
                 // if we do not have an (in)direct Prev, we split anyway.
@@ -2754,6 +2755,7 @@ void SwTabFrame::MakeAll(vcl::RenderContext* pRenderContext)
                         if (!nThrowAwayValidLayoutLimit)
                             continue;
                         const bool bInitialLoopEndCondition(isFrameAreaDefinitionValid());
+                        bHadFollowFlowLineBeforeSplit = true;
                         RemoveFollowFlowLine();
                         const bool bFinalLoopEndCondition(isFrameAreaDefinitionValid());
 
@@ -2790,7 +2792,15 @@ void SwTabFrame::MakeAll(vcl::RenderContext* pRenderContext)
                     // If splitting the table was successful or not,
                     // we do not want to have 'empty' follow tables.
                     if ( GetFollow() && !GetFollow()->GetFirstNonHeadlineRow() )
-                        Join();
+                    {
+                        // For split flys, if we just removed the follow flow line before split,
+                        // then avoid the join in the error + rowsplit case, so split can be called
+                        // again, this time without a rowsplit.
+                        if (!bFlySplit || !bHadFollowFlowLineBeforeSplit || !bSplitError || !bTryToSplit)
+                        {
+                            Join();
+                        }
+                    }
 
                     // We want to restore the situation before the failed
                     // split operation as good as possible. Therefore we
@@ -2808,6 +2818,8 @@ void SwTabFrame::MakeAll(vcl::RenderContext* pRenderContext)
                         continue;
                     }
 
+                    // If split failed, then next time try without
+                    // allowing to split the table rows.
                     bTryToSplit = !bSplitError;
 
                     //To avoid oscillations the Follow must become valid now