tdf#148389 sw: fix Delete Undo/Redo of bookmark positions

The main problem is that in SwUndoSaveContent::DelContentIndex() if the
selection start/end is equal to the bookmark start/end, the bookmark is
not deleted and no SwHistoryBookmark is created, hence on Undo the
bookmark positions are not restored.

Since deleting bookmarks in more situations might cause user complaints,
let's just extend the creation of SwHistoryBookmark to these cases,
which means we need to take care both here and in
SwHistoryBookmark::SetInDoc() that there is now a situation where all
bookmark positions are saved and restored but the bookmark still exists
in the document because it wasn't deleted.

The next problem is that using Backspace/Delete keys sets the
ArtificialSelection flag which is stored in SwUndoDelete, but when used
multiple times the SwUndoDelete::CanGrouping() extends an existing
SwUndoDelete, and if it previously would not delete a bookmark, the
extended range might fully contain the bookmark and thus delete it on
Redo, so check if there are saved bookmark positions and prevent
grouping in that case.

Another problem is then that SwUndoDelete::RedoImpl() deletes the
bookmark anyway, as already indicated with a FIXME comment.

This can be prevented by passing the now-existing m_DeleteFlags into
DelBookmarks() from DeleteRangeImplImpl().

Change-Id: Id5eb1a58927aaa6e7e8b75be82d7f854d8057cfc
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159875
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>
diff --git a/sw/inc/IDocumentMarkAccess.hxx b/sw/inc/IDocumentMarkAccess.hxx
index d53f180..d63b58f6 100644
--- a/sw/inc/IDocumentMarkAccess.hxx
+++ b/sw/inc/IDocumentMarkAccess.hxx
@@ -228,7 +228,8 @@ class IDocumentMarkAccess
            const SwNode& rEnd,
            std::vector< ::sw::mark::SaveBookmark>* pSaveBkmk, // Ugly: SaveBookmark is core-internal
            std::optional<sal_Int32> oStartContentIdx,
            std::optional<sal_Int32> oEndContentIdx) =0;
            std::optional<sal_Int32> oEndContentIdx,
            bool isReplace) = 0;

        /** Deletes a mark.

diff --git a/sw/qa/extras/uiwriter/uiwriter.cxx b/sw/qa/extras/uiwriter/uiwriter.cxx
index a58853e..9825b20 100644
--- a/sw/qa/extras/uiwriter/uiwriter.cxx
+++ b/sw/qa/extras/uiwriter/uiwriter.cxx
@@ -1435,6 +1435,152 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest, testBookmarkUndo)
    CPPUNIT_ASSERT_EQUAL(sal_Int32(0), pMarkAccess->getAllMarksCount());
}

CPPUNIT_TEST_FIXTURE(SwUiWriterTest, testTdf148389_Left)
{
    createSwDoc();
    SwDoc* pDoc = getSwDoc();
    SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell();
    CPPUNIT_ASSERT(pWrtShell);
    pWrtShell->Insert("foo bar baz");
    pWrtShell->Left(SwCursorSkipMode::Chars, /*bSelect=*/false, 4, /*bBasicCall=*/false);
    pWrtShell->Left(SwCursorSkipMode::Chars, /*bSelect=*/true, 3, /*bBasicCall=*/false);
    IDocumentMarkAccess* const pMarkAccess = pDoc->getIDocumentMarkAccess();

    auto pMark = pMarkAccess->makeMark(*pWrtShell->GetCursor(), "Mark",
        IDocumentMarkAccess::MarkType::BOOKMARK, ::sw::mark::InsertMode::New);
    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount());
    pWrtShell->Right(SwCursorSkipMode::Chars, /*bSelect=*/false, 4, /*bBasicCall=*/false);
    pWrtShell->DelLeft();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(7), pMark->GetOtherMarkPos().GetContentIndex());
    pWrtShell->DelLeft();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(6), pMark->GetOtherMarkPos().GetContentIndex());
    pWrtShell->DelLeft();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(5), pMark->GetOtherMarkPos().GetContentIndex());
    pWrtShell->DelLeft();
    // historically it wasn't deleted if empty, not sure if it should be
    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(4), pMark->GetMarkPos().GetContentIndex());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(4), pMark->GetOtherMarkPos().GetContentIndex());
    pWrtShell->Undo();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount());
    // Undo re-creates the mark...
    pMark = *pMarkAccess->getAllMarksBegin();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(4), pMark->GetMarkPos().GetContentIndex());
    // the problem was that the end position was not restored
    CPPUNIT_ASSERT_EQUAL(sal_Int32(5), pMark->GetOtherMarkPos().GetContentIndex());
    pWrtShell->Undo();
    // this undo is no longer grouped, to prevent Redo deleting bookmark
    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount());
    // Undo re-creates the mark...
    pMark = *pMarkAccess->getAllMarksBegin();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(4), pMark->GetMarkPos().GetContentIndex());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(7), pMark->GetOtherMarkPos().GetContentIndex());
    pWrtShell->Redo();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(4), pMark->GetMarkPos().GetContentIndex());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(5), pMark->GetOtherMarkPos().GetContentIndex());
    pWrtShell->Redo();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(4), pMark->GetMarkPos().GetContentIndex());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(4), pMark->GetOtherMarkPos().GetContentIndex());
    pWrtShell->Undo();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount());
    // Undo re-creates the mark...
    pMark = *pMarkAccess->getAllMarksBegin();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(4), pMark->GetMarkPos().GetContentIndex());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(5), pMark->GetOtherMarkPos().GetContentIndex());
    pWrtShell->Undo();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount());
    // Undo re-creates the mark...
    pMark = *pMarkAccess->getAllMarksBegin();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(4), pMark->GetMarkPos().GetContentIndex());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(7), pMark->GetOtherMarkPos().GetContentIndex());
}

