tdf#138907 sw TitlePageDlg: Goto correct (inserted) page

Especially with inserted-in-the-middle pages, until the layout
was re-calculated, the new pages were not recognized.

This patch should bring this format option into pretty good shape.

I made a couple of decisions:
1.) Insert an additional content page when adding to end of document
(otherwise the last index page turned into content).
2.) Ensure that index pages don't inherit page-renumbering

This CalcLayout could take a fair amount of time on a huge document.
I tested on a document that took over a minute to load.
It inserted pages in about 10 seconds. The experience seemed
reasonable to me given the known slowness of the document.

make UITest_writer_tests5 UITEST_TEST_NAME=\
titlePageWizard2.tdf138907.test_tdf138907

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

3 files changed, 228 insertions, 21 deletions

diff --git a/sw/qa/uitest/writer_tests5/titlePageWizard.py b/sw/qa/uitest/writer_tests5/titlePageWizard.py
index a04ebaf61eff..f2c235e20fd5 100644
--- a/sw/qa/uitest/writer_tests5/titlePageWizard.py
+++ b/sw/qa/uitest/writer_tests5/titlePageWizard.py
@@ -10,6 +10,7 @@ from libreoffice.uno.propertyvalue import mkPropertyValues
 
 # This tests the Format->Title Page wizard, specifically the reset page number portion,
 # replacing some pages with title pages,
+# inserting pages in the middle of the document,
 # and inserting at the very end of the document.
 class tdf138907(UITestCase):
     def test_tdf138907(self):
@@ -45,8 +46,6 @@ class tdf138907(UITestCase):
         xOKBtn = xDialog.getChild("ok")
         self.ui_test.close_dialog_through_button(xOKBtn)
 
-        # This correctly reset the starting page number for page 2 as "1".
-        # It wasn't persistent across round-trips though
         Paragraphs = document.Text.createEnumeration()
         Para1 = Paragraphs.nextElement()
         self.assertEqual(Para1.String, "6")
@@ -81,7 +80,7 @@ class tdf138907(UITestCase):
         self.ui_test.execute_dialog_through_command(".uno:TitlePageDialog")
         xDialog = self.xUITest.getTopFocusWindow()
         print(xDialog.getChildren())
-        #Insert the title/index pages at page two.
+        #Convert three pages to title/index pages starting at page two.
         xPageCount = xDialog.getChild("NF_PAGE_COUNT")
         for _ in range(0,2):
             xPageCount.executeAction("UP", tuple())
@@ -117,7 +116,7 @@ class tdf138907(UITestCase):
         self.ui_test.execute_dialog_through_command(".uno:TitlePageDialog")
         xDialog = self.xUITest.getTopFocusWindow()
         print(xDialog.getChildren())
-        #Insert title/index pages at the end of the document.
+        #Insert three title/index pages at the end of the document (plus a content page).
         newPages = xDialog.getChild("RB_INSERT_NEW_PAGES")
         newPages.executeAction("CLICK", tuple())
         xPageCount = xDialog.getChild("NF_PAGE_COUNT")
@@ -133,7 +132,6 @@ class tdf138907(UITestCase):
         self.ui_test.close_dialog_through_button(xOKBtn)
 
         # Without the fix, the pages were being inserted before the last page.
-        # NOTE: there are still LOTS of problems here that still need fixing.
         text = document.Text.String.replace('\r\n', '\n')
         self.assertEqual(text[0:1], "6")
         self.assertEqual(text[2:3], "6")
@@ -141,6 +139,76 @@ class tdf138907(UITestCase):
         self.assertEqual(text[6:7], "8")
         # Without the fix, the new pages were inserted before the last page.
         self.assertFalse("\n" in text[8:9])
