sw/inc/IDocumentMarkAccess.hxx | 3
sw/qa/extras/uiwriter/uiwriter.cxx | 146 ++++++++++++++++
sw/source/core/doc/DocumentContentOperationsManager.cxx | 3
sw/source/core/doc/docbm.cxx | 12 -
sw/source/core/inc/MarkManager.hxx | 3
sw/source/core/inc/mvsave.hxx | 3
sw/source/core/undo/rolbck.cxx | 50 ++---
sw/source/core/undo/undel.cxx | 26 ++
sw/source/core/undo/undobj.cxx | 16 +
9 files changed, 220 insertions(+), 42 deletions(-)
New commits:
commit d272ab078b401bf4d70919680f2aa18ddbb15c6a
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Thu Nov 23 20:45:09 2023 +0100
Commit: Xisco Fauli <xiscofa...@libreoffice.org>
CommitDate: Thu Nov 30 13:09:43 2023 +0100
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.st...@allotropia.de>
(cherry picked from commit 57974af130e7421da6b07589d4a63a754b757ad6)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159894
Reviewed-by: Xisco Fauli <xiscofa...@libreoffice.org>
diff --git a/sw/inc/IDocumentMarkAccess.hxx b/sw/inc/IDocumentMarkAccess.hxx
index d53f180d3fe9..d63b58f606c4 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 58ebbcde09ff..00719b072416 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 bac73a325a5f..81c8c14e1a4b 100644
--- a/sw/source/core/doc/DocumentContentOperationsManager.cxx
+++ b/sw/source/core/doc/DocumentContentOperationsManager.cxx
@@ -4433,7 +4433,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 2d8a411a9c5d..bb8e75969239 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 7e9280f12aaa..ef0e79d74c11 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 024276b78ccd..f36dee9a26cf 100644
--- a/sw/source/core/inc/mvsave.hxx
+++ b/sw/source/core/inc/mvsave.hxx
@@ -95,7 +95,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 d17c08e4795b..230673f21597 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 cd1b3b1d9182..52efad11b69f 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 dce7f3eb2724..d7f4795c95dd 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->Add( *pBkmk, bSavePos, bSaveOtherPos );
}
if ( bSavePos
- && ( bSaveOtherPos
- || !pBkmk->IsExpanded() ) )
+ && (bDelete || !pBkmk->IsExpanded()))
{
pMarkAccess->deleteMark(pMarkAccess->getAllMarksBegin()+n,
false);
n--;
