tdf#159565: make sure to handle leading hidden section correctly

Change-Id: I41c7d2b6e765f03c72a968fd05e8de7047f1ce41
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163371
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Signed-off-by: Xisco Fauli <xiscofauli@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163478
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>

7 files changed, 105 insertions, 16 deletions

diff --git a/sw/inc/crsrsh.hxx b/sw/inc/crsrsh.hxx
index bddea2b87cb6..03afe5006ad6 100644
--- a/sw/inc/crsrsh.hxx
+++ b/sw/inc/crsrsh.hxx
@@ -334,7 +334,7 @@ public:
     void ExtendedSelectAll(bool bFootnotes = true);
     /// If ExtendedSelectAll() was called and selection didn't change since then.
     ::std::optional<::std::pair<SwNode const*, ::std::vector<SwTableNode*>>> ExtendedSelectedAll() const;
-    enum class StartsWith { None, Table, HiddenPara };
+    enum class StartsWith { None, Table, HiddenPara, HiddenSection };
     /// If document body starts with a table or starts/ends with hidden paragraph.
     StartsWith StartsWith_();
 
diff --git a/sw/inc/ndarr.hxx b/sw/inc/ndarr.hxx
index 7afe8d2bce46..7383c253a2e8 100644
--- a/sw/inc/ndarr.hxx
+++ b/sw/inc/ndarr.hxx
@@ -131,6 +131,11 @@ class SW_DLLPUBLIC SwNodes final
 
     SwNodes(SwDoc& rDoc);
 
+    // Returns start of the document section (PostIts/Inserts/Autotext/Redlines/Content),
+    // or of a specific fly / header / footer / footnote, where this node is, which must not
+    // be crossed when moving backwards
+    SwNodeOffset StartOfGlobalSection(const SwNode& node) const;
+
 public:
     ~SwNodes();
 
@@ -188,8 +193,8 @@ public:
 
     SwContentNode* GoNext(SwNodeIndex *) const;
     SwContentNode* GoNext(SwPosition *) const;
-    static SwContentNode* GoPrevious(SwNodeIndex *);
-    static SwContentNode* GoPrevious(SwPosition *);
+    static SwContentNode* GoPrevious(SwNodeIndex *, bool canCrossBoundary = false);
+    static SwContentNode* GoPrevious(SwPosition *, bool canCrossBoundary = false);
 
     /** Go to next content-node that is not protected or hidden
        (Both set FALSE ==> GoNext/GoPrevious!!!). */
diff --git a/sw/qa/extras/uiwriter/data/FrameInHiddenSection.fodt b/sw/qa/extras/uiwriter/data/FrameInHiddenSection.fodt
new file mode 100644
index 000000000000..2095c7173046
--- /dev/null
+++ b/sw/qa/extras/uiwriter/data/FrameInHiddenSection.fodt
@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<office:document xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0" xmlns:fo="urn:oasis:names:tc:opendocument:xmlns:xsl-fo-compatible:1.0" xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0" xmlns:draw="urn:oasis:names:tc:opendocument:xmlns:drawing:1.0" xmlns:svg="urn:oasis:names:tc:opendocument:xmlns:svg-compatible:1.0" office:version="1.3" office:mimetype="application/vnd.oasis.opendocument.text">
+ <office:body>
+  <office:text>
+   <text:section text:name="Section1">
+    <text:section text:name="Section2Hidden" text:display="none">
+     <text:p><draw:frame text:anchor-type="paragraph" svg:x="1cm" svg:y="1cm" svg:width="1cm">
+      <draw:text-box/>
+     </draw:frame>lorem</text:p>
+    </text:section>
+    <text:section text:name="Section3"/>
+    <text:section text:name="Section4"/>
+    <text:section text:name="Section5">
+     <text:p>ipsum</text:p>
+    </text:section>
+   </text:section>
+  </office:text>
+ </office:body>
+</office:document>
\ No newline at end of file
diff --git a/sw/qa/extras/uiwriter/uiwriter9.cxx b/sw/qa/extras/uiwriter/uiwriter9.cxx
index f870f5ea480a..58c95f21d03d 100644
--- a/sw/qa/extras/uiwriter/uiwriter9.cxx
+++ b/sw/qa/extras/uiwriter/uiwriter9.cxx
@@ -16,6 +16,8 @@
 #include <com/sun/star/text/XTextTable.hpp>
 #include <com/sun/star/text/XTextViewCursorSupplier.hpp>
 #include <com/sun/star/text/XPageCursor.hpp>
+#include <com/sun/star/view/XSelectionSupplier.hpp>
+
 #include <comphelper/propertysequence.hxx>
 #include <swdtflvr.hxx>
 #include <o3tl/string_view.hxx>
@@ -138,6 +140,25 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest9, testHiddenSectionsAroundPageBreak)
     CPPUNIT_ASSERT_EQUAL(u"Landscape"_ustr, getProperty<OUString>(xCursor, "PageStyleName"));
 }
 