CPPUNIT_TEST_FIXTURE(SwUiWriterTest, testTdf148389_Right)
{
    createSwDoc();
    SwDoc* pDoc = getSwDoc();
    SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell();
    CPPUNIT_ASSERT(pWrtShell);
    pWrtShell->Insert("foo bar baz");
    pWrtShell->Left(SwCursorSkipMode::Chars, /*bSelect=*/false, 4, /*bBasicCall=*/false);
    pWrtShell->Left(SwCursorSkipMode::Chars, /*bSelect=*/true, 3, /*bBasicCall=*/false);
    IDocumentMarkAccess* const pMarkAccess = pDoc->getIDocumentMarkAccess();

    auto pMark = pMarkAccess->makeMark(*pWrtShell->GetCursor(), "Mark",
        IDocumentMarkAccess::MarkType::BOOKMARK, ::sw::mark::InsertMode::New);
    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount());
    pWrtShell->Left(SwCursorSkipMode::Chars, /*bSelect=*/false, 2, /*bBasicCall=*/false);
    pWrtShell->DelRight();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(3), pMark->GetMarkPos().GetContentIndex());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(6), pMark->GetOtherMarkPos().GetContentIndex());
    pWrtShell->DelRight();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(2), pMark->GetMarkPos().GetContentIndex());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(5), pMark->GetOtherMarkPos().GetContentIndex());
    pWrtShell->DelRight();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(2), pMark->GetMarkPos().GetContentIndex());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(4), pMark->GetOtherMarkPos().GetContentIndex());
    pWrtShell->DelRight();
    // historically it wasn't deleted if empty, not sure if it should be
    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(2), pMark->GetMarkPos().GetContentIndex());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(3), pMark->GetOtherMarkPos().GetContentIndex());
    pWrtShell->Undo();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount());
    // Undo re-creates the mark...
    pMark = *pMarkAccess->getAllMarksBegin();
    // the problem was that the start position was not restored
    CPPUNIT_ASSERT_EQUAL(sal_Int32(2), pMark->GetMarkPos().GetContentIndex());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(5), pMark->GetOtherMarkPos().GetContentIndex());
    pWrtShell->Undo();
    // this undo is no longer grouped, to prevent Redo deleting bookmark
    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount());
    // Undo re-creates the mark...
    pMark = *pMarkAccess->getAllMarksBegin();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(3), pMark->GetMarkPos().GetContentIndex());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(6), pMark->GetOtherMarkPos().GetContentIndex());
    pWrtShell->Undo();
    // this undo is no longer grouped, to prevent Redo deleting bookmark
    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount());
    // Undo re-creates the mark...
    pMark = *pMarkAccess->getAllMarksBegin();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(4), pMark->GetMarkPos().GetContentIndex());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(7), pMark->GetOtherMarkPos().GetContentIndex());
    pWrtShell->Redo();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(3), pMark->GetMarkPos().GetContentIndex());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(6), pMark->GetOtherMarkPos().GetContentIndex());
    pWrtShell->Redo();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(2), pMark->GetMarkPos().GetContentIndex());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(5), pMark->GetOtherMarkPos().GetContentIndex());
    pWrtShell->Redo();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(2), pMark->GetMarkPos().GetContentIndex());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(3), pMark->GetOtherMarkPos().GetContentIndex());
    pWrtShell->Undo();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount());
    // Undo re-creates the mark...
    pMark = *pMarkAccess->getAllMarksBegin();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(2), pMark->GetMarkPos().GetContentIndex());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(5), pMark->GetOtherMarkPos().GetContentIndex());
    pWrtShell->Undo();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount());
    // Undo re-creates the mark...
    pMark = *pMarkAccess->getAllMarksBegin();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(3), pMark->GetMarkPos().GetContentIndex());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(6), pMark->GetOtherMarkPos().GetContentIndex());
    pWrtShell->Undo();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), pMarkAccess->getAllMarksCount());
    // Undo re-creates the mark...
    pMark = *pMarkAccess->getAllMarksBegin();
    CPPUNIT_ASSERT_EQUAL(sal_Int32(4), pMark->GetMarkPos().GetContentIndex());
    CPPUNIT_ASSERT_EQUAL(sal_Int32(7), pMark->GetOtherMarkPos().GetContentIndex());
}

