From c42dfe1ef3cc53f7e7457ec76a43681d72bc6f24 Mon Sep 17 00:00:00 2001
From: wmayer <wmayer@freecad.org>
Date: Thu, 23 Jan 2025 23:49:13 +0100
Subject: [PATCH] Gui: Fix undo/redo behaviour in transformation tool

If the user clicks on undo while the transformation dialog is
open the currently active transaction will be closed.
From now on any change of the placement won't be recorded any
more so that e.g. canceling the dialog or an undo after
accepting the dialog leads to unexpected behaviour.

To fix the issue two new virtual methods onUndo() and onRedo()
are added to TaskDialog and reimplemented in TaskTransformDialog.
These functions make sure to open a new transaction.

This fixes issue 19152
---
 src/Gui/TaskTransform.cpp       | 32 ++++++++++++++++++++++----------
 src/Gui/TaskTransform.h         |  5 +++++
 src/Gui/TaskView/TaskDialog.cpp | 10 ++++++++++
 src/Gui/TaskView/TaskDialog.h   |  6 +++++-
 src/Gui/TaskView/TaskView.cpp   | 12 ++++++++----
 src/Gui/TaskView/TaskView.h     |  2 +-
 6 files changed, 51 insertions(+), 16 deletions(-)

diff --git a/src/Gui/TaskTransform.cpp b/src/Gui/TaskTransform.cpp
index fddb2dc46a..2c04213b52 100644
--- a/src/Gui/TaskTransform.cpp
+++ b/src/Gui/TaskTransform.cpp
@@ -728,16 +728,31 @@ void TaskTransformDialog::open()
 
     Gui::TaskView::TaskDialog::open();
 
-    Gui::Application::Instance->activeDocument()->openCommand(
-        QT_TRANSLATE_NOOP("Command", "Transform"));
+    openCommand();
+}
+
+void TaskTransformDialog::openCommand()
+{
+    if (auto document = vp->getDocument()) {
+        if (!document->hasPendingCommand()) {
+            document->openCommand(QT_TRANSLATE_NOOP("Command", "Transform"));
+        }
+    }
+}
+
+void TaskTransformDialog::onUndo()
+{
+    openCommand();
+}
+
+void TaskTransformDialog::onRedo()
+{
+    openCommand();
 }
 
 bool TaskTransformDialog::accept()
 {
-    if (auto documentObject = vp->getObject()) {
-        Gui::Document* document =
-            Gui::Application::Instance->getDocument(documentObject->getDocument());
-        assert(document);
+    if (auto document = vp->getDocument()) {
         document->commitCommand();
         document->resetEdit();
         document->getDocument()->recompute();
@@ -748,10 +763,7 @@ bool TaskTransformDialog::accept()
 
 bool TaskTransformDialog::reject()
 {
-    if (auto documentObject = vp->getObject()) {
-        Gui::Document* document =
-            Gui::Application::Instance->getDocument(documentObject->getDocument());
-        assert(document);
+    if (auto document = vp->getDocument()) {
         document->abortCommand();
         document->resetEdit();
         document->getDocument()->recompute();
diff --git a/src/Gui/TaskTransform.h b/src/Gui/TaskTransform.h
index 746f12e43b..c8f144e673 100644
--- a/src/Gui/TaskTransform.h
+++ b/src/Gui/TaskTransform.h
@@ -166,6 +166,11 @@ public:
     void open() override;
     bool accept() override;
     bool reject() override;
+    void onUndo() override;
+    void onRedo() override;
+
+private:
+    void openCommand();
 
 private:
     ViewProviderDragger* vp;
diff --git a/src/Gui/TaskView/TaskDialog.cpp b/src/Gui/TaskView/TaskDialog.cpp
index 5c5bceb088..aba148c36d 100644
--- a/src/Gui/TaskView/TaskDialog.cpp
+++ b/src/Gui/TaskView/TaskDialog.cpp
@@ -172,6 +172,16 @@ void TaskDialog::helpRequested()
 
 }
 
+void TaskDialog::onUndo()
+{
+
+}
+
+void TaskDialog::onRedo()
+{
+
+}
+
 
 
 
diff --git a/src/Gui/TaskView/TaskDialog.h b/src/Gui/TaskView/TaskDialog.h
index 41f6d6e54b..6162efa19d 100644
--- a/src/Gui/TaskView/TaskDialog.h
+++ b/src/Gui/TaskView/TaskDialog.h
@@ -162,8 +162,12 @@ public:
     virtual bool accept();
     /// is called by the framework if the dialog is rejected (Cancel)
     virtual bool reject();
-    /// is called by the framework if the user press the help button 
+    /// is called by the framework if the user press the help button
     virtual void helpRequested();
+    /// is called by the framework if the user press the undo button
+    virtual void onUndo();
+    /// is called by the framework if the user press the redo button
+    virtual void onRedo();
 
     void emitDestructionSignal() {
         Q_EMIT aboutToBeDestroyed();
diff --git a/src/Gui/TaskView/TaskView.cpp b/src/Gui/TaskView/TaskView.cpp
index 25ee639aed..37cee0a18d 100644
--- a/src/Gui/TaskView/TaskView.cpp
+++ b/src/Gui/TaskView/TaskView.cpp
@@ -503,11 +503,15 @@ void TaskView::slotViewClosed(const Gui::MDIView* view)
     }
 }
 
-void TaskView::transactionChangeOnDocument(const App::Document& doc)
+void TaskView::transactionChangeOnDocument(const App::Document& doc, bool undo)
 {
     if (ActiveDialog) {
+        std::string name = ActiveDialog->getDocumentName();
+        if (name == doc.getName()) {
+            undo ? ActiveDialog->onUndo() : ActiveDialog->onRedo();
+        }
+
         if (ActiveDialog->isAutoCloseOnTransactionChange()) {
-            std::string name = ActiveDialog->getDocumentName();
             if (name.empty()) {
                 Base::Console().warning(std::string("TaskView::transactionChangeOnDocument"),
                                         "No document name set\n");
@@ -527,12 +531,12 @@ void TaskView::transactionChangeOnDocument(const App::Document& doc)
 
 void TaskView::slotUndoDocument(const App::Document& doc)
 {
-    transactionChangeOnDocument(doc);
+    transactionChangeOnDocument(doc, true);
 }
 
 void TaskView::slotRedoDocument(const App::Document& doc)
 {
-    transactionChangeOnDocument(doc);
+    transactionChangeOnDocument(doc, false);
 }
 
 /// @cond DOXERR
diff --git a/src/Gui/TaskView/TaskView.h b/src/Gui/TaskView/TaskView.h
index f613c4d0d1..bf4537f1b7 100644
--- a/src/Gui/TaskView/TaskView.h
+++ b/src/Gui/TaskView/TaskView.h
@@ -187,7 +187,7 @@ private:
     void slotViewClosed(const Gui::MDIView*);
     void slotUndoDocument(const App::Document&);
     void slotRedoDocument(const App::Document&);
-    void transactionChangeOnDocument(const App::Document&);
+    void transactionChangeOnDocument(const App::Document&, bool undo);
 
 protected:
     void keyPressEvent(QKeyEvent* event) override;