+CPPUNIT_TEST_FIXTURE(SwUiWriterTest9, testTdf159565)
+{
+    // Given a document with a hidden section in the beginning, additionally containing a frame
+    createSwDoc("FrameInHiddenSection.fodt");
+
+    dispatchCommand(mxComponent, u".uno:SelectAll"_ustr, {});
+
+    // Check that the selection covers the whole visible text
+    auto xModel(mxComponent.queryThrow<css::frame::XModel>());
+    auto xSelSupplier(xModel->getCurrentController().queryThrow<css::view::XSelectionSupplier>());
+    auto xSelections(xSelSupplier->getSelection().queryThrow<css::container::XIndexAccess>());
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), xSelections->getCount());
+    auto xSelection(xSelections->getByIndex(0).queryThrow<css::text::XTextRange>());
+
+    // Without the fix, this would fail - there was no selection
+    CPPUNIT_ASSERT_EQUAL(u"" SAL_NEWLINE_STRING SAL_NEWLINE_STRING "ipsum"_ustr,
+                         xSelection->getString());
+}
+
 } // end of anonymous namespace
 CPPUNIT_PLUGIN_IMPLEMENT();
 
diff --git a/sw/source/core/crsr/crsrsh.cxx b/sw/source/core/crsr/crsrsh.cxx
index d08e4971f556..b7f6962982b5 100644
--- a/sw/source/core/crsr/crsrsh.cxx
+++ b/sw/source/core/crsr/crsrsh.cxx
@@ -775,6 +775,8 @@ static typename SwCursorShell::StartsWith StartsWith(SwStartNode const& rStart)
         switch (rNode.GetNodeType())
         {
             case SwNodeType::Section:
+                if (rNode.GetSectionNode()->GetSection().IsHidden())
+                    return SwCursorShell::StartsWith::HiddenSection;
                 continue;
             case SwNodeType::Table:
                 return SwCursorShell::StartsWith::Table;
@@ -799,11 +801,16 @@ static typename SwCursorShell::StartsWith EndsWith(SwStartNode const& rStart)
         switch (rNode.GetNodeType())
         {
             case SwNodeType::End:
-                if (rNode.StartOfSectionNode()->IsTableNode())
+                if (auto pStartNode = rNode.StartOfSectionNode(); pStartNode->IsTableNode())
                 {
                     return SwCursorShell::StartsWith::Table;
                 }
-//TODO buggy SwUndoRedline in testTdf137503?                assert(rNode.StartOfSectionNode()->IsSectionNode());
+                else if (pStartNode->IsSectionNode())
+                {
+                    if (pStartNode->GetSectionNode()->GetSection().IsHidden())
+                        return SwCursorShell::StartsWith::HiddenSection;
+                }
+                    //TODO buggy SwUndoRedline in testTdf137503?                assert(rNode.StartOfSectionNode()->IsSectionNode());
             break;
             case SwNodeType::Text:
                 if (rNode.GetTextNode()->IsHidden())
@@ -3470,7 +3477,7 @@ bool SwCursorShell::FindValidContentNode( bool bOnlyText )
         GetDoc()->GetDocShell()->IsReadOnlyUI() )
         return true;
 
-    if( m_pCurrentCursor->HasMark() )
+    if( m_pCurrentCursor->HasMark() && !mbSelectAll )
         ClearMark();
 
     // first check for frames
diff --git a/sw/source/core/crsr/swcrsr.cxx b/sw/source/core/crsr/swcrsr.cxx
index 8d0246bed14f..5a2f9afeada5 100644
--- a/sw/source/core/crsr/swcrsr.cxx
+++ b/sw/source/core/crsr/swcrsr.cxx
@@ -913,7 +913,7 @@ static bool lcl_MakeSelFwrd( const SwNode& rSttNd, const SwNode& rEndNd,
 
     rPam.SetMark();
     rPam.GetPoint()->Assign(rEndNd);
-    pCNd = SwNodes::GoPrevious( rPam.GetPoint() );
+    pCNd = SwNodes::GoPrevious(rPam.GetPoint(), true);
     if( !pCNd )
         return false;
     rPam.GetPoint()->AssignEndIndex(*pCNd);
@@ -933,7 +933,7 @@ static bool lcl_MakeSelBkwrd( const SwNode& rSttNd, const SwNode& rEndNd,
     if( !bFirst )
     {
         rPam.GetPoint()->Assign(rSttNd);
-        pCNd = SwNodes::GoPrevious( rPam.GetPoint() );
+        pCNd = SwNodes::GoPrevious(rPam.GetPoint(), true);
         if( !pCNd )
             return false;
         rPam.GetPoint()->AssignEndIndex(*pCNd);
diff --git a/sw/source/core/docnode/nodes.cxx b/sw/source/core/docnode/nodes.cxx
index 8967833a64b1..3506dff2300b 100644
--- a/sw/source/core/docnode/nodes.cxx
+++ b/sw/source/core/docnode/nodes.cxx
@@ -1336,34 +1336,68 @@ SwContentNode* SwNodes::GoNext(SwPosition *pIdx) const
     return static_cast<SwContentNode*>(pNd);
 }
 
-SwContentNode* SwNodes::GoPrevious(SwNodeIndex *pIdx)
+SwNodeOffset SwNodes::StartOfGlobalSection(const SwNode& node) const
+{
+    const SwNodeOffset pos = node.GetIndex();
+    if (GetEndOfExtras().GetIndex() < pos)
+        // Regular ContentSection
+        return GetEndOfExtras().GetIndex() + SwNodeOffset(1);
+    if (GetEndOfAutotext().GetIndex() < pos)
+        // Redlines
+        return GetEndOfAutotext().GetIndex() + SwNodeOffset(1);
+    if (GetEndOfInserts().GetIndex() < pos)
+    {
+        // Flys/Headers/Footers
+        if (auto* p = node.FindFlyStartNode())
+            return p->GetIndex();
+        if (auto* p = node.FindHeaderStartNode())
+            return p->GetIndex();
+        if (auto* p = node.FindFooterStartNode())
+            return p->GetIndex();
+        return GetEndOfInserts().GetIndex() + SwNodeOffset(1);
+    }
+    if (GetEndOfPostIts().GetIndex() < pos)
+    {
+        // Footnotes
+        if (auto* p = node.FindFootnoteStartNode())
+            return p->GetIndex();
+        return GetEndOfPostIts().GetIndex() + SwNodeOffset(1);
+    }
+    return SwNodeOffset(0);
+}
+
+SwContentNode* SwNodes::GoPrevious(SwNodeIndex* pIdx, bool canCrossBoundary)
 {
     if( !pIdx->GetIndex() )
         return nullptr;
 
     SwNodeIndex aTmp( *pIdx, -1 );
+    SwNodeOffset aGlobalStart(
+        canCrossBoundary ? SwNodeOffset(0) : aTmp.GetNodes().StartOfGlobalSection(pIdx->GetNode()));
     SwNode* pNd = nullptr;
-    while( aTmp.GetIndex() && !( pNd = &aTmp.GetNode())->IsContentNode() )
+    while (aTmp > aGlobalStart && !(pNd = &aTmp.GetNode())->IsContentNode())
         --aTmp;
 
-    if( !aTmp.GetIndex() )
+    if (aTmp <= aGlobalStart)
         pNd = nullptr;
     else
         (*pIdx) = aTmp;
     return static_cast<SwContentNode*>(pNd);
 }
 
-SwContentNode* SwNodes::GoPrevious(SwPosition *pIdx)
+SwContentNode* SwNodes::GoPrevious(SwPosition* pIdx, bool canCrossBoundary)
 {
     if( !pIdx->GetNodeIndex() )
         return nullptr;
 
     SwNodeIndex aTmp( pIdx->GetNode(), -1 );
+    SwNodeOffset aGlobalStart(
+        canCrossBoundary ? SwNodeOffset(0) : aTmp.GetNodes().StartOfGlobalSection(pIdx->GetNode()));
     SwNode* pNd = nullptr;
-    while( aTmp.GetIndex() && !( pNd = &aTmp.GetNode())->IsContentNode() )
+    while( aTmp > aGlobalStart && !( pNd = &aTmp.GetNode())->IsContentNode() )
         --aTmp;
 
-    if( !aTmp.GetIndex() )
+    if (aTmp <= aGlobalStart)
         pNd = nullptr;
     else
         pIdx->Assign(aTmp);
@@ -2072,8 +2106,9 @@ SwContentNode* SwNodes::GoPrevSection( SwNodeIndex * pIdx,
 {
     bool bFirst = true;
     SwNodeIndex aTmp( *pIdx );
+    SwNodeOffset aGlobalStart(aTmp.GetNodes().StartOfGlobalSection(pIdx->GetNode()));
     const SwNode* pNd;
-    while( aTmp > SwNodeOffset(0) )
+    while (aTmp > aGlobalStart)
     {
         pNd = & aTmp.GetNode();
         if (SwNodeType::End == pNd->GetNodeType())
@@ -2129,8 +2164,9 @@ SwContentNode* SwNodes::GoPrevSection( SwPosition * pIdx,
 {
     bool bFirst = true;
     SwNodeIndex aTmp( pIdx->GetNode() );
+    SwNodeOffset aGlobalStart(aTmp.GetNodes().StartOfGlobalSection(pIdx->GetNode()));
     const SwNode* pNd;
-    while( aTmp > SwNodeOffset(0) )
+    while (aTmp > aGlobalStart)
     {
         pNd = & aTmp.GetNode();
         if (SwNodeType::End == pNd->GetNodeType())