officecfg/registry/schema/org/openoffice/Office/Security.xcs |    6 
 sd/source/ui/dlg/present.cxx                                 |   14 -
 sd/source/ui/inc/present.hxx                                 |    2 
 sd/source/ui/remotecontrol/Server.cxx                        |   17 -
 sd/source/ui/view/drviews7.cxx                               |    6 
 sd/uiconfig/simpress/ui/presentationdialog.ui                |   17 +
 sw/inc/IDocumentMarkAccess.hxx                               |    3 
 sw/qa/extras/uiwriter/uiwriter.cxx                           |  146 +++++++++++
 sw/source/core/attr/cellatr.cxx                              |    2 
 sw/source/core/doc/DocumentContentOperationsManager.cxx      |    5 
 sw/source/core/doc/docbm.cxx                                 |   12 
 sw/source/core/doc/docfmt.cxx                                |    2 
 sw/source/core/doc/docftn.cxx                                |    4 
 sw/source/core/docnode/ndtbl.cxx                             |    2 
 sw/source/core/inc/MarkManager.hxx                           |    3 
 sw/source/core/inc/mvsave.hxx                                |    3 
 sw/source/core/inc/rolbck.hxx                                |   12 
 sw/source/core/undo/rolbck.cxx                               |   75 ++---
 sw/source/core/undo/undel.cxx                                |   30 ++
 sw/source/core/undo/undobj.cxx                               |   24 +
 sw/source/core/undo/unins.cxx                                |    4 
 sw/source/core/undo/unmove.cxx                               |    6 
 sw/source/core/undo/untbl.cxx                                |    4 
 23 files changed, 311 insertions(+), 88 deletions(-)

New commits:
commit 57974af130e7421da6b07589d4a63a754b757ad6
Author:     Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Thu Nov 23 20:45:09 2023 +0100
Commit:     Michael Stahl <michael.st...@allotropia.de>
CommitDate: Fri Nov 24 10:23:51 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>

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 a58853ec7a65..9825b2090204 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 96ba6b6f3456..d7d15d279776 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 5a6c55ad1f96..00681135b468 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 718e0e02d2f9..7491718191e4 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 e16dbc8ebd48..426be22dcc25 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 d9698ed884f3..4c199c8ac321 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 3c5c93556715..ce27962aa3ec 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--;
commit 5eb6293a895c18ac91a3ba552de71f0571e24da6
Author:     Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Tue Nov 21 19:48:40 2023 +0100
Commit:     Michael Stahl <michael.st...@allotropia.de>
CommitDate: Fri Nov 24 10:23:31 2023 +0100

    sw: rename the rest of the SwHistory Add() member functions
    
    Change-Id: I990fe383341757087d468b3eeae38fca4d8d0175
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159874
    Tested-by: Jenkins
    Reviewed-by: Michael Stahl <michael.st...@allotropia.de>

