sd/source/ui/view/sdview2.cxx | 19 ++++++++-----------
sd/source/ui/view/sdview3.cxx | 21 +++++++++++++++++++++
2 files changed, 29 insertions(+), 11 deletions(-)
New commits:
commit 491678690b5b40f446b40c368ec4b4423ee603c1
Author: Caolán McNamara <caol...@redhat.com>
AuthorDate: Tue Sep 7 16:46:38 2021 +0100
Commit: Caolán McNamara <caol...@redhat.com>
CommitDate: Tue Sep 7 19:54:03 2021 +0200
unbalanced [Beg/End]Undo possible on self-dnd in impress
repeatedly holding the mouse button on an object long enough to start a
drag and then trying again will eventually result in a situation where
View::StartDrag is called, triggering a gtk_drag_begin which neither
fails with drag-failure or succeeds with drag-end and so no
View::DragFinished gets called. This is possibly only an issue under
(Fedora 34) wayland.
When it happens it leaves an unbalanced BegUndo from StartDrag with no
EndUndo which will eventually be fatal if the user uses undo.
So remove the BegUndo from StartDrag because we can't be sure to get
a DragFinished and...
a) put the removal of original objects (when ACTION_MOVE) in
DragFinished in a simple BegUndo/EndUndo block of its own. Whether the
dnd is to another application or to ourself won't matter.
b) in View::InsertData detect if the action is a self-dnd and if so
put that in another BegUndo/EndUndo block and explicitly call
DragFinished before EndUndo to put a) within that group so a drag
of an object from one slide to the other gives a single "Drag and Drop"
move undo action listed for the copy and delete. DragFinished will
get called again but that's ok in this case and its 2nd call won't
affect the undo stack.
c) when this problem arises, later Drag attempts fail to do anything
because View::StartDrag returns early if a mpDragSrcMarkList from an
earlier attempt is set. Remove that guard and allow mpDragSrcMarkList
to be replaced after a silently failed drag attempt.
Change-Id: I3933663ff74f1ca99d9410b7752227ecf8d0da46
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/121787
Tested-by: Jenkins
Tested-by: Caolán McNamara <caol...@redhat.com>
Reviewed-by: Caolán McNamara <caol...@redhat.com>
diff --git a/sd/source/ui/view/sdview2.cxx b/sd/source/ui/view/sdview2.cxx
index 6ac19f72e1e7..5f305228c1e3 100644
--- a/sd/source/ui/view/sdview2.cxx
+++ b/sd/source/ui/view/sdview2.cxx
@@ -341,7 +341,7 @@ void View::DoPaste (::sd::Window* pWindow)
void View::StartDrag( const Point& rStartPos, vcl::Window* pWindow )
{
- if( !AreObjectsMarked() || !IsAction() || !mpViewSh || !pWindow ||
mpDragSrcMarkList )
+ if (!AreObjectsMarked() || !IsAction() || !mpViewSh || !pWindow)
return;
BrkAction();
@@ -359,17 +359,18 @@ void View::StartDrag( const Point& rStartPos,
vcl::Window* pWindow )
mpDragSrcMarkList.reset( new SdrMarkList(GetMarkedObjectList()) );
mnDragSrcPgNum = GetSdrPageView()->GetPage()->GetPageNum();
- if( IsUndoEnabled() )
- {
- OUString aStr(SdResId(STR_UNDO_DRAGDROP));
- BegUndo(aStr + " " + mpDragSrcMarkList->GetMarkDescription());
- }
CreateDragDataObject( this, *pWindow, rStartPos );
}
void View::DragFinished( sal_Int8 nDropAction )
{
const bool bUndo = IsUndoEnabled();
+ const bool bGroupUndo = bUndo && mpDragSrcMarkList;
+ if (bGroupUndo)
+ {
+ OUString aStr(SdResId(STR_UNDO_DRAGDROP));
+ BegUndo(aStr + " " + mpDragSrcMarkList->GetMarkDescription());
+ }
SdTransferable* pDragTransferable = SD_MOD()->pTransferDrag;
@@ -419,11 +420,7 @@ void View::DragFinished( sal_Int8 nDropAction )
if( pDragTransferable )
pDragTransferable->SetInternalMove( false );
- //This Undo appears to matches with the STR_UNDO_DRAGDROP Undo Start of
- //View::StartDrag But this DragFinished can be called without a matching
- //StartDrag. So use the existence of mpDragSrcMarkList as a flag that
- //this EndUndo has a matching BegUndo
- if (bUndo && mpDragSrcMarkList)
+ if (bGroupUndo)
EndUndo();
mnDragSrcPgNum = SDRPAGE_NOTFOUND;
mpDragSrcMarkList.reset();
diff --git a/sd/source/ui/view/sdview3.cxx b/sd/source/ui/view/sdview3.cxx
index 8bbb9ce7e24f..a7204648ee4d 100644
--- a/sd/source/ui/view/sdview3.cxx
+++ b/sd/source/ui/view/sdview3.cxx
@@ -293,17 +293,29 @@ bool View::InsertData( const TransferableDataHelper&
rDataHelper,
pImplementation = nullptr;
}
+ bool bSelfDND = false;
+
// try to get own transfer data
if( pImplementation )
{
if( SD_MOD()->pTransferClip == pImplementation )
pOwnData = SD_MOD()->pTransferClip;
else if( SD_MOD()->pTransferDrag == pImplementation )
+ {
pOwnData = SD_MOD()->pTransferDrag;
+ bSelfDND = true;
+ }
else if( SD_MOD()->pTransferSelection == pImplementation )
pOwnData = SD_MOD()->pTransferSelection;
}
+ const bool bGroupUndoFromDragWithDrop = bSelfDND && mpDragSrcMarkList &&
IsUndoEnabled();
+ if (bGroupUndoFromDragWithDrop)
+ {
+ OUString aStr(SdResId(STR_UNDO_DRAGDROP));
+ BegUndo(aStr + " " + mpDragSrcMarkList->GetMarkDescription());
+ }
+
// ImageMap?
if( !pOwnData && aDataHelper.HasFormat( SotClipboardFormatId::SVIM ) )
{
@@ -1523,6 +1535,15 @@ bool View::InsertData( const TransferableDataHelper&
rDataHelper,
mbIsDropAllowed = true;
rDnDAction = mnAction;
+ if (bGroupUndoFromDragWithDrop)
+ {
+ // this is called eventually by the underlying toolkit anyway in the
case of a self-dnd
+ // but we call it early in this case to group its undo actions into
this open dnd undo group
+ // and rely on that repeated calls to View::DragFinished are safe to do
+ DragFinished(mnAction);
+ EndUndo();
+ }
+
return bReturn;
}
