tdf#40427: use node index as position, not Y position on screen

As mentioned in comment to SwContent::nYPosition in
sw/source/uibase/inc/swcont.hxx:

some subclasses appear to use this for a tools/gen.hxx-style
geometric Y position, while e.g. SwOutlineContent wants to store
the index in its subtree

Abusing the nYPosition to store vertical position *on screen* gives
wrong results when a following section is positioned on screen higher
than a previous section - e.g., when multiple-page view is active.

So just use the section's node as Y position of the Navigator entry.
When the section is inside a fly frame, use the frame's anchor node.

Change-Id: I6caf26aeb19d845129dc837138c37f42bbc18655
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/112197
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

3 files changed, 123 insertions, 19 deletions

diff --git a/sw/qa/uitest/data/tdf40427_SectionPositions.odt b/sw/qa/uitest/data/tdf40427_SectionPositions.odt
new file mode 100644
index 000000000000..67d30d0ccfee
--- /dev/null
+++ b/sw/qa/uitest/data/tdf40427_SectionPositions.odt
diff --git a/sw/qa/uitest/navigator/tdf40427.py b/sw/qa/uitest/navigator/tdf40427.py
new file mode 100644
index 000000000000..25c168981a4b
--- /dev/null
+++ b/sw/qa/uitest/navigator/tdf40427.py
@@ -0,0 +1,81 @@
+# -*- tab-width: 4; indent-tabs-mode: nil; py-indent-offset: 4 -*-
+#
+# This file is part of the LibreOffice project.
+#
+# 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 libreoffice.uno.propertyvalue import mkPropertyValues
+from uitest.uihelper.common import get_state_as_dict, get_url_for_data_file
+
+class tdf40427(UITestCase):
+
+  def test_tdf40427(self):
+    self.ui_test.load_file(get_url_for_data_file("tdf40427_SectionPositions.odt"))
+    xMainWindow = self.xUITest.getTopFocusWindow()
+    xWriterEdit = xMainWindow.getChild("writer_edit")
+
+    self.assertEqual(2, self.ui_test.get_component().CurrentController.PageCount)
+
+    # Make sure that the view is 2 pages side-by-side - look at dialog View-Zoom-Zoom
+    self.ui_test.execute_dialog_through_command(".uno:Zoom")
+    xDialog = self.xUITest.getTopFocusWindow()
+
+    columnssb = xDialog.getChild("columnssb")
+    columns = xDialog.getChild("columns")
+    bookmode = xDialog.getChild("bookmode")
+    self.assertEqual("true", get_state_as_dict(columns)["Checked"])
+    self.assertEqual("2", get_state_as_dict(columnssb)["Text"])
+    self.assertEqual("false", get_state_as_dict(bookmode)["Selected"])
+
+    xOKBtn = xDialog.getChild("ok")
+    self.ui_test.close_dialog_through_button(xOKBtn)
+
+    # In this view, the sections "SectionB" and "SectionC" on second page are positioned on screen
+    # higher than "SectionY" and "SectionA" respectively; there are nested and anchored sections.
+    # Make sure that order in Navigator follows their relative position in document, not vertical
+    # position on screen, nor sorted alphabetically. Sections in flying frames are sorted by their
+    # anchor position in the document.
+
+    self.xUITest.executeCommand(".uno:Sidebar")
+    xWriterEdit.executeAction("SIDEBAR", mkPropertyValues({"PANEL": "SwNavigatorPanel"}))
+
+    # wait until the navigator panel is available
+    xNavigatorPanel = self.ui_test.wait_until_child_is_available('NavigatorPanelParent')
+
+    xContentTree = xNavigatorPanel.getChild("contenttree")
+    xSections = xContentTree.getChild('6')
+    self.assertEqual('Sections', get_state_as_dict(xSections)['Text'])
+    xSections.executeAction("EXPAND", ())
+
+    refSectionNames = [
+      'SectionZ',
+      'SectionY', # SectionB should not get before this, despite its Y position on screen is higher
+      'SectionT3', # Sections in tables go in rows, then across rows
+      'SectionT1',
+      'SectionT2',
+      'SectionT0',
+      'SectionF2', # Goes before SectionF1, because their fly anchors go in that order
+      'SectionF3', # Same as SectionF1, but anchor section is in fly itself
+      'SectionFinF3', # Check order of nested sections inside fly
+      'SectionA',
+      'SectionF1', # Section in fly anchored in a section goes immediately after its anchor section
+      'SectionB', # High on screen, but late in list because it's on second page
+      'SectionC',
+    ]
+    self.assertEqual(len(refSectionNames), len(xSections.getChildren()))
+
+    actSectionNames = []
+    for i in range(len(refSectionNames)):
+      actSectionNames.append(get_state_as_dict(xSections.getChild(str(i)))['Text'])
+    # Without the fix in place, this would fail with
+    #   AssertionError: Lists differ: ['SectionZ', 'SectionY', 'SectionT3', 'SectionT1', 'SectionT2'[100 chars]onC'] != ['SectionZ', 'SectionB', 'SectionF3', 'SectionFinF3', 'Section[100 chars]onA']
+    self.assertEqual(refSectionNames, actSectionNames)
+
+    self.xUITest.executeCommand(".uno:Sidebar")
+    self.ui_test.close_doc()
+
+# vim: set shiftwidth=4 softtabstop=4 expandtab:
diff --git a/sw/source/uibase/utlui/content.cxx b/sw/source/uibase/utlui/content.cxx
index 8371dd31c385..6a1e66bbb1aa 100644
--- a/sw/source/uibase/utlui/content.cxx
+++ b/sw/source/uibase/utlui/content.cxx
@@ -273,6 +273,26 @@ namespace
 
         return false;
     }