diff --git a/sw/source/core/attr/cellatr.cxx b/sw/source/core/attr/cellatr.cxx
index 19e7b2f39d53..69eb6b1dd314 100644
--- a/sw/source/core/attr/cellatr.cxx
+++ b/sw/source/core/attr/cellatr.cxx
@@ -132,7 +132,7 @@ void 
SwTableBoxFormula::ToSplitMergeBoxNmWithHistory(SwTableFormulaUpdate& rUpda
     {
         // external rendering
         aCopy.PtrToBoxNm(&pNd->FindTableNode()->GetTable());
-        pHistory->Add(
+        pHistory->AddPoolItem(
             &aCopy,
             &aCopy,
             pNd->FindTableBoxStartNode()->GetIndex());
diff --git a/sw/source/core/doc/DocumentContentOperationsManager.cxx 
b/sw/source/core/doc/DocumentContentOperationsManager.cxx
index d36ee7a8b31a..96ba6b6f3456 100644
--- a/sw/source/core/doc/DocumentContentOperationsManager.cxx
+++ b/sw/source/core/doc/DocumentContentOperationsManager.cxx
@@ -1520,7 +1520,7 @@ namespace //local functions originally from docfmt.cxx
                 if (pCharFormat)
                 {
                     if (pHistory)
-                        pHistory->Add(pCharFormat->GetAttrSet(), *pCharFormat);
+                        pHistory->AddCharFormat(pCharFormat->GetAttrSet(), 
*pCharFormat);
 
                     if ( pCharSet )
                         pCharFormat->SetFormatAttr(*pCharSet);
diff --git a/sw/source/core/doc/docfmt.cxx b/sw/source/core/doc/docfmt.cxx
index b6ac04ef682a..d5854b3633e6 100644
--- a/sw/source/core/doc/docfmt.cxx
+++ b/sw/source/core/doc/docfmt.cxx
@@ -1072,7 +1072,7 @@ static bool lcl_SetTextFormatColl( SwNode* pNode, void* 
pArgs )
 
     // add to History so that old data is saved, if necessary
     if( pPara->pHistory )
-        pPara->pHistory->Add( pCNd->GetFormatColl(), pCNd->GetIndex(),
+        pPara->pHistory->AddColl(pCNd->GetFormatColl(), pCNd->GetIndex(),
                 SwNodeType::Text );
 
     pCNd->ChgFormatColl( pFormat );
diff --git a/sw/source/core/doc/docftn.cxx b/sw/source/core/doc/docftn.cxx
index 61dbe5071cd2..16f6694c94c5 100644
--- a/sw/source/core/doc/docftn.cxx
+++ b/sw/source/core/doc/docftn.cxx
@@ -471,7 +471,7 @@ bool SwDoc::SetCurFootnote( const SwPaM& rPam, const 
OUString& rNumStr,
                 bChg = true;
                 if ( pUndo )
                 {
-                    pUndo->GetHistory().Add( *pTextFootnote );
+                    pUndo->GetHistory().AddFootnote(*pTextFootnote);
                 }
 
                 pTextFootnote->SetNumber(rFootnote.GetNumber(), 
rFootnote.GetNumberRLHidden(), rNumStr);
@@ -505,7 +505,7 @@ bool SwDoc::SetCurFootnote( const SwPaM& rPam, const 
OUString& rNumStr,
                 bChg = true;
                 if ( pUndo )
                 {
-                    pUndo->GetHistory().Add( *pTextFootnote );
+                    pUndo->GetHistory().AddFootnote(*pTextFootnote);
                 }
 
                 pTextFootnote->SetNumber(rFootnote.GetNumber(), 
rFootnote.GetNumberRLHidden(), rNumStr);
diff --git a/sw/source/core/docnode/ndtbl.cxx b/sw/source/core/docnode/ndtbl.cxx
index c5509fa9488b..a7a2bee478da 100644
--- a/sw/source/core/docnode/ndtbl.cxx
+++ b/sw/source/core/docnode/ndtbl.cxx
@@ -2946,7 +2946,7 @@ void SwDoc::SetRowsToRepeat( SwTable &rTable, sal_uInt16 
nSet )
 void SwCollectTableLineBoxes::AddToUndoHistory( const SwContentNode& rNd )
 {
     if( m_pHistory )
-        m_pHistory->Add( rNd.GetFormatColl(), rNd.GetIndex(), SwNodeType::Text 
);
+        m_pHistory->AddColl(rNd.GetFormatColl(), rNd.GetIndex(), 
SwNodeType::Text);
 }
 
 void SwCollectTableLineBoxes::AddBox( const SwTableBox& rBox )
diff --git a/sw/source/core/inc/rolbck.hxx b/sw/source/core/inc/rolbck.hxx
index 20fa600a6301..9d74a6a9e61d 100644
--- a/sw/source/core/inc/rolbck.hxx
+++ b/sw/source/core/inc/rolbck.hxx
@@ -368,15 +368,15 @@ public:
     // call all objects between nStart and TmpEnd; store nStart as TmpEnd
     bool TmpRollback( SwDoc* pDoc, sal_uInt16 nStart, bool ToFirst = true );
 
-    void Add( const SfxPoolItem* pOldValue, const SfxPoolItem* pNewValue,
+    void AddPoolItem(const SfxPoolItem* pOldValue, const SfxPoolItem* 
pNewValue,
               SwNodeOffset nNodeIdx );
-    void Add( SwTextAttr* pTextHt, SwNodeOffset nNodeIdx, bool bNewAttr );
-    void Add( SwFormatColl*, SwNodeOffset nNodeIdx, SwNodeType nWhichNd );
-    void Add( const ::sw::mark::IMark&, bool bSavePos, bool bSaveOtherPos );
+    void AddTextAttr(SwTextAttr* pTextHt, SwNodeOffset nNodeIdx, bool 
bNewAttr);
+    void AddColl(SwFormatColl*, SwNodeOffset nNodeIdx, SwNodeType nWhichNd);
+    void AddIMark(const ::sw::mark::IMark&, bool bSavePos, bool bSaveOtherPos);
     void AddChangeFlyAnchor(sw::SpzFrameFormat& rFormat);
     void AddDeleteFly( SwFrameFormat&, sal_uInt16& rSetPos );
-    void Add( const SwTextFootnote& );
-    void Add( const SfxItemSet & rSet, const SwCharFormat & rCharFormat);
+    void AddFootnote( const SwTextFootnote& );
+    void AddCharFormat(const SfxItemSet & rSet, const SwCharFormat & 
rCharFormat);
 
     sal_uInt16 Count() const { return m_SwpHstry.size(); }
     sal_uInt16 GetTmpEnd() const { return m_SwpHstry.size() - m_nEndDiff; }
diff --git a/sw/source/core/undo/rolbck.cxx b/sw/source/core/undo/rolbck.cxx
index 9a18fde99ffe..e16dbc8ebd48 100644
--- a/sw/source/core/undo/rolbck.cxx
+++ b/sw/source/core/undo/rolbck.cxx
@@ -1026,7 +1026,7 @@ SwHistory::~SwHistory()
 {
 }
 
-void SwHistory::Add(
+void SwHistory::AddPoolItem(
     const SfxPoolItem* pOldValue,
     const SfxPoolItem* pNewValue,
     SwNodeOffset nNodeIdx)
@@ -1063,7 +1063,8 @@ void SwHistory::Add(
 }
 
 // FIXME: refactor the following "Add" methods (DRY)?
-void SwHistory::Add( SwTextAttr* pHint, SwNodeOffset nNodeIdx, bool bNewAttr )
+void SwHistory::AddTextAttr(SwTextAttr *const pHint,
+        SwNodeOffset const nNodeIdx, bool const bNewAttr)
 {
     OSL_ENSURE( !m_nEndDiff, "History was not deleted after REDO" );
 
@@ -1105,7 +1106,8 @@ void SwHistory::Add( SwTextAttr* pHint, SwNodeOffset 
nNodeIdx, bool bNewAttr )
     m_SwpHstry.push_back( std::move(pHt) );
 }
 
-void SwHistory::Add( SwFormatColl* pColl, SwNodeOffset nNodeIdx, SwNodeType 
nWhichNd )
+void SwHistory::AddColl(SwFormatColl *const pColl, SwNodeOffset const nNodeIdx,
+        SwNodeType const nWhichNd)
 {
     OSL_ENSURE( !m_nEndDiff, "History was not deleted after REDO" );
 
@@ -1114,7 +1116,8 @@ void SwHistory::Add( SwFormatColl* pColl, SwNodeOffset 
nNodeIdx, SwNodeType nWhi
     m_SwpHstry.push_back( std::move(pHt) );
 }
 
-void SwHistory::Add(const ::sw::mark::IMark& rBkmk, bool bSavePos, bool 
bSaveOtherPos)
+void SwHistory::AddIMark(const ::sw::mark::IMark& rBkmk,
+        bool const bSavePos, bool const bSaveOtherPos)
 {
     OSL_ENSURE( !m_nEndDiff, "History was not deleted after REDO" );
 
@@ -1186,14 +1189,14 @@ void SwHistory::AddDeleteFly(SwFrameFormat& rFormat, 
sal_uInt16& rSetPos)
     }
 }
 
-void SwHistory::Add( const SwTextFootnote& rFootnote )
+void SwHistory::AddFootnote(const SwTextFootnote& rFootnote)
 {
     std::unique_ptr<SwHistoryHint> pHt(new SwHistorySetFootnote( rFootnote ));
     m_SwpHstry.push_back( std::move(pHt) );
 }
 
 // #i27615#
-void SwHistory::Add(const SfxItemSet & rSet, const SwCharFormat & rFormat)
+void SwHistory::AddCharFormat(const SfxItemSet & rSet, const SwCharFormat & 
rFormat)
 {
     std::unique_ptr<SwHistoryHint> pHt(new SwHistoryChangeCharFormat(rSet, 
rFormat.GetName()));
     m_SwpHstry.push_back( std::move(pHt) );
@@ -1273,7 +1276,7 @@ void SwHistory::CopyFormatAttr(
     {
         if(!IsInvalidItem(pItem))
         {
-            Add(pItem, pItem, nNodeIdx);
+            AddPoolItem(pItem, pItem, nNodeIdx);
         }
 
         pItem = aIter.NextItem();
@@ -1341,12 +1344,12 @@ void SwHistory::CopyAttr(
         {
             if ( nEnd > nAttrStt )
             {
-                Add( pHt, nNodeIdx, false );
+                AddTextAttr(pHt, nNodeIdx, false);
             }
         }
         else if ( pEndIdx && nStart < *pEndIdx )
         {
-            Add( pHt, nNodeIdx, false );
+            AddTextAttr(pHt, nNodeIdx, false);
         }
     }
 }
@@ -1389,7 +1392,7 @@ void SwRegHistory::SwClientNotify(const SwModify&, const 
SfxHint& rHint)
     {
         if(RES_UPDATE_ATTR == pLegacyHint->m_pNew->Which())
         {
-            m_pHistory->Add(pLegacyHint->m_pOld, pLegacyHint->m_pNew, 
m_nNodeIndex);
+            m_pHistory->AddPoolItem(pLegacyHint->m_pOld, pLegacyHint->m_pNew, 
m_nNodeIndex);
         }
         else
         {
@@ -1424,7 +1427,7 @@ void SwRegHistory::SwClientNotify(const SwModify&, const 
SfxHint& rHint)
 
 void SwRegHistory::AddHint( SwTextAttr* pHt, const bool bNew )
 {
-    m_pHistory->Add( pHt, m_nNodeIndex, bNew );
+    m_pHistory->AddTextAttr(pHt, m_nNodeIndex, bNew);
 }
 
 bool SwRegHistory::InsertItems( const SfxItemSet& rSet,
diff --git a/sw/source/core/undo/undel.cxx b/sw/source/core/undo/undel.cxx
index cd1b3b1d9182..d9698ed884f3 100644
--- a/sw/source/core/undo/undel.cxx
+++ b/sw/source/core/undo/undel.cxx
@@ -266,8 +266,8 @@ SwUndoDelete::SwUndoDelete(
     if( pSttTextNd && pEndTextNd && pSttTextNd != pEndTextNd )
     {
         // two different TextNodes, thus save also the TextFormatCollection
-        m_pHistory->Add( pSttTextNd->GetTextColl(),pStt->GetNodeIndex(), 
SwNodeType::Text );
-        m_pHistory->Add( pEndTextNd->GetTextColl(),pEnd->GetNodeIndex(), 
SwNodeType::Text );
+        m_pHistory->AddColl(pSttTextNd->GetTextColl(), pStt->GetNodeIndex(), 
SwNodeType::Text);
+        m_pHistory->AddColl(pEndTextNd->GetTextColl(), pEnd->GetNodeIndex(), 
SwNodeType::Text);
 
         if( !m_bJoinNext )        // Selection from bottom to top
         {
diff --git a/sw/source/core/undo/undobj.cxx b/sw/source/core/undo/undobj.cxx
index bd258dcd5767..3c5c93556715 100644
--- a/sw/source/core/undo/undobj.cxx
+++ b/sw/source/core/undo/undobj.cxx
@@ -950,7 +950,7 @@ void SwUndoSaveContent::DelContentIndex( const SwPosition& 
rMark,
                     pTextNd->GetTextAttrForCharAt( nFootnoteSttIdx );
                 assert(pFootnoteHint);
                 SwContentIndex aIdx( pTextNd, nFootnoteSttIdx );
-                m_pHistory->Add( pFootnoteHint, pTextNd->GetIndex(), false );
+                m_pHistory->AddTextAttr(pFootnoteHint, pTextNd->GetIndex(), 
false);
                 pTextNd->EraseText( aIdx, 1 );
             }
 
@@ -975,7 +975,7 @@ void SwUndoSaveContent::DelContentIndex( const SwPosition& 
rMark,
                     pTextNd->GetTextAttrForCharAt( nFootnoteSttIdx );
                 assert(pFootnoteHint);
                 SwContentIndex aIdx( pTextNd, nFootnoteSttIdx );
-                m_pHistory->Add( pFootnoteHint, pTextNd->GetIndex(), false );
+                m_pHistory->AddTextAttr(pFootnoteHint, pTextNd->GetIndex(), 
false);
                 pTextNd->EraseText( aIdx, 1 );
             }
         }
@@ -1013,7 +1013,7 @@ void SwUndoSaveContent::DelContentIndex( const 
SwPosition& rMark,
                         SwTextAttr* const pFlyHint = 
pTextNd->GetTextAttrForCharAt(
                             pAPos->GetContentIndex());
                         assert(pFlyHint);
-                        m_pHistory->Add( pFlyHint, SwNodeOffset(0), false );
+                        m_pHistory->AddTextAttr(pFlyHint, SwNodeOffset(0), 
false);
                         // reset n so that no Format is skipped
                         n = n >= rSpzArr.size() ? rSpzArr.size() : n+1;
                     }
@@ -1229,7 +1229,7 @@ void SwUndoSaveContent::DelContentIndex( const 
SwPosition& rMark,
             {
                 if( !m_pHistory )
                     m_pHistory.reset( new SwHistory );
-                m_pHistory->Add( *pBkmk, bSavePos, bSaveOtherPos );
+                m_pHistory->AddIMark(*pBkmk, bSavePos, bSaveOtherPos);
             }
             if ( bSavePos
                  && ( bSaveOtherPos
diff --git a/sw/source/core/undo/unins.cxx b/sw/source/core/undo/unins.cxx
index 0ac95d0c192f..bf6c2877b2d4 100644
--- a/sw/source/core/undo/unins.cxx
+++ b/sw/source/core/undo/unins.cxx
@@ -613,7 +613,7 @@ SwUndoReplace::Impl::Impl(
     {
         if( pNd->HasSwAttrSet() )
             m_pHistory->CopyFormatAttr( *pNd->GetpSwAttrSet(), nNewPos );
-        m_pHistory->Add( pNd->GetTextColl(), nNewPos, SwNodeType::Text );
+        m_pHistory->AddColl(pNd->GetTextColl(), nNewPos, SwNodeType::Text);
 
         SwTextNode* pNext = pEnd->GetNode().GetTextNode();
         SwNodeOffset nTmp = pNext->GetIndex();
@@ -621,7 +621,7 @@ SwUndoReplace::Impl::Impl(
                             pNext->GetText().getLength(), true );
         if( pNext->HasSwAttrSet() )
             m_pHistory->CopyFormatAttr( *pNext->GetpSwAttrSet(), nTmp );
-        m_pHistory->Add( pNext->GetTextColl(),nTmp, SwNodeType::Text );
+        m_pHistory->AddColl(pNext->GetTextColl(),nTmp, SwNodeType::Text);
         // METADATA: store
         m_pMetadataUndoStart = pNd  ->CreateUndo();
         m_pMetadataUndoEnd   = pNext->CreateUndo();
diff --git a/sw/source/core/undo/unmove.cxx b/sw/source/core/undo/unmove.cxx
index ea939fd027c4..0ea6443f611a 100644
--- a/sw/source/core/undo/unmove.cxx
+++ b/sw/source/core/undo/unmove.cxx
@@ -50,7 +50,7 @@ SwUndoMove::SwUndoMove( const SwPaM& rRange, const 
SwPosition& rMvPos )
 
     if( pTextNd )
     {
-        m_pHistory->Add( pTextNd->GetTextColl(), m_nSttNode, SwNodeType::Text 
);
+        m_pHistory->AddColl(pTextNd->GetTextColl(), m_nSttNode, 
SwNodeType::Text);
         if ( pTextNd->GetpSwpHints() )
         {
             m_pHistory->CopyAttr( pTextNd->GetpSwpHints(), m_nSttNode,
@@ -61,7 +61,7 @@ SwUndoMove::SwUndoMove( const SwPaM& rRange, const 
SwPosition& rMvPos )
     }
     if( pEndTextNd && pEndTextNd != pTextNd )
     {
-        m_pHistory->Add( pEndTextNd->GetTextColl(), m_nEndNode, 
SwNodeType::Text );
+        m_pHistory->AddColl(pEndTextNd->GetTextColl(), m_nEndNode, 
SwNodeType::Text);
         if ( pEndTextNd->GetpSwpHints() )
         {
             m_pHistory->CopyAttr( pEndTextNd->GetpSwpHints(), m_nEndNode,
@@ -74,7 +74,7 @@ SwUndoMove::SwUndoMove( const SwPaM& rRange, const 
SwPosition& rMvPos )
     pTextNd = rMvPos.GetNode().GetTextNode();
     if (nullptr != pTextNd)
     {
-        m_pHistory->Add( pTextNd->GetTextColl(), m_nMoveDestNode, 
SwNodeType::Text );
+        m_pHistory->AddColl(pTextNd->GetTextColl(), m_nMoveDestNode, 
SwNodeType::Text);
         if ( pTextNd->GetpSwpHints() )
         {
             m_pHistory->CopyAttr( pTextNd->GetpSwpHints(), m_nMoveDestNode,
diff --git a/sw/source/core/undo/untbl.cxx b/sw/source/core/undo/untbl.cxx
index 3092df0c2fd3..6f122e73b6da 100644
--- a/sw/source/core/undo/untbl.cxx
+++ b/sw/source/core/undo/untbl.cxx
@@ -384,7 +384,7 @@ SwTableToTextSave::SwTableToTextSave( SwDoc& rDoc, 
SwNodeOffset nNd, SwNodeOffse
     {
         m_pHstry.reset( new SwHistory );
 
-        m_pHstry->Add( pNd->GetTextColl(), nNd, SwNodeType::Text );
+        m_pHstry->AddColl(pNd->GetTextColl(), nNd, SwNodeType::Text);
         if ( pNd->GetpSwpHints() )
         {
             m_pHstry->CopyAttr( pNd->GetpSwpHints(), nNd, 0,
@@ -2036,7 +2036,7 @@ void SwUndoTableMerge::SaveCollection( const SwTableBox& 
rBox )
     if( !pCNd )
         pCNd = aIdx.GetNodes().GoNext( &aIdx );
 
-    m_pHistory->Add( pCNd->GetFormatColl(), aIdx.GetIndex(), 
pCNd->GetNodeType());
+    m_pHistory->AddColl(pCNd->GetFormatColl(), aIdx.GetIndex(), 
pCNd->GetNodeType());
     if( pCNd->HasSwAttrSet() )
         m_pHistory->CopyFormatAttr( *pCNd->GetpSwAttrSet(), aIdx.GetIndex() );
 }
commit aee059ffabf46b57c7eeea337d0ba5e337737a55
Author:     Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Tue Nov 21 15:22:23 2023 +0100
Commit:     Michael Stahl <michael.st...@allotropia.de>
CommitDate: Fri Nov 24 10:23:09 2023 +0100

    officecfg,sd: add Office::Security::Net::AllowInsecureImpressRemoteWiFi
    
    The IPRemoteServer listens on all interfaces and accepts connections
    with a "PIN", which is transmitted in the clear (without encryption),
    so it is not as secure as a naive user might assume and should probably
    not be used in public/untrusted networks.
    
    Make it possible to use the Impress remote with Bluetooth only, which
    appears to be more secure, by adding a new setting which defaults to
    disabled.
    
    The DiscoveryService which does mDNS/ZeroConf/Bonjour/Renezvous/whatever
    -it's-called-today should not be needed for Bluetooth.
    
    Adapt the RemoteServer code to the new situation that there can be a
    BluetoothServer but not a IPRemoteServer.
    
    Also add a checkbox to the "Slide Show Settings" dialog.
    
    Change-Id: I0cd3f23b616e23351f166bc9681b45786df8a26a
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159786
    Tested-by: Jenkins
    Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
    Reviewed-by: Michael Stahl <michael.st...@allotropia.de>

diff --git a/officecfg/registry/schema/org/openoffice/Office/Security.xcs 
b/officecfg/registry/schema/org/openoffice/Office/Security.xcs
index 67bd4078585f..de72566c0dfa 100644
--- a/officecfg/registry/schema/org/openoffice/Office/Security.xcs
+++ b/officecfg/registry/schema/org/openoffice/Office/Security.xcs
@@ -48,6 +48,12 @@
       <info>
         <desc>Specifies security aspects of network connections.</desc>
       </info>
+      <prop oor:name="AllowInsecureImpressRemoteWiFi" oor:type="xs:boolean" 
oor:nillable="false">
+        <info>
+          <desc>Allow using the Impress Remote via WiFi or other local IP 
network. The server will listen on all local interfaces, and the protocol lacks 
encryption (the PIN is transmitted in clear text).</desc>
+        </info>
+        <value>false</value>
+      </prop>
       <prop oor:name="AllowInsecureProtocols" oor:type="xs:boolean" 
oor:nillable="false">
         <info>
           <desc>Allow using insecure and/or unencrypted protocols such as 
HTTP, SMTP, FTP.</desc>
diff --git a/sd/source/ui/dlg/present.cxx b/sd/source/ui/dlg/present.cxx
index 0c2e4619dd7e..27b9ee345878 100644
--- a/sd/source/ui/dlg/present.cxx
+++ b/sd/source/ui/dlg/present.cxx
@@ -18,6 +18,7 @@
  */
 
 #include <officecfg/Office/Impress.hxx>
+#include <officecfg/Office/Security.hxx>
 #include <svl/itemset.hxx>
 #include <svl/intitem.hxx>
 #include <svl/eitem.hxx>
@@ -70,6 +71,7 @@ SdStartPresentationDlg::SdStartPresentationDlg(weld::Window* 
pWindow, const SfxI
     , 
m_xFtNavigationButtonsSize(m_xBuilder->weld_label("navbar_btn_size_label"))
     , m_xFrameEnableRemote(m_xBuilder->weld_frame("frameremote"))
     , m_xCbxEnableRemote(m_xBuilder->weld_check_button("enableremote"))
+    , 
m_xCbxEnableRemoteInsecure(m_xBuilder->weld_check_button("enableremoteinsecure"))
     , m_xLbConsole(m_xBuilder->weld_combo_box("console_cb"))
     , m_xFtMonitor(m_xBuilder->weld_label("presdisplay_label"))
     , m_xLBMonitor(m_xBuilder->weld_combo_box("presdisplay_cb"))
@@ -166,7 +168,11 @@ 
SdStartPresentationDlg::SdStartPresentationDlg(weld::Window* pWindow, const SfxI
         m_xLbConsole->set_active(PresenterConsoleMode::Windowed);
 
 #ifdef ENABLE_SDREMOTE
+    m_xCbxEnableRemote->connect_toggled( LINK(this, SdStartPresentationDlg, 
ChangeRemoteHdl) );
     
m_xCbxEnableRemote->set_active(officecfg::Office::Impress::Misc::Start::EnableSdremote::get());
+    ChangeRemoteHdl(*m_xCbxEnableRemote);
+    m_xCbxEnableRemoteInsecure->set_active(m_xCbxEnableRemote->get_active()
+        && 
officecfg::Office::Security::Net::AllowInsecureImpressRemoteWiFi::get());
 #else
     m_xFrameEnableRemote->hide();
 #endif
@@ -204,7 +210,8 @@ short SdStartPresentationDlg::run()
             m_xLbNavigationButtonsSize->get_active(), batch);
 
 #ifdef ENABLE_SDREMOTE
-    
officecfg::Office::Impress::Misc::Start::EnableSdremote::set(m_xCbxEnableRemote->get_active(),
 batch);
+        
officecfg::Office::Impress::Misc::Start::EnableSdremote::set(m_xCbxEnableRemote->get_active(),
 batch);
+        
officecfg::Office::Security::Net::AllowInsecureImpressRemoteWiFi::set(m_xCbxEnableRemoteInsecure->get_active(),
 batch);
 #endif
         batch->commit();
     }
@@ -339,6 +346,11 @@ void SdStartPresentationDlg::GetAttr( SfxItemSet& rAttr )
         pCustomShowList->Seek( nPos );
 }
 
+IMPL_LINK_NOARG(SdStartPresentationDlg, ChangeRemoteHdl, weld::Toggleable&, 
void)
+{
+    
m_xCbxEnableRemoteInsecure->set_sensitive(m_xCbxEnableRemote->get_active());
+}
+
 /**
  *      Handler: Enabled/Disabled Listbox "Dias"
  */
diff --git a/sd/source/ui/inc/present.hxx b/sd/source/ui/inc/present.hxx
index e7cc165f4c4e..0d626bc234d7 100644
--- a/sd/source/ui/inc/present.hxx
+++ b/sd/source/ui/inc/present.hxx
@@ -59,6 +59,7 @@ private:
     std::unique_ptr<weld::Label>       m_xFtNavigationButtonsSize;
     std::unique_ptr<weld::Frame> m_xFrameEnableRemote;
     std::unique_ptr<weld::CheckButton> m_xCbxEnableRemote;
+    std::unique_ptr<weld::CheckButton> m_xCbxEnableRemoteInsecure;
 
     std::unique_ptr<weld::ComboBox> m_xLbConsole;
 
@@ -70,6 +71,7 @@ private:
     std::unique_ptr<weld::Label> m_xMonitorExternal;
     std::unique_ptr<weld::Label> m_xExternal;
 
+    DECL_LINK(ChangeRemoteHdl, weld::Toggleable&, void);
     DECL_LINK(ChangeRangeHdl, weld::Toggleable&, void);
     DECL_LINK(ClickWindowPresentationHdl, weld::Toggleable&, void);
     void ChangePause();
diff --git a/sd/source/ui/remotecontrol/Server.cxx 
b/sd/source/ui/remotecontrol/Server.cxx
index 3f77758bf728..928f4ef7fc6a 100644
--- a/sd/source/ui/remotecontrol/Server.cxx
+++ b/sd/source/ui/remotecontrol/Server.cxx
@@ -12,6 +12,7 @@
 #include <vector>
 
 #include <officecfg/Office/Impress.hxx>
+#include <officecfg/Office/Security.hxx>
 
 #include <com/sun/star/container/XNameAccess.hpp>
 #include <com/sun/star/container/XNameContainer.hpp>
@@ -191,8 +192,8 @@ void IPRemoteServer::setup()
 void RemoteServer::presentationStarted( const css::uno::Reference<
                 css::presentation::XSlideShowController > &rController )
 {
-    if (!IPRemoteServer::spServer)
-        return;
+    // note this can be invoked even when there is no IPRemoteServer instance
+    // but there are communicators belonging to a BluetoothServer
     MutexGuard aGuard( sDataMutex );
     for ( const auto& rpCommunicator : sCommunicators )
     {
@@ -201,8 +202,6 @@ void RemoteServer::presentationStarted( const 
css::uno::Reference<
 }
 void RemoteServer::presentationStopped()
 {
-    if (!IPRemoteServer::spServer)
-        return;
     MutexGuard aGuard( sDataMutex );
     for ( const auto& rpCommunicator : sCommunicators )
     {
@@ -212,8 +211,6 @@ void RemoteServer::presentationStopped()
 
 void RemoteServer::removeCommunicator( Communicator const * mCommunicator )
 {
-    if (!IPRemoteServer::spServer)
-        return;
     MutexGuard aGuard( sDataMutex );
     auto aIt = std::find(sCommunicators.begin(), sCommunicators.end(), 
mCommunicator);
     if (aIt != sCommunicators.end())
@@ -352,7 +349,15 @@ void SdDLL::RegisterRemotes()
     sd::BluetoothServer::setup( &RemoteServer::sCommunicators );
 #endif
 
+    if 
(!officecfg::Office::Security::Net::AllowInsecureImpressRemoteWiFi::get())
+    {
+        SAL_WARN("desktop", "Impress remote WiFi is disabled by 
configuration");
+        return;
+    }
+
+    // this is the IP/WiFi server
     sd::IPRemoteServer::setup();
+    // assumption is that BluetoothServer doesn't need DiscoveryService
     sd::DiscoveryService::setup();
 }
 
diff --git a/sd/source/ui/view/drviews7.cxx b/sd/source/ui/view/drviews7.cxx
index d8dbbac0362d..a238ec7fd04b 100644
--- a/sd/source/ui/view/drviews7.cxx
+++ b/sd/source/ui/view/drviews7.cxx
@@ -33,6 +33,7 @@
 #include <editeng/sizeitem.hxx>
 #include <editeng/urlfieldhelper.hxx>
 #include <officecfg/Office/Impress.hxx>
+#include <officecfg/Office/Security.hxx>
 #include <svx/svxids.hrc>
 #include <svx/svdpagv.hxx>
 #include <svx/clipfmtitem.hxx>
@@ -1010,8 +1011,9 @@ void DrawViewShell::GetMenuState( SfxItemSet &rSet )
 #ifndef ENABLE_SDREMOTE
         bDisableSdremoteForGood = true;
 #endif
-        bDisableSdremoteForGood |= ! ( 
/*officecfg::Office::Common::Misc::ExperimentalMode::get() &&*/
-                                       
officecfg::Office::Impress::Misc::Start::EnableSdremote::get() );
+        bDisableSdremoteForGood |= 
!(officecfg::Office::Impress::Misc::Start::EnableSdremote::get()
+                                     && 
officecfg::Office::Security::Net::AllowInsecureImpressRemoteWiFi::get()
+                                       );
 
         // This dialog is only useful for TCP/IP remote control
         // which is unusual, under-tested and a security issue.
diff --git a/sd/uiconfig/simpress/ui/presentationdialog.ui 
b/sd/uiconfig/simpress/ui/presentationdialog.ui
index 9829dc38a43f..9a474200f366 100644
--- a/sd/uiconfig/simpress/ui/presentationdialog.ui
+++ b/sd/uiconfig/simpress/ui/presentationdialog.ui
@@ -777,7 +777,7 @@
                 <property name="label-xalign">0</property>
                 <property name="shadow-type">none</property>
                 <child>
-                  <!-- n-columns=2 n-rows=1 -->
+                  <!-- n-columns=2 n-rows=2 -->
                   <object class="GtkGrid" id="grid10">
                     <property name="visible">True</property>
                     <property name="can-focus">False</property>
@@ -816,6 +816,21 @@
                         <property name="top-attach">0</property>
                       </packing>
                     </child>
+                    <child>
+                      <object class="GtkCheckButton" id="enableremoteinsecure">
+                        <property name="label" translatable="yes" 
context="presentationdialog|enableremoteinsecure">Enable insecure WiFi 
connections</property>
+                        <property name="visible">True</property>
+                        <property name="can-focus">True</property>
+                        <property name="receives-default">False</property>
+                        <property name="tooltip-text" translatable="yes" 
context="presentationdialog|enableremoteinsecure|tooltip_text">In addition to 
Bluetooth connections, enable insecure and unencrypted connections via IP on 
all network interfaces. Not recommended in public settings.</property>
+                        <property name="use-underline">True</property>
+                        <property name="draw-indicator">True</property>
+                      </object>
+                      <packing>
+                        <property name="left-attach">0</property>
+                        <property name="top-attach">1</property>
+                      </packing>
+                    </child>
                   </object>
                 </child>
                 <child type="label">

Reply via email to