static void lcl_setWeight(SwWrtShell* pWrtShell, FontWeight aWeight)
{
    SvxWeightItem aWeightItem(aWeight, EE_CHAR_WEIGHT);
diff --git a/sw/source/core/doc/DocumentContentOperationsManager.cxx b/sw/source/core/doc/DocumentContentOperationsManager.cxx
index 96ba6b6..d7d15d2 100644
--- a/sw/source/core/doc/DocumentContentOperationsManager.cxx
+++ b/sw/source/core/doc/DocumentContentOperationsManager.cxx
@@ -4496,7 +4496,8 @@ bool DocumentContentOperationsManager::DeleteRangeImplImpl(SwPaM & rPam, SwDelet
        pEnd->GetNode(),
        nullptr,
        pStt->GetContentIndex(),
        pEnd->GetContentIndex());
        pEnd->GetContentIndex(),
        bool(flags & SwDeleteFlags::ArtificialSelection));

    SwNodeIndex aSttIdx( pStt->GetNode() );
    SwContentNode * pCNd = aSttIdx.GetNode().GetContentNode();
diff --git a/sw/source/core/doc/docbm.cxx b/sw/source/core/doc/docbm.cxx
index 5a6c55a..0068113 100644
--- a/sw/source/core/doc/docbm.cxx
+++ b/sw/source/core/doc/docbm.cxx
@@ -1097,7 +1097,8 @@ namespace sw::mark
            const SwNode& rEnd,
            std::vector<SaveBookmark>* pSaveBkmk,
            std::optional<sal_Int32> oStartContentIdx,
            std::optional<sal_Int32> oEndContentIdx )
            std::optional<sal_Int32> oEndContentIdx,
            bool const isReplace)
    {
        std::vector<const_iterator_t> vMarksToDelete;
        bool bIsSortingNeeded = false;
@@ -1116,7 +1117,8 @@ namespace sw::mark
            ::sw::mark::MarkBase *const pMark = *ppMark;
            bool bIsPosInRange(false);
            bool bIsOtherPosInRange(false);
            bool const bDeleteMark = isDeleteMark(pMark, false, rStt, rEnd, oStartContentIdx, oEndContentIdx, bIsPosInRange, bIsOtherPosInRange);
            bool const bDeleteMark = isDeleteMark(pMark, isReplace, rStt, rEnd,
                oStartContentIdx, oEndContentIdx, bIsPosInRange, bIsOtherPosInRange);

            if ( bIsPosInRange
                 && ( bIsOtherPosInRange
@@ -2013,7 +2015,8 @@ void DelBookmarks(
    const SwNode& rEnd,
    std::vector<SaveBookmark> * pSaveBkmk,
    std::optional<sal_Int32> oStartContentIdx,
    std::optional<sal_Int32> oEndContentIdx)
    std::optional<sal_Int32> oEndContentIdx,
    bool const isReplace)
{
    // illegal range ??
    if(rStt.GetIndex() > rEnd.GetIndex()
@@ -2023,7 +2026,8 @@ void DelBookmarks(

    rDoc.getIDocumentMarkAccess()->deleteMarks(rStt, rEnd, pSaveBkmk,
        oStartContentIdx,
        oEndContentIdx);
        oEndContentIdx,
        isReplace);

    // Copy all Redlines which are in the move area into an array
    // which holds all position information as offset.
diff --git a/sw/source/core/inc/MarkManager.hxx b/sw/source/core/inc/MarkManager.hxx
index 7e9280f..ef0e79d 100644
--- a/sw/source/core/inc/MarkManager.hxx
+++ b/sw/source/core/inc/MarkManager.hxx
@@ -67,7 +67,8 @@ namespace sw::mark {
                                    const SwNode& rEnd,
                                    std::vector< ::sw::mark::SaveBookmark>* pSaveBkmk,
                                    std::optional<sal_Int32> oStartContentIdx,
                                    std::optional<sal_Int32> oEndContentIdx) override;
                                    std::optional<sal_Int32> oEndContentIdx,
                                    bool isReplace) override;

            // deleters
            virtual std::unique_ptr<ILazyDeleter>
diff --git a/sw/source/core/inc/mvsave.hxx b/sw/source/core/inc/mvsave.hxx
index 718e0e0..7491718 100644
--- a/sw/source/core/inc/mvsave.hxx
+++ b/sw/source/core/inc/mvsave.hxx
@@ -93,7 +93,8 @@ void DelBookmarks(SwNode& rStt,
    const SwNode& rEnd,
    std::vector< ::sw::mark::SaveBookmark> * SaveBkmk =nullptr,
    std::optional<sal_Int32> oStartContentIdx = std::nullopt,
    std::optional<sal_Int32> oEndContentIdx = std::nullopt);
    std::optional<sal_Int32> oEndContentIdx = std::nullopt,
    bool isReplace = false);

/** data structure to temporarily hold fly anchor positions relative to some
 *  location. */
diff --git a/sw/source/core/undo/rolbck.cxx b/sw/source/core/undo/rolbck.cxx
index e16dbc8..426be22 100644
--- a/sw/source/core/undo/rolbck.cxx
+++ b/sw/source/core/undo/rolbck.cxx
@@ -645,60 +645,50 @@ void SwHistoryBookmark::SetInDoc( SwDoc* pDoc, bool )

    SwNodes& rNds = pDoc->GetNodes();
    IDocumentMarkAccess* pMarkAccess = pDoc->getIDocumentMarkAccess();
    std::optional<SwPaM> pPam;
    std::optional<SwPaM> oPam;
    ::sw::mark::IMark* pMark = nullptr;

    // now the situation is that m_bSavePos and m_bSaveOtherPos don't determine
    // whether the mark was deleted
    if (auto const iter = pMarkAccess->findMark(m_aName); iter != pMarkAccess->getAllMarksEnd())
    {
        pMark = *iter;
    }
    if(m_bSavePos)
    {
        SwContentNode* const pContentNd = rNds[m_nNode]->GetContentNode();
        OSL_ENSURE(pContentNd,
            "<SwHistoryBookmark::SetInDoc(..)>"
            " - wrong node for a mark");

        // #111660# don't crash when nNode1 doesn't point to content node.
        if(pContentNd)
            pPam.emplace(*pContentNd, m_nContent);
        assert(pContentNd);
        oPam.emplace(*pContentNd, m_nContent);
    }
    else
    {
        pMark = *pMarkAccess->findMark(m_aName);
        pPam.emplace(pMark->GetMarkPos());
        assert(pMark);
        oPam.emplace(pMark->GetMarkPos());
    }
    assert(oPam);

    if(m_bSaveOtherPos)
    {
        SwContentNode* const pContentNd = rNds[m_nOtherNode]->GetContentNode();
        OSL_ENSURE(pContentNd,
            "<SwHistoryBookmark::SetInDoc(..)>"
            " - wrong node for a mark");

        if (pPam && pContentNd)
        {
            pPam->SetMark();
            pPam->GetMark()->Assign(*pContentNd, m_nOtherContent);
        }
        assert(pContentNd);
        oPam->SetMark();
        oPam->GetMark()->Assign(*pContentNd, m_nOtherContent);
    }
    else if(m_bHadOtherPos)
    {
        if(!pMark)
            pMark = *pMarkAccess->findMark(m_aName);
        OSL_ENSURE(pMark->IsExpanded(),
            "<SwHistoryBookmark::SetInDoc(..)>"
            " - missing pos on old mark");
        pPam->SetMark();
        *pPam->GetMark() = pMark->GetOtherMarkPos();
        assert(pMark);
        assert(pMark->IsExpanded());
        oPam->SetMark();
        *oPam->GetMark() = pMark->GetOtherMarkPos();
    }

    if (!pPam)
        return;

    if ( pMark != nullptr )
    {
        pMarkAccess->deleteMark( pMark );
    }
    ::sw::mark::IBookmark* const pBookmark =
        dynamic_cast<::sw::mark::IBookmark*>(
            pMarkAccess->makeMark(*pPam, m_aName, m_eBkmkType, sw::mark::InsertMode::New));
            pMarkAccess->makeMark(*oPam, m_aName, m_eBkmkType, sw::mark::InsertMode::New));
    if ( pBookmark == nullptr )
        return;

diff --git a/sw/source/core/undo/undel.cxx b/sw/source/core/undo/undel.cxx
index d9698ed..4c199c8 100644
--- a/sw/source/core/undo/undel.cxx
+++ b/sw/source/core/undo/undel.cxx
@@ -539,6 +539,7 @@ bool SwUndoDelete::CanGrouping( SwDoc& rDoc, const SwPaM& rDelPam )
        if( m_bGroup && !m_bBackSp ) return false;
        m_bBackSp = true;
    }
    // note: compare m_nSttContent here because the text isn't there any more!
    else if( pStt->GetContentIndex() == m_nSttContent )
    {
        if( m_bGroup && m_bBackSp ) return false;
@@ -567,6 +568,30 @@ bool SwUndoDelete::CanGrouping( SwDoc& rDoc, const SwPaM& rDelPam )
        return false;
    }

    if ((m_DeleteFlags & SwDeleteFlags::ArtificialSelection) && m_pHistory)
    {
        IDocumentMarkAccess const& rIDMA(*rDoc.getIDocumentMarkAccess());
        for (auto i = m_pHistory->Count(); 0 < i; )
        {
            --i;
            SwHistoryHint const*const pHistory((*m_pHistory)[i]);
            if (pHistory->Which() == HSTRY_BOOKMARK)
            {
                SwHistoryBookmark const*const pHistoryBM(
                        static_cast<SwHistoryBookmark const*>(pHistory));
                auto const ppMark(rIDMA.findMark(pHistoryBM->GetName()));
                if (ppMark != rIDMA.getAllMarksEnd()
                    && (m_bBackSp
                            ? ((**ppMark).GetMarkPos() == *pStt)
                            : ((**ppMark).IsExpanded()
                                && (**ppMark).GetOtherMarkPos() == *pEnd)))
                {   // prevent grouping that would delete this mark on Redo()
                    return false;
                }
            }
        }
    }

    {
        SwRedlineSaveDatas aTmpSav;
        const bool bSaved = FillSaveData( rDelPam, aTmpSav, false );
@@ -1294,7 +1319,6 @@ void SwUndoDelete::RedoImpl(::sw::UndoRedoContext & rContext)
        rDoc.getIDocumentContentOperations().DelFullPara( rPam );
    }
    else
        // FIXME: this ends up calling DeleteBookmarks() on the entire rPam which deletes too many!
        rDoc.getIDocumentContentOperations().DeleteAndJoin(rPam, m_DeleteFlags);
}

