tdf#132642 sw layout: try2 emulate table kept-with-next not splitting

This adjusts my LO 5.2 code for tdf#91083 that tried to emulate the
table's keep-with-next property which doesn't have a matching
counterpart in MS formats.

I always confused myself trying to understand what my year-long coding
attempt was trying to do. This seems much understandable, and efficient.
The big clue was that it affected non-MS formats - which was unintended.

Change-Id: I7886e52430cb34799e25f7fcf73500e28bbe2a55
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/93443
Tested-by: Jenkins
Reviewed-by: Justin Luth <justin_luth@sil.org>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>

7 files changed, 43 insertions, 14 deletions

diff --git a/sw/qa/extras/odfexport/data/tdf132642_keepWithNextTable.odt b/sw/qa/extras/odfexport/data/tdf132642_keepWithNextTable.odt
new file mode 100644
index 000000000000..739cd0078ad7
--- /dev/null
+++ b/sw/qa/extras/odfexport/data/tdf132642_keepWithNextTable.odt
diff --git a/sw/qa/extras/odfexport/odfexport.cxx b/sw/qa/extras/odfexport/odfexport.cxx
index bc7780e8f742..dd8729f21912 100644
--- a/sw/qa/extras/odfexport/odfexport.cxx
+++ b/sw/qa/extras/odfexport/odfexport.cxx
@@ -2105,6 +2105,13 @@ DECLARE_ODFEXPORT_TEST(testTdf129568ui, "tdf129568-ui.fodt")
     CPPUNIT_ASSERT_EQUAL(sal_Int32(0xffff00), getProperty<sal_Int32>(xStyle, "BackColor"));
 }
 
+DECLARE_ODFEXPORT_TEST(testTdf132642_keepWithNextTable, "tdf132642_keepWithNextTable.odt")
+{
+    // Since the row is very big, it should split over two pages.
+    // Since up to this point we haven't tried to make it match MS formats, it should start on page 1.
+    CPPUNIT_ASSERT_EQUAL_MESSAGE("Row splits over 2 pages", 2, getPages());
+}
+
 DECLARE_ODFEXPORT_TEST(testImageMimetype, "image-mimetype.odt")
 {
     CPPUNIT_ASSERT_EQUAL(1, getShapes());
diff --git a/sw/qa/extras/ww8export/data/tdf91083_tableKeep.doc b/sw/qa/extras/ww8export/data/tdf91083_tableKeep.doc
deleted file mode 100644
index 2215ab0e92d3..000000000000
--- a/sw/qa/extras/ww8export/data/tdf91083_tableKeep.doc
+++ /dev/null
diff --git a/sw/qa/extras/ww8export/data/tdf91083_tableKeep2.odt b/sw/qa/extras/ww8export/data/tdf91083_tableKeep2.odt
new file mode 100644
index 000000000000..19a1ac843a78
--- /dev/null
+++ b/sw/qa/extras/ww8export/data/tdf91083_tableKeep2.odt
diff --git a/sw/qa/extras/ww8export/data/tdf91083_tableKeep3.odt b/sw/qa/extras/ww8export/data/tdf91083_tableKeep3.odt
new file mode 100644
index 000000000000..d3a13baf490a
--- /dev/null
+++ b/sw/qa/extras/ww8export/data/tdf91083_tableKeep3.odt
diff --git a/sw/qa/extras/ww8export/ww8export.cxx b/sw/qa/extras/ww8export/ww8export.cxx
index f3325616f725..b1ca079711bd 100644
--- a/sw/qa/extras/ww8export/ww8export.cxx
+++ b/sw/qa/extras/ww8export/ww8export.cxx
@@ -1305,6 +1305,28 @@ DECLARE_WW8EXPORT_TEST(testTableKeep, "tdf91083.odt")
 }
 #endif
 