+
+// Gets "YPos" for SwRegionContent, i.e. a number used to sort sections in Navigator's list
+tools::Long getYPosForSection(const SwNodeIndex& rNodeIndex)
+{
+    sal_uLong nIndex = rNodeIndex.GetIndex();
+    if (rNodeIndex.GetNodes().GetEndOfExtras().GetIndex() >= nIndex)
+    {
+        // Not a node of BodyText
+        // Are we in a fly?
+        if (const auto pFlyFormat = rNodeIndex.GetNode().GetFlyFormat())
+        {
+            // Get node index of anchor
+            if (auto pSwPosition = pFlyFormat->GetAnchor().GetContentAnchor())
+            {
+                nIndex = getYPosForSection(pSwPosition->nNode);
+            }
+        }
+    }
+    return static_cast<tools::Long>(nIndex);
+}
 } // end of anonymous namespace
 
 SwContentType::SwContentType(SwWrtShell* pShell, ContentTypeId nType, sal_uInt8 nLevel) :
@@ -365,18 +385,20 @@ void SwContentType::Init(bool* pbInvalidateWindow)
                 pOldMember = std::move(m_pMember);
                 m_pMember.reset( new SwContentArr );
             }
-            const Point aNullPt;
             m_nMemberCount = m_pWrtShell->GetSectionFormatCount();
             for(size_t i = 0; i < m_nMemberCount; ++i)
             {
-                const SwSectionFormat* pFormat;
-                SectionType eTmpType;
-                if( (pFormat = &m_pWrtShell->GetSectionFormat(i))->IsInNodesArr() &&
-                (eTmpType = pFormat->GetSection()->GetType()) != SectionType::ToxContent
-                && SectionType::ToxHeader != eTmpType )
+                const SwSectionFormat* pFormat = &m_pWrtShell->GetSectionFormat(i);
+                if (!pFormat->IsInNodesArr())
+                    continue;
+                const SwSection* pSection = pFormat->GetSection();
+                if (SectionType eTmpType = pSection->GetType();
+                    eTmpType == SectionType::ToxContent || eTmpType == SectionType::ToxHeader)
+                    continue;
+                const SwNodeIndex* pNodeIndex = pFormat->GetContent().GetContentIdx();
+                if (pNodeIndex)
                 {
-                    const OUString& rSectionName =
-                        pFormat->GetSection()->GetSectionName();
+                    const OUString& rSectionName = pSection->GetSectionName();
                     sal_uInt8 nLevel = 0;
                     SwSectionFormat* pParentFormat = pFormat->GetParent();
                     while(pParentFormat)
@@ -386,8 +408,7 @@ void SwContentType::Init(bool* pbInvalidateWindow)
                     }
 
                     std::unique_ptr<SwContent> pCnt(new SwRegionContent(this, rSectionName,
-                            nLevel,
-                            pFormat->FindLayoutRect( false, &aNullPt ).Top()));
+                            nLevel, getYPosForSection(*pNodeIndex)));
 
                     SwPtrMsgPoolItem aAskItem( RES_CONTENT_VISIBLE, nullptr );
                     if( !pFormat->GetInfo( aAskItem ) &&
@@ -687,17 +708,20 @@ void SwContentType::FillMemberList(bool* pbLevelOrVisibilityChanged)
         break;
         case ContentTypeId::REGION    :
         {
-            const Point aNullPt;
             m_nMemberCount = m_pWrtShell->GetSectionFormatCount();
             for(size_t i = 0; i < m_nMemberCount; ++i)
             {
-                const SwSectionFormat* pFormat;
-                SectionType eTmpType;
-                if( (pFormat = &m_pWrtShell->GetSectionFormat(i))->IsInNodesArr() &&
-                (eTmpType = pFormat->GetSection()->GetType()) != SectionType::ToxContent
-                && SectionType::ToxHeader != eTmpType )
+                const SwSectionFormat* pFormat = &m_pWrtShell->GetSectionFormat(i);
+                if (!pFormat->IsInNodesArr())
+                    continue;
+                const SwSection* pSection = pFormat->GetSection();
+                if (SectionType eTmpType = pSection->GetType();
+                    eTmpType == SectionType::ToxContent || eTmpType == SectionType::ToxHeader)
+                    continue;
+                const SwNodeIndex* pNodeIndex = pFormat->GetContent().GetContentIdx();
+                if (pNodeIndex)
                 {
-                    OUString sSectionName = pFormat->GetSection()->GetSectionName();
+                    const OUString& sSectionName = pSection->GetSectionName();
 
                     sal_uInt8 nLevel = 0;
                     SwSectionFormat* pParentFormat = pFormat->GetParent();
@@ -708,8 +732,7 @@ void SwContentType::FillMemberList(bool* pbLevelOrVisibilityChanged)
                     }
 
                     std::unique_ptr<SwContent> pCnt(new SwRegionContent(this, sSectionName,
-                            nLevel,
-                            pFormat->FindLayoutRect( false, &aNullPt ).Top()));
+                            nLevel, getYPosForSection(*pNodeIndex)));
                     if( !pFormat->GetInfo( aAskItem ) &&
                         !aAskItem.pObject )     // not visible
                         pCnt->SetInvisible();