diff --git a/sw/source/core/undo/undobj.cxx b/sw/source/core/undo/undobj.cxx
index 3c5c935..ce27962 100644
--- a/sw/source/core/undo/undobj.cxx
+++ b/sw/source/core/undo/undobj.cxx
@@ -1123,6 +1123,7 @@ void SwUndoSaveContent::DelContentIndex( const SwPosition& rMark,
        // #i81002#
        bool bSavePos = false;
        bool bSaveOtherPos = false;
        bool bDelete = false;
        const ::sw::mark::IMark *const pBkmk = pMarkAccess->getAllMarksBegin()[n];
        auto const type(IDocumentMarkAccess::GetType(*pBkmk));

@@ -1139,6 +1140,7 @@ void SwUndoSaveContent::DelContentIndex( const SwPosition& rMark,
            {
                bSaveOtherPos = true;
            }
            bDelete = bSavePos && bSaveOtherPos;
        }
        else
        {
@@ -1179,8 +1181,16 @@ void SwUndoSaveContent::DelContentIndex( const SwPosition& rMark,
                {
                    if( bMaybe )
                        bSavePos = true;
                    bSaveOtherPos = true;
                    bDelete = true;
                }
                if (bDelete || pBkmk->GetOtherMarkPos() == *pEnd)
                {
                    bSaveOtherPos = true; // tdf#148389 always undo if at end
                }
            }
            if (!bSavePos && bMaybe && pBkmk->IsExpanded() && *pStt == pBkmk->GetMarkPos())
            {
                bSavePos = true; // tdf#148389 always undo if at start
            }

            if ( !bSavePos && !bSaveOtherPos
@@ -1219,6 +1229,7 @@ void SwUndoSaveContent::DelContentIndex( const SwPosition& rMark,
                {
                    bSavePos = true;
                    bSaveOtherPos = pBkmk->IsExpanded(); //tdf#90138, only save the other pos if there is one
                    bDelete = true;
                }
            }
        }
@@ -1232,8 +1243,7 @@ void SwUndoSaveContent::DelContentIndex( const SwPosition& rMark,
                m_pHistory->AddIMark(*pBkmk, bSavePos, bSaveOtherPos);
            }
            if ( bSavePos
                 && ( bSaveOtherPos
                      || !pBkmk->IsExpanded() ) )
                 && (bDelete || !pBkmk->IsExpanded()))
            {
                pMarkAccess->deleteMark(pMarkAccess->getAllMarksBegin()+n, false);
                n--;