+DECLARE_WW8EXPORT_TEST(tesTdf91083_tableKeep2, "tdf91083_tableKeep2.odt")
+{
+    //emulate table "keep with next" - split large row in order to keep with previous paragraph
+    CPPUNIT_ASSERT_EQUAL_MESSAGE("Table doesn't split, so it starts on page 2",
+                                 OUString("0"), parseDump("count(//page[1]//tab)") );
+    CPPUNIT_ASSERT_EQUAL_MESSAGE("Page 2 starts with a paragraph/title, not a table",
+                                 OUString("KeepWithNext"), parseDump("//page[2]/body/txt[1]") );
+    CPPUNIT_ASSERT_EQUAL_MESSAGE("Table sticks with previous paragraph, so it starts on page 2",
+                                 OUString("1"), parseDump("count(//page[2]//tab)") );
+    CPPUNIT_ASSERT_MESSAGE("Row itself splits, not the table at a row boundary",
+                                 "Cell 2" != parseDump("//page[3]//tab//row[2]/cell[1]/txt[1]") );
+}
+
+DECLARE_WW8EXPORT_TEST(tesTdf91083_tableKeep3, "tdf91083_tableKeep3.odt")
+{
+    //emulate table "keep with next" - split single row table in order to keep with previous paragraph
+    CPPUNIT_ASSERT_EQUAL_MESSAGE("Table doesn't split, so it starts on page 2",
+                                 OUString("0"), parseDump("count(//page[1]//tab)") );
+    CPPUNIT_ASSERT_EQUAL_MESSAGE("Table sticks with previous paragraph, so it starts on page 2",
+                                 OUString("1"), parseDump("count(//page[2]//tab)") );
+}
+
 DECLARE_WW8EXPORT_TEST(testTdf76349_textboxMargins, "tdf76349_textboxMargins.doc")
 {
     // textboxes without borders were losing their spacing items in round-tripping
diff --git a/sw/source/core/layout/tabfrm.cxx b/sw/source/core/layout/tabfrm.cxx
index bedce3a3f588..300232cefe17 100644
--- a/sw/source/core/layout/tabfrm.cxx
+++ b/sw/source/core/layout/tabfrm.cxx
@@ -1910,11 +1910,6 @@ void SwTabFrame::MakeAll(vcl::RenderContext* pRenderContext)
     auto pAccess = std::make_unique<SwBorderAttrAccess>(SwFrame::GetCache(), this);
     const SwBorderAttrs *pAttrs = pAccess->Get();
 
-    const bool bLargeTable = GetTable()->GetTabLines().size() > 64;  //arbitrary value, virtually guaranteed to be larger than one page.
-    const bool bEmulateTableKeep = !bLargeTable && AreAllRowsKeepWithNext( GetFirstNonHeadlineRow(), /*bCheckParents=*/false );
-    // The beloved keep attribute
-    const bool bKeep = IsKeep(pAttrs->GetAttrSet().GetKeep(), GetBreakItem(), bEmulateTableKeep);
-
     // All rows should keep together
     const bool bDontSplit = !IsFollow() &&
                             ( !GetFormat()->GetLayoutSplit().GetValue() );
@@ -1940,6 +1935,11 @@ void SwTabFrame::MakeAll(vcl::RenderContext* pRenderContext)
     bool bLastRowHasToMoveToFollow = false;
     bool bLastRowMoveNoMoreTries = false;
 
+    const bool bLargeTable = GetTable()->GetTabLines().size() > 64;  //arbitrary value, virtually guaranteed to be larger than one page.
+    const bool bEmulateTableKeep = !bLargeTable && bTableRowKeep && AreAllRowsKeepWithNext( GetFirstNonHeadlineRow(), /*bCheckParents=*/false );
+    // The beloved keep attribute
+    const bool bKeep = IsKeep(pAttrs->GetAttrSet().GetKeep(), GetBreakItem(), bEmulateTableKeep);
+
     // Join follow table, if this table is not allowed to split:
     if ( bDontSplit )
     {
@@ -2384,10 +2384,12 @@ void SwTabFrame::MakeAll(vcl::RenderContext* pRenderContext)
                                         AreAllRowsKeepWithNext( pFirstNonHeadlineRow ) ) &&
                                       !pIndPrev &&
                                       !bDontSplit;
-        const bool bEmulateTableKeepFwdMoveAllowed = IsKeepFwdMoveAllowed(bEmulateTableKeep);
+        // tdf91083 MSCompat: this extends bAllowSplitOfRow (and perhaps should just replace it).
+        // If the kept-together items cannot move to a new page, a table split is in general allowed.
+        const bool bEmulateTableKeepSplitAllowed =  bEmulateTableKeep && !IsKeepFwdMoveAllowed(/*IgnoreMyOwnKeepValue=*/true);
 
         if ( pFirstNonHeadlineRow && nUnSplitted > 0 &&
-             ( !bEmulateTableKeepFwdMoveAllowed ||
+             ( bEmulateTableKeepSplitAllowed ||
                ( ( !bTableRowKeep || pFirstNonHeadlineRow->GetNext() ||
                    !pFirstNonHeadlineRow->ShouldRowKeepWithNext() || bAllowSplitOfRow
                  ) && ( !bDontSplit || !pIndPrev )
@@ -2395,14 +2397,12 @@ void SwTabFrame::MakeAll(vcl::RenderContext* pRenderContext)
         {
             // #i29438#
             // Special DoNotSplit cases:
-            // We better avoid splitting if the table keeps with next paragraph and can move fwd still.
             // We better avoid splitting of a row frame if we are inside a columned
             // section which has a height of 0, because this is not growable and thus
             // all kinds of unexpected things could happen.
-            if ( !bEmulateTableKeepFwdMoveAllowed ||
-                 ( IsInSct() && FindSctFrame()->Lower()->IsColumnFrame() &&
-                   0 == aRectFnSet.GetHeight(GetUpper()->getFrameArea())
-               ) )
+            if ( IsInSct() && FindSctFrame()->Lower()->IsColumnFrame() &&
+                 0 == aRectFnSet.GetHeight(GetUpper()->getFrameArea())
+               )
             {
                 bTryToSplit = false;
             }
@@ -2472,7 +2472,7 @@ void SwTabFrame::MakeAll(vcl::RenderContext* pRenderContext)
                 // The repeating lines / keeping lines still fit into the upper or
                 // if we do not have an (in)direct Prev, we split anyway.
                 if( aRectFnSet.YDiff(nDeadLine, nBreakLine) >=0
-                    || !pIndPrev || !bEmulateTableKeepFwdMoveAllowed )
+                    || !pIndPrev || bEmulateTableKeepSplitAllowed )
                 {
                     aNotify.SetLowersComplete( false );
                     bSplit = true;
@@ -2492,7 +2492,7 @@ void SwTabFrame::MakeAll(vcl::RenderContext* pRenderContext)
                         }
                     }
 
-                    const bool bSplitError = !Split( nDeadLine, bTryToSplit, ( bTableRowKeep && !(bAllowSplitOfRow || !bEmulateTableKeepFwdMoveAllowed) ) );
+                    const bool bSplitError = !Split( nDeadLine, bTryToSplit, ( bTableRowKeep && !(bAllowSplitOfRow || bEmulateTableKeepSplitAllowed) ) );
                     if (!bTryToSplit && !bSplitError)
                     {
                         --nUnSplitted;