tdf#155685 sw: fix crash on undo of ExtendedSelectAll with table at end

While the selection with table at the end works for Copy, unfortunately
it doesn't work for Delete, because SwUndoDelete can't handle it,
and an empty SwTableNode+SwEndNode pair remains.

This needs some extra code, extract a SwDoc::DelTable() out of
SwDoc::DeleteRowCol() and call it from SwEditShell::DeleteSel() to
create separate SwUndoDelete objects per table, which appears to work.

(regression from commit d81379db730a163c5ff75d4f3a3cddbd7b5eddda)

Change-Id: I1534b100d31bc279bce298d3c2bd235cffc1f9d5
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/152628
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>

4 files changed, 179 insertions, 144 deletions

diff --git a/sw/source/core/crsr/crsrsh.cxx b/sw/source/core/crsr/crsrsh.cxx
index c34827f092d8..325ff54d52cd 100644
--- a/sw/source/core/crsr/crsrsh.cxx
+++ b/sw/source/core/crsr/crsrsh.cxx
@@ -819,11 +819,12 @@ static typename SwCursorShell::StartsWith EndsWith(SwStartNode const& rStart)
 
 // return the node that is the start of the extended selection (to include table
 // or section start nodes; looks like extending for end nodes is not required)
-SwNode const* SwCursorShell::ExtendedSelectedAll() const
+::std::optional<::std::pair<SwNode const*, ::std::vector<SwTableNode*>>>
+SwCursorShell::ExtendedSelectedAll() const
 {
     if (m_pTableCursor)
     {
-        return nullptr;
+        return {};
     }
 
     SwNodes& rNodes = GetDoc()->GetNodes();
@@ -834,27 +835,48 @@ SwNode const* SwCursorShell::ExtendedSelectedAll() const
     SwContentNode* pStart = rNodes.GoNext(&nNode);
     if (!pStart)
     {
-        return nullptr;
+        return {};
     }
 
     nNode = *pStartNode->EndOfSectionNode();
     SwContentNode* pEnd = SwNodes::GoPrevious(&nNode);
     if (!pEnd)
     {
-        return nullptr;
+        return {};
     }
 
     SwPosition aStart(*pStart, 0);
     SwPosition aEnd(*pEnd, pEnd->Len());
     if (!(aStart == *pShellCursor->Start() && aEnd == *pShellCursor->End()))
     {
-        return nullptr;
+        return {};
     }
 
+    auto const ends(::EndsWith(*pStartNode));
     if (::StartsWith(*pStartNode) == StartsWith::None
-        && ::EndsWith(*pStartNode) == StartsWith::None)
+        && ends == StartsWith::None)
     {
-        return nullptr; // "ordinary" selection will work
+        return {}; // "ordinary" selection will work
+    }
+
+    ::std::vector<SwTableNode*> tablesAtEnd;
+    if (ends == StartsWith::Table)
+    {
+        SwNode * pLastNode(rNodes[pStartNode->EndOfSectionIndex() - 1]);
+        while (pLastNode->IsEndNode())
+        {
+            SwNode *const pNode(pLastNode->StartOfSectionNode());
+            if (pNode->IsTableNode())
+            {
+                tablesAtEnd.push_back(pNode->GetTableNode());
+                pLastNode = rNodes[pNode->GetIndex() - 1];
+            }
+            else if (pNode->IsSectionNode())
+            {
+                pLastNode = rNodes[pLastNode->GetIndex() - 1];
+            }
+        }
+        assert(!tablesAtEnd.empty());
     }
 
     // tdf#133990 ensure directly containing section is included in SwUndoDelete
@@ -867,7 +889,7 @@ SwNode const* SwCursorShell::ExtendedSelectedAll() const
 
     // pStartNode is the node that fully contains the selection - the first
     // node of the selection is the first node inside pStartNode
-    return pStartNode->GetNodes()[pStartNode->GetIndex() + 1];
+    return ::std::make_pair(rNodes[pStartNode->GetIndex() + 1], tablesAtEnd);
 }
 
 typename SwCursorShell::StartsWith SwCursorShell::StartsWith_()
