[Libreoffice-commits] core.git: Branch 'distro/vector/vector-5.4' - sw/qa sw/source

[Miklos Vajna (via logerrit)]

 sw/qa/extras/uiwriter/data/tables-move-backwards.odt |binary
 sw/qa/extras/uiwriter/uiwriter.cxx                   |   26 +++++++++++++++++++
 sw/source/core/layout/layact.cxx                     |   13 ++++++++-
 3 files changed, 38 insertions(+), 1 deletion(-)

New commits:
commit 88649c8d4cbfc04b4a319ae448036b59d1ce29d4
Author:     Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Mon Mar 30 17:53:51 2020 +0200
Commit:     Miklos Vajna <vmiklos at collabora.com>
CommitDate: Tue Mar 31 09:20:52 2020 +0200

    sw: fix moving more than 20 table frames to a previous page
    
    Steps to reproduce the problem:
    
    - have some content on page1
    - have more than 20 tables on page 2
    - delete all content on page 1
    
    The first 20 tables are moved to page 1 then the layout process stops as
    the layout loop control aborts it:
    
    warn:legacy.osl:8282:8282:sw/source/core/layout/layact.cxx:544: LoopControl_1 in SwLayAction::InternalAction
    
    and the remaining tables stay on page 2, even if page 1 would have space
    for them.
    
    There are various other ways to trigger the same problem, e.g. have a
    ToC, add lots of headings, update the ToC, undo.
    
    Fix the problem by doing more work in SwLayAction::FormatLayout in a
    single iteration: if a table frame is moved to a different parent we can
    still format the table's next frame in the same iteration with a bit of
    effort.
    
    (cherry picked from commit e4da634b983052f300cd0e9b2bbaa60eb02c1b28)
    
    Conflicts:
            sw/qa/core/layout/layout.cxx
            sw/source/core/layout/layact.cxx
    
    Change-Id: I25912a69c19e042f0e0375898f4e0a5fa13321fc

diff --git a/sw/qa/extras/uiwriter/data/tables-move-backwards.odt b/sw/qa/extras/uiwriter/data/tables-move-backwards.odt
new file mode 100644
index 000000000000..861dc4f4ad86
Binary files /dev/null and b/sw/qa/extras/uiwriter/data/tables-move-backwards.odt differ
diff --git a/sw/qa/extras/uiwriter/uiwriter.cxx b/sw/qa/extras/uiwriter/uiwriter.cxx
index 735ab11d4518..6dee000e182f 100644
--- a/sw/qa/extras/uiwriter/uiwriter.cxx
+++ b/sw/qa/extras/uiwriter/uiwriter.cxx
@@ -281,6 +281,7 @@ public:
     void testTdf116789();
     void testTdf117225();
     void testOleSaveWhileEdit();
+    void testTablesMoveBackwards();
 
     CPPUNIT_TEST_SUITE(SwUiWriterTest);
     CPPUNIT_TEST(testReplaceForward);
@@ -423,6 +424,7 @@ public:
     CPPUNIT_TEST(testTdf116789);
     CPPUNIT_TEST(testTdf117225);
     CPPUNIT_TEST(testOleSaveWhileEdit);
+    CPPUNIT_TEST(testTablesMoveBackwards);
     CPPUNIT_TEST_SUITE_END();
 
 private:
@@ -5121,6 +5123,30 @@ void SwUiWriterTest::testOleSaveWhileEdit()
     comphelper::LibreOfficeKit::setActive(false);
 }
 
+void SwUiWriterTest::testTablesMoveBackwards()
+{
+    // Load a document with 1 pages: empty content on first page, then 21 tables on the second page.
+    load(DATA_DIRECTORY, "tables-move-backwards.odt");
+    SwXTextDocument* pTextDoc = dynamic_cast<SwXTextDocument*>(mxComponent.get());
+    SwDocShell* pDocShell = pTextDoc->GetDocShell();
+    SwWrtShell* pWrtShell = pDocShell->GetWrtShell();
+
+    // Delete the content on the first page.
+    pWrtShell->SttEndDoc(/*bStart=*/true);
+    pWrtShell->EndPg(/*bSelect=*/true);
+    pWrtShell->DelLeft();
+
+    // Calc the layout and check the number of pages.
+    pWrtShell->CalcLayout();
+    xmlDocPtr pLayout = parseLayoutDump();
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: 1
+    // - Actual  : 2
+    // i.e. there was an unexpected 2nd page, as only 20 out of 21 tables were moved to the first
+    // page.
+    assertXPath(pLayout, "//page", 1);
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(SwUiWriterTest);
 CPPUNIT_PLUGIN_IMPLEMENT();
 
diff --git a/sw/source/core/layout/layact.cxx b/sw/source/core/layout/layact.cxx
index db136d963873..fff8e10ea4cc 100644
--- a/sw/source/core/layout/layact.cxx
+++ b/sw/source/core/layout/layact.cxx
@@ -1365,10 +1365,17 @@ bool SwLayAction::FormatLayout( OutputDevice *pRenderContext, SwLayoutFrame *pLa
     bool bTabChanged = false;
     while ( pLow && pLow->GetUpper() == pLay )
     {
+        SwFrame* pNext = nullptr;
         if ( pLow->IsLayoutFrame() )
         {
             if ( pLow->IsTabFrame() )
+            {
+                // Remember what was the next of the lower. Formatting may move it to the previous
+                // page, in which case it looses its next.
+                pNext = pLow->GetNext();
+
                 bTabChanged |= FormatLayoutTab( static_cast<SwTabFrame*>(pLow), bAddRect );
+            }
             // Skip the ones already registered for deletion
             else if( !pLow->IsSctFrame() || static_cast<SwSectionFrame*>(pLow)->GetSection() )
                 bChanged |= FormatLayout( pRenderContext, static_cast<SwLayoutFrame*>(pLow), bAddRect );
@@ -1380,7 +1387,11 @@ bool SwLayAction::FormatLayout( OutputDevice *pRenderContext, SwLayoutFrame *pLa
 
         if ( IsAgain() )
             return false;
-        pLow = pLow->GetNext();
+        if (!pNext)
+        {
+            pNext = pLow->GetNext();
+        }
+        pLow = pNext;
     }
     // add complete frame area as paint area, if frame
     // area has been already added and after formatting its lowers the frame area