+        #Note: 13 total virtual pages, including four blanks, as seen in book view
+        self.assertEqual(document.CurrentController.PageCount, 13)
+
+        #Now test inserting in the middle of the document
+
+        #dialog Title Page
+        self.ui_test.execute_dialog_through_command(".uno:TitlePageDialog")
+        xDialog = self.xUITest.getTopFocusWindow()
+        print(xDialog.getChildren())
+        #Insert three title/index pages starting at page 2.
+        newPages = xDialog.getChild("RB_INSERT_NEW_PAGES")
+        newPages.executeAction("CLICK", tuple())
+        xPageCount = xDialog.getChild("NF_PAGE_COUNT")
+        for _ in range(0,2):
+            xPageCount.executeAction("UP", tuple())
+        xUseStartingPage = xDialog.getChild("RB_PAGE_START")
+        xUseStartingPage.executeAction("CLICK", tuple())
+        xStartingPage = xDialog.getChild("NF_PAGE_START")
+        for _ in range(0,10):
+            xStartingPage.executeAction("DOWN", tuple()) #Reset to page 1
+        xStartingPage.executeAction("UP", tuple()) #Start at page 2.
+
+        xOKBtn = xDialog.getChild("ok")
+        self.ui_test.close_dialog_through_button(xOKBtn)
+
+        # Without first re-calculating the layout, the styles were applied to the wrong pages.
+        Paragraphs = document.Text.createEnumeration()
+        Para1 = Paragraphs.nextElement()
+        self.assertEqual(Para1.String, "6")
+        self.assertEqual(Para1.PageDescName, "First Page")
+        # The next three pages are the ones that were just inserted.
+        Para2 = Paragraphs.nextElement()
+        self.assertEqual(Para2.String, "")
+        self.assertEqual(Para2.PageDescName, "First Page")
+        Para3 = Paragraphs.nextElement()
+        self.assertEqual(Para3.String, "")
+        self.assertEqual(Para3.PageDescName, "Index")
+        Para4 = Paragraphs.nextElement()
+        self.assertEqual(Para4.String, "")
+        self.assertEqual(Para4.PageDescName, "Index")
+        # A bit of a quirk is that the style of the first page after the
+        # title page is still First Page - so that is used as the Normal page style.
+        # OK - this is a bit of a strange workflow, so just accept that.
+        Para5 = Paragraphs.nextElement()
+        self.assertEqual(Para5.String, "6")
+        self.assertEqual(Para5.PageDescName, "First Page")
+        Para6 = Paragraphs.nextElement()
+        self.assertEqual(Para6.String, "7")
+        self.assertEqual(Para6.PageDescName, "Index")
+        Para7 = Paragraphs.nextElement()
+        self.assertEqual(Para7.String, "8")
+        self.assertEqual(Para7.PageDescName, "Index")
+        Para8 = Paragraphs.nextElement()
+        self.assertEqual(Para8.String, "2")
+        self.assertEqual(Para8.PageDescName, "Landscape")
+        Para9 = Paragraphs.nextElement()
+        self.assertEqual(Para9.String, "")
+        self.assertEqual(Para9.PageDescName, "First Page")
+        Para10 = Paragraphs.nextElement()
+        self.assertEqual(Para10.String, "")
+        self.assertEqual(Para10.PageDescName, "Index")
+        Para11 = Paragraphs.nextElement()
+        self.assertEqual(Para11.String, "")
+        self.assertEqual(Para11.PageDescName, "Index")
+        # The quirk resets this extra content page to the "style after First page == First Page"
+        Para12 = Paragraphs.nextElement()
+        self.assertEqual(Para12.String, "")
+        self.assertEqual(Para12.PageDescName, "First Page")
+        #Note: 17 total virtual pages, including five blanks, as seen in book view
+        self.assertEqual(document.CurrentController.PageCount, 17)
 
         self.ui_test.close_doc()
 