diff --git a/sw/source/core/docnode/ndtbl.cxx b/sw/source/core/docnode/ndtbl.cxx
index 8e055c5242bc..089b518c1cf7 100644
--- a/sw/source/core/docnode/ndtbl.cxx
+++ b/sw/source/core/docnode/ndtbl.cxx
@@ -1915,6 +1915,140 @@ void SwDoc::DeleteCol( const SwCursor& rCursor )
     GetIDocumentUndoRedo().EndUndo(SwUndoId::COL_DELETE, nullptr);
 }
 
+void SwDoc::DelTable(SwTableNode *const pTableNd)
+{
+    bool bNewTextNd = false;
+    // Is it alone in a FlyFrame?
+    SwNodeIndex aIdx( *pTableNd, -1 );
+    const SwStartNode* pSttNd = aIdx.GetNode().GetStartNode();
+    if (pSttNd)
+    {
+        const SwNodeOffset nTableEnd = pTableNd->EndOfSectionIndex() + 1;
+        const SwNodeOffset nSectEnd = pSttNd->EndOfSectionIndex();
+        if (nTableEnd == nSectEnd)
+        {
+            if (SwFlyStartNode == pSttNd->GetStartNodeType())
+            {
+                SwFrameFormat* pFormat = pSttNd->GetFlyFormat();
+                if (pFormat)
+                {
+                    // That's the FlyFormat we're looking for
+                    getIDocumentLayoutAccess().DelLayoutFormat( pFormat );
+                    return;
+                }
+            }
+            // No Fly? Thus Header or Footer: always leave a TextNode
+            // We can forget about Undo then!
+            bNewTextNd = true;
+        }
+    }
+
+    // No Fly? Then it is a Header or Footer, so keep always a TextNode
+    ++aIdx;
+    if (GetIDocumentUndoRedo().DoesUndo())
+    {
+        GetIDocumentUndoRedo().ClearRedo();
+        SwPaM aPaM( *pTableNd->EndOfSectionNode(), aIdx.GetNode() );
+
+        if (bNewTextNd)
+        {
+            const SwNodeIndex aTmpIdx( *pTableNd->EndOfSectionNode(), 1 );
+            GetNodes().MakeTextNode( aTmpIdx.GetNode(),
+                        getIDocumentStylePoolAccess().GetTextCollFromPool( RES_POOLCOLL_STANDARD ) );
+        }
+
+        // Save the cursors (UNO and otherwise)
+        SwPaM aSavePaM( *pTableNd->EndOfSectionNode() );
+        if (! aSavePaM.Move(fnMoveForward, GoInNode))
+        {
+            aSavePaM.GetMark()->Assign( *pTableNd );
+            aSavePaM.Move( fnMoveBackward, GoInNode );
+        }
+        {
+            SwPaM const tmpPaM(*pTableNd, *pTableNd->EndOfSectionNode());
+            ::PaMCorrAbs(tmpPaM, *aSavePaM.GetMark());
+        }
+
+        // Move hard PageBreaks to the succeeding Node
+        bool bSavePageBreak = false, bSavePageDesc = false;
+        SwNodeOffset nNextNd = pTableNd->EndOfSectionIndex()+1;
+        SwContentNode* pNextNd = GetNodes()[ nNextNd ]->GetContentNode();
+        if (pNextNd)
+        {
+            SwFrameFormat* pTableFormat = pTableNd->GetTable().GetFrameFormat();
+            const SfxPoolItem *pItem;
+            if (SfxItemState::SET == pTableFormat->GetItemState(RES_PAGEDESC,
+                    false, &pItem))
+            {
+                pNextNd->SetAttr( *pItem );
+                bSavePageDesc = true;
+            }
+
+            if (SfxItemState::SET == pTableFormat->GetItemState(RES_BREAK,
+                    false, &pItem))
+            {
+                pNextNd->SetAttr( *pItem );
+                bSavePageBreak = true;
+            }
+        }
+        std::unique_ptr<SwUndoDelete> pUndo(new SwUndoDelete(aPaM, SwDeleteFlags::Default));
+        if (bNewTextNd)
+            pUndo->SetTableDelLastNd();
+        pUndo->SetPgBrkFlags( bSavePageBreak, bSavePageDesc );
+        pUndo->SetTableName(pTableNd->GetTable().GetFrameFormat()->GetName());
+        GetIDocumentUndoRedo().AppendUndo( std::move(pUndo) );
+    }
+    else
+    {
+        if (bNewTextNd)
+        {
+            const SwNodeIndex aTmpIdx( *pTableNd->EndOfSectionNode(), 1 );
+            GetNodes().MakeTextNode( aTmpIdx.GetNode(),
+                        getIDocumentStylePoolAccess().GetTextCollFromPool( RES_POOLCOLL_STANDARD ) );
+        }
+
+        // Save the cursors (UNO and otherwise)
+        SwPaM aSavePaM( *pTableNd->EndOfSectionNode() );
+        if (! aSavePaM.Move(fnMoveForward, GoInNode))
+        {
+            aSavePaM.GetMark()->Assign( *pTableNd );
+            aSavePaM.Move( fnMoveBackward, GoInNode );
+        }
+        {
+            SwPaM const tmpPaM(*pTableNd, *pTableNd->EndOfSectionNode());
+            ::PaMCorrAbs(tmpPaM, *aSavePaM.GetMark());
+        }
+
+        // Move hard PageBreaks to the succeeding Node
+        SwContentNode* pNextNd = GetNodes()[ pTableNd->EndOfSectionIndex()+1 ]->GetContentNode();
+        if (pNextNd)
+        {
+            SwFrameFormat* pTableFormat = pTableNd->GetTable().GetFrameFormat();
+            const SfxPoolItem *pItem;
+            if (SfxItemState::SET == pTableFormat->GetItemState(RES_PAGEDESC,
+                    false, &pItem))
+            {
+                pNextNd->SetAttr( *pItem );
+            }
+
+            if (SfxItemState::SET == pTableFormat->GetItemState(RES_BREAK,
+                    false, &pItem))
+            {
+                pNextNd->SetAttr( *pItem );
+            }
+        }
+
+        pTableNd->DelFrames();
+        getIDocumentContentOperations().DeleteSection( pTableNd );
+    }
+
+    if (SwFEShell* pFEShell = GetDocShell()->GetFEShell())
+        pFEShell->UpdateTableStyleFormatting();
+
+    getIDocumentState().SetModified();
+    getIDocumentFieldsAccess().SetFieldsDirty( true, nullptr, SwNodeOffset(0) );
+}
+
 bool SwDoc::DeleteRowCol(const SwSelBoxes& rBoxes, RowColMode const eMode)
 {
     if (!(eMode & SwDoc::RowColMode::DeleteProtected)
@@ -1954,133 +2088,7 @@ bool SwDoc::DeleteRowCol(const SwSelBoxes& rBoxes, RowColMode const eMode)
         aSelBoxes[0]->GetSttIdx()-1 == nTmpIdx1 &&
         nTmpIdx2 == pTableNd->EndOfSectionIndex() )
     {
-        bool bNewTextNd = false;
-        // Is it alone in a FlyFrame?
-        SwNodeIndex aIdx( *pTableNd, -1 );
-        const SwStartNode* pSttNd = aIdx.GetNode().GetStartNode();
-        if( pSttNd )
-        {
-            const SwNodeOffset nTableEnd = pTableNd->EndOfSectionIndex() + 1;
-            const SwNodeOffset nSectEnd = pSttNd->EndOfSectionIndex();
-            if( nTableEnd == nSectEnd )
-            {
-                if( SwFlyStartNode == pSttNd->GetStartNodeType() )
-                {
-                    SwFrameFormat* pFormat = pSttNd->GetFlyFormat();
-                    if( pFormat )
-                    {
-                        // That's the FlyFormat we're looking for
-                        getIDocumentLayoutAccess().DelLayoutFormat( pFormat );
-                        return true;
-                    }
-                }
-                // No Fly? Thus Header or Footer: always leave a TextNode
-                // We can forget about Undo then!
-                bNewTextNd = true;
-            }
-        }
-
-        // No Fly? Then it is a Header or Footer, so keep always a TextNode
-        ++aIdx;
-        if (GetIDocumentUndoRedo().DoesUndo())
-        {
-            GetIDocumentUndoRedo().ClearRedo();
-            SwPaM aPaM( *pTableNd->EndOfSectionNode(), aIdx.GetNode() );
-
-            if( bNewTextNd )
-            {
-                const SwNodeIndex aTmpIdx( *pTableNd->EndOfSectionNode(), 1 );
-                GetNodes().MakeTextNode( aTmpIdx.GetNode(),
-                            getIDocumentStylePoolAccess().GetTextCollFromPool( RES_POOLCOLL_STANDARD ) );
-            }
-
-            // Save the cursors (UNO and otherwise)
-            SwPaM aSavePaM( *pTableNd->EndOfSectionNode() );
-            if( ! aSavePaM.Move( fnMoveForward, GoInNode ) )
-            {
-                aSavePaM.GetMark()->Assign( *pTableNd );
-                aSavePaM.Move( fnMoveBackward, GoInNode );
-            }
-            {
-                SwPaM const tmpPaM(*pTableNd, *pTableNd->EndOfSectionNode());
-                ::PaMCorrAbs(tmpPaM, *aSavePaM.GetMark());
-            }
-
-            // Move hard PageBreaks to the succeeding Node
-            bool bSavePageBreak = false, bSavePageDesc = false;
-            SwNodeOffset nNextNd = pTableNd->EndOfSectionIndex()+1;
-            SwContentNode* pNextNd = GetNodes()[ nNextNd ]->GetContentNode();
-            if( pNextNd )
-            {
-                SwFrameFormat* pTableFormat = pTableNd->GetTable().GetFrameFormat();
-                const SfxPoolItem *pItem;
-                if( SfxItemState::SET == pTableFormat->GetItemState( RES_PAGEDESC,
-                    false, &pItem ) )
-                {
-                    pNextNd->SetAttr( *pItem );
-                    bSavePageDesc = true;
-                }
-
-                if( SfxItemState::SET == pTableFormat->GetItemState( RES_BREAK,
-                    false, &pItem ) )
-                {
-                    pNextNd->SetAttr( *pItem );
-                    bSavePageBreak = true;
-                }
-            }
-            std::unique_ptr<SwUndoDelete> pUndo(new SwUndoDelete(aPaM, SwDeleteFlags::Default));
-            if( bNewTextNd )
-                pUndo->SetTableDelLastNd();
-            pUndo->SetPgBrkFlags( bSavePageBreak, bSavePageDesc );
-            pUndo->SetTableName(pTableNd->GetTable().GetFrameFormat()->GetName());
-            GetIDocumentUndoRedo().AppendUndo( std::move(pUndo) );
-        }
-        else
-        {
-            if( bNewTextNd )
-            {
-                const SwNodeIndex aTmpIdx( *pTableNd->EndOfSectionNode(), 1 );
-                GetNodes().MakeTextNode( aTmpIdx.GetNode(),
-                            getIDocumentStylePoolAccess().GetTextCollFromPool( RES_POOLCOLL_STANDARD ) );
-            }
-
-            // Save the cursors (UNO and otherwise)
-            SwPaM aSavePaM( *pTableNd->EndOfSectionNode() );
-            if( ! aSavePaM.Move( fnMoveForward, GoInNode ) )
-            {
-                aSavePaM.GetMark()->Assign( *pTableNd );
-                aSavePaM.Move( fnMoveBackward, GoInNode );
-            }
-            {
-                SwPaM const tmpPaM(*pTableNd, *pTableNd->EndOfSectionNode());
-                ::PaMCorrAbs(tmpPaM, *aSavePaM.GetMark());
-            }
-
-            // Move hard PageBreaks to the succeeding Node
-            SwContentNode* pNextNd = GetNodes()[ pTableNd->EndOfSectionIndex()+1 ]->GetContentNode();
-            if( pNextNd )
-            {
-                SwFrameFormat* pTableFormat = pTableNd->GetTable().GetFrameFormat();
-                const SfxPoolItem *pItem;
-                if( SfxItemState::SET == pTableFormat->GetItemState( RES_PAGEDESC,
-                    false, &pItem ) )
-                    pNextNd->SetAttr( *pItem );
-
-                if( SfxItemState::SET == pTableFormat->GetItemState( RES_BREAK,
-                    false, &pItem ) )
-                    pNextNd->SetAttr( *pItem );
-            }
-
-            pTableNd->DelFrames();
-            getIDocumentContentOperations().DeleteSection( pTableNd );
-        }
-
-        if (SwFEShell* pFEShell = GetDocShell()->GetFEShell())
-            pFEShell->UpdateTableStyleFormatting();
-
-        getIDocumentState().SetModified();
-        getIDocumentFieldsAccess().SetFieldsDirty( true, nullptr, SwNodeOffset(0) );
-
+        DelTable(pTableNd);
         return true;
     }
 
diff --git a/sw/source/core/edit/eddel.cxx b/sw/source/core/edit/eddel.cxx
index 83855ff2d99a..6082dabb9ef0 100644
--- a/sw/source/core/edit/eddel.cxx
+++ b/sw/source/core/edit/eddel.cxx
@@ -35,9 +35,9 @@
 
 void SwEditShell::DeleteSel(SwPaM& rPam, bool const isArtificialSelection, bool *const pUndo)
 {
-    SwNode const*const pSelectAllStart(StartsWith_() != SwCursorShell::StartsWith::None
+    auto const oSelectAll(StartsWith_() != SwCursorShell::StartsWith::None
         ? ExtendedSelectedAll()
-        : nullptr);
+        : ::std::optional<::std::pair<SwNode const*, ::std::vector<SwTableNode *>>>{});
     // only for selections
     if (!rPam.HasMark()
         || (*rPam.GetPoint() == *rPam.GetMark()
@@ -53,7 +53,7 @@ void SwEditShell::DeleteSel(SwPaM& rPam, bool const isArtificialSelection, bool
     // 3. Point and Mark are at the document start and end, Point is in a table: delete selection as usual
     if( rPam.GetPointNode().FindTableNode() &&
         rPam.GetPointNode().StartOfSectionNode() !=
-        rPam.GetMarkNode().StartOfSectionNode() && pSelectAllStart == nullptr)
+        rPam.GetMarkNode().StartOfSectionNode() && !oSelectAll)
     {
         // group the Undo in the table
         if( pUndo && !*pUndo )
@@ -97,13 +97,18 @@ void SwEditShell::DeleteSel(SwPaM& rPam, bool const isArtificialSelection, bool
     {
         std::optional<SwPaM> pNewPam;
         SwPaM * pPam = &rPam;
-        if (pSelectAllStart)
+        if (oSelectAll)
         {
+            // tdf#155685 tables at the end must be deleted separately
+            for (SwTableNode *const pTable : oSelectAll->second)
+            {
+                GetDoc()->DelTable(pTable);
+            }
             assert(dynamic_cast<SwShellCursor*>(&rPam)); // must be corrected pam
             pNewPam.emplace(*rPam.GetMark(), *rPam.GetPoint());
             // Selection starts at the first para of the first cell, but we
             // want to delete the table node before the first cell as well.
-            pNewPam->Start()->Assign(*pSelectAllStart);
+            pNewPam->Start()->Assign(*oSelectAll->first);
             pPam = &*pNewPam;
         }
         // delete everything
diff --git a/sw/source/core/edit/edglss.cxx b/sw/source/core/edit/edglss.cxx
index 18e5d4ec823b..80310bab4f47 100644
--- a/sw/source/core/edit/edglss.cxx
+++ b/sw/source/core/edit/edglss.cxx
@@ -197,9 +197,9 @@ bool SwEditShell::CopySelToDoc( SwDoc& rInsDoc )
         bool bColSel = GetCursor_()->IsColumnSelection();
         if( bColSel && rInsDoc.IsClipBoard() )
             rInsDoc.SetColumnSelection( true );
-        SwNode const*const pSelectAllStart(StartsWith_() != SwCursorShell::StartsWith::None
+        auto const oSelectAll(StartsWith_() != SwCursorShell::StartsWith::None
             ? ExtendedSelectedAll()
-            : nullptr);
+            : ::std::optional<::std::pair<SwNode const*, ::std::vector<SwTableNode*>>>{});
         {
             for(SwPaM& rPaM : GetCursor()->GetRingContainer())
             {
@@ -223,12 +223,12 @@ bool SwEditShell::CopySelToDoc( SwDoc& rInsDoc )
                     // for the purpose of copying, our shell cursor is not touched.
                     // (Otherwise we would have to restore it.)
                     SwPaM aPaM(*rPaM.GetMark(), *rPaM.GetPoint());
-                    if (pSelectAllStart)
+                    if (oSelectAll)
                     {
                         // Selection starts at the first para of the first cell,
                         // but we want to copy the table and the start node before
                         // the first cell as well.
-                        aPaM.Start()->Assign(*pSelectAllStart);
+                        aPaM.Start()->Assign(*oSelectAll->first);
                     }
                     bRet = GetDoc()->getIDocumentContentOperations().CopyRange( aPaM, aPos, SwCopyFlags::CheckPosInFly)
                         || bRet;