diff --git a/sw/qa/uitest/writer_tests5/titlePageWizard2.py b/sw/qa/uitest/writer_tests5/titlePageWizard2.py
new file mode 100644
index 000000000000..1359dcc8b16e
--- /dev/null
+++ b/sw/qa/uitest/writer_tests5/titlePageWizard2.py
@@ -0,0 +1,137 @@
+# -*- tab-width: 4; indent-tabs-mode: nil; py-indent-offset: 4 -*-
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+from uitest.framework import UITestCase
+from uitest.uihelper.common import get_state_as_dict, get_url_for_data_file
+from libreoffice.uno.propertyvalue import mkPropertyValues
+
+# This tests both an edge cases, and some more realistic situations.
+class tdf138907(UITestCase):
+    def test_tdf138907(self):
+        self.ui_test.load_file(get_url_for_data_file("tdf138907_titlePageDialog.odt"))
+        document = self.ui_test.get_component()
+
+        # Test an undefined situation - try to modify pages beyond the end of the document.
+
+        #dialog Title Page
+        self.ui_test.execute_dialog_through_command(".uno:TitlePageDialog")
+        xDialog = self.xUITest.getTopFocusWindow()
+        #set restart page number to 2. With this doc, it defaults to resetting to 1.
+        xRestartNumbering = xDialog.getChild("NF_RESTART_NUMBERING")
+        xRestartNumbering.executeAction("UP", tuple()) # restart numbering at 2
+
+        #Convert three pages to title/index pages starting at non-existing page twenty.
+        xPageCount = xDialog.getChild("NF_PAGE_COUNT")
+        for _ in range(0,2):
+            xPageCount.executeAction("UP", tuple())
+        xUseStartingPage = xDialog.getChild("RB_PAGE_START")
+        xUseStartingPage.executeAction("CLICK", tuple())
+        xStartingPage = xDialog.getChild("NF_PAGE_START")
+        for _ in range(0,19):
+            xStartingPage.executeAction("UP", tuple()) #Start at mythical page 20.
+
+        xOKBtn = xDialog.getChild("ok")
+        self.ui_test.close_dialog_through_button(xOKBtn)
+
+        # Nothing should happen when modifying pages that don't exist.
+        # Just a page break, without a valid restart page number on page 2
+        self.assertEqual(document.CurrentController.PageCount, 5)
+        Paragraphs = document.Text.createEnumeration()
+        Para1 = Paragraphs.nextElement()
+        self.assertEqual(Para1.String, "6")
+        self.assertEqual(Para1.PageDescName, "First Page")
+        Para2 = Paragraphs.nextElement()
+        self.assertEqual(Para2.String, "7")
+        self.assertEqual(Para2.PageDescName, None)
+        Para3 = Paragraphs.nextElement()
+        self.assertEqual(Para3.String, "8")
+        self.assertEqual(Para3.PageDescName, None)
+        Para4 = Paragraphs.nextElement()
+        self.assertEqual(Para4.String, "9")
+        self.assertEqual(Para4.PageDescName, None)
+        Para5 = Paragraphs.nextElement()
+        self.assertEqual(Para5.String, "10")
+        self.assertEqual(Para5.PageDescName, None)
+
+
+        #dialog Title Page
+        self.ui_test.execute_dialog_through_command(".uno:TitlePageDialog")
+        xDialog = self.xUITest.getTopFocusWindow()
+        #set restart page number to 1 - which is the default.
+        #set restart title page to 1 - the current value for this document is 6.
+        xRestartNumbering = xDialog.getChild("NF_SET_PAGE_NUMBER")
+        print(xDialog.getChildren())
+        for _ in range(0,5):
+            xRestartNumbering.executeAction("DOWN", tuple()) # restart title numbering at 1
+        #Insert two title/index pages at beginning of the document.
+        newPages = xDialog.getChild("RB_INSERT_NEW_PAGES")
+        newPages.executeAction("CLICK", tuple())
+        xPageCount = xDialog.getChild("NF_PAGE_COUNT")
+        for _ in range(0,1):
+            xPageCount.executeAction("UP", tuple())
+
+        xOKBtn = xDialog.getChild("ok")
+        self.ui_test.close_dialog_through_button(xOKBtn)
+
+        Paragraphs = document.Text.createEnumeration()
+        Para1 = Paragraphs.nextElement()
+        self.assertEqual(Para1.String, "")
+        self.assertEqual(Para1.PageDescName, "First Page")
+        Para2 = Paragraphs.nextElement()
+        self.assertEqual(Para2.String, "")
+        self.assertEqual(Para2.PageDescName, "Index")
+        Para3 = Paragraphs.nextElement()
+        self.assertEqual(Para3.String, "1")
+        self.assertEqual(Para3.PageDescName, "Landscape")
+        Para4 = Paragraphs.nextElement()
+        self.assertEqual(Para4.String, "2")
+        Para5 = Paragraphs.nextElement()
+        self.assertEqual(Para5.String, "3")
+        Para6 = Paragraphs.nextElement()
+        self.assertEqual(Para6.String, "4")
+        Para7 = Paragraphs.nextElement()
+        self.assertEqual(Para7.String, "5")
+
+        #Now test replacing several pages with title and index styles
+
+        #dialog Title Page
+        self.ui_test.execute_dialog_through_command(".uno:TitlePageDialog")
+        xDialog = self.xUITest.getTopFocusWindow()
+        print(xDialog.getChildren())
+        #Convert four pages to title/index pages starting at page one.
+        xPageCount = xDialog.getChild("NF_PAGE_COUNT")
+        for _ in range(0,3):
+            xPageCount.executeAction("DOWN", tuple())  #reset to 1 first
+        for _ in range(0,3):
+            xPageCount.executeAction("UP", tuple())
+
+        xOKBtn = xDialog.getChild("ok")
+        self.ui_test.close_dialog_through_button(xOKBtn)
+
+        Paragraphs = document.Text.createEnumeration()
+        Para1 = Paragraphs.nextElement()
+        self.assertEqual(Para1.String, "")
+        self.assertEqual(Para1.PageDescName, "First Page")
+        Para2 = Paragraphs.nextElement()
+        self.assertEqual(Para2.String, "")
+        self.assertEqual(Para2.PageDescName, "Index")
+        Para3 = Paragraphs.nextElement()
+        self.assertEqual(Para3.String, "3")
+        self.assertEqual(Para3.PageDescName, "Index")
+        Para4 = Paragraphs.nextElement()
+        self.assertEqual(Para4.String, "4")
+        self.assertEqual(Para4.PageDescName, "Index")
+        Para5 = Paragraphs.nextElement()
+        self.assertEqual(Para5.String, "1")
+        self.assertEqual(Para5.PageDescName, "Landscape")
+        Para6 = Paragraphs.nextElement()
+        self.assertEqual(Para6.String, "2")
+        Para7 = Paragraphs.nextElement()
+        self.assertEqual(Para7.String, "3")
+
+        self.ui_test.close_doc()
+
+# vim: set shiftwidth=4 softtabstop=4 expandtab:
diff --git a/sw/source/ui/misc/titlepage.cxx b/sw/source/ui/misc/titlepage.cxx
index eb431a35d757..33b7743d90e9 100644
--- a/sw/source/ui/misc/titlepage.cxx
+++ b/sw/source/ui/misc/titlepage.cxx
@@ -52,7 +52,8 @@ namespace
         sal_uInt16 nPgNo = 0;
         if (nNewNumber)
         {
-            nPgNo = nNewNumber;
+            // -1: Allow special case to prevent inheriting re-numbering from the existing page.
+            nPgNo = nNewNumber == SAL_MAX_UINT16 ? 0 : nNewNumber;
         }
         else if (pPageFormatDesc)
         {
@@ -279,32 +280,33 @@ IMPL_LINK_NOARG(SwTitlePageDlg, OKHdl, weld::Button&, void)
         // Assuming that a failure to GotoPage means the end of the document,
         // insert new pages after the last page.
         if (!lcl_GotoPage(mrSh, GetInsertPosition()))
+        {
             mrSh.EndPg();
-        // FIXME: These new pages cannot be accessed currently with GotoPage. It doesn't know they exist.
+            // Add one more page as a content page to follow the new title pages.
+            mrSh.InsertPageBreak();
+        }
         for (sal_uInt16 nI = 0; nI < nNumTitlePages; ++nI)
             mrSh.InsertPageBreak();
+        // In order to be able to access these new pages, the layout needs to be recalculated first.
+        mrSh.CalcLayout();
     }
 
     if (lcl_GotoPage(mrSh, GetInsertPosition()))
-        mrSh.SetAttrItem(aTitleDesc);
-    for (sal_uInt16 nI = 1; nI < nNumTitlePages; ++nI)
-    {
-        if (mrSh.SttNxtPg())
-            lcl_ChangePage(mrSh, 0, mpIndexDesc);
-    }
-
-    if (nNumTitlePages > 1 && mrSh.GotoPage(GetInsertPosition() + nNumTitlePages, false))
     {
-        // FIXME: By definition, GotoPage(x,bRecord=false) returns false. This is dead code.
-        SwFormatPageDesc aPageFormatDesc(mpNormalDesc);
-        mrSh.SetAttrItem(aPageFormatDesc);
+        mrSh.SetAttrItem(aTitleDesc);
+        for (sal_uInt16 nI = 1; nI < nNumTitlePages; ++nI)
+        {
+            if (mrSh.SttNxtPg())
+                lcl_ChangePage(mrSh, SAL_MAX_UINT16, mpIndexDesc);
+        }
     }
 
-    if (m_xRestartNumberingCB->get_active() || nNumTitlePages > 1)
+    if ((m_xRestartNumberingCB->get_active() || nNumTitlePages > 1)
+        && lcl_GotoPage(mrSh, GetInsertPosition(), nNumTitlePages))
     {
-        sal_uInt16 nPgNo = m_xRestartNumberingCB->get_active() ? m_xRestartNumberingNF->get_value() : 0;
+        sal_uInt16 nPgNo
+            = m_xRestartNumberingCB->get_active() ? m_xRestartNumberingNF->get_value() : 0;
         const SwPageDesc* pNewDesc = nNumTitlePages > 1 ? mpNormalDesc : nullptr;
-        lcl_GotoPage(mrSh, GetInsertPosition(), nNumTitlePages);
         lcl_ChangePage(mrSh, nPgNo, pNewDesc);
     }