From fc6120b8ddde85e1b6e45ac828fdb30c80da535a Mon Sep 17 00:00:00 2001
From: Pieter Hijma <pieterhijma@users.noreply.github.com>
Date: Wed, 3 Jul 2024 23:03:51 +0200
Subject: [PATCH] Gui: Prevent invalid editors in VarSet dialog

In the VarSet dialog, we can create an editor after the name and type
has been determined.  However, if the name is changed after an editor
has been created, the editor is invalid because the underlying property
has been removed.  In that case, the function onNameDetermined() should
clean up the invalid editor and this happens in most cases.
Unfortunately, it cannot handle the case in which a click happens on the
invalid editor itself.  This click should result in onNameDetermined() but
since the editor is already invalid, onNameDetermined() is triggered too
late.

The current commit solves this by listening for every change in the name
of the property and handle the editors accordingly.
---
 src/Gui/DlgAddPropertyVarSet.cpp | 24 +++++++++++++++++++-----
 src/Gui/DlgAddPropertyVarSet.h   |  2 +-
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/src/Gui/DlgAddPropertyVarSet.cpp b/src/Gui/DlgAddPropertyVarSet.cpp
index b3b477c3e5..bf97649bd4 100644
--- a/src/Gui/DlgAddPropertyVarSet.cpp
+++ b/src/Gui/DlgAddPropertyVarSet.cpp
@@ -154,7 +154,7 @@ void DlgAddPropertyVarSet::initializeWidgets(ViewProviderVarSet* viewProvider)
 
     connect(this, &QDialog::finished,
             this, [viewProvider](int result) { viewProvider->onFinished(result); });
-    connect(ui->lineEditName, &QLineEdit::editingFinished,
+    connect(ui->lineEditName, &QLineEdit::textChanged,
             this, &DlgAddPropertyVarSet::onNamePropertyDetermined);
 
     std::string title = "Add a property to " + varSet->getFullName();
@@ -179,7 +179,9 @@ void DlgAddPropertyVarSet::setOkEnabled(bool enabled)
 
 void DlgAddPropertyVarSet::clearEditors()
 {
+    bool beforeBlocked = ui->lineEditName->blockSignals(true);
     ui->lineEditName->clear();
+    ui->lineEditName->blockSignals(beforeBlocked);
     removeEditor();
     setOkEnabled(false);
     namePropertyToAdd.clear();
@@ -269,14 +271,26 @@ void DlgAddPropertyVarSet::createProperty(std::string& name, std::string& group)
     setOkEnabled(true);
 }
 
-void DlgAddPropertyVarSet::onNamePropertyDetermined()
+void DlgAddPropertyVarSet::onNamePropertyDetermined(const QString& text)
 {
     if (!namePropertyToAdd.empty()) {
-        // we were already adding a name, so remove that property
+        // We were already adding a property with this name.  We have to remove
+        // the property, the editor because it is associated with the property,
+        // and we have to abort the transaction.
         varSet->removeDynamicProperty(namePropertyToAdd.c_str());
+        removeEditor();
+        App::Document* doc = varSet->getDocument();
+        if (doc->hasPendingTransaction()) {
+            doc->abortTransaction();
+        }
+        namePropertyToAdd.clear();
     }
-    QString nameProperty = ui->lineEditName->text();
-    std::string name = nameProperty.toUtf8().constData();
+    if (text.isEmpty()) {
+        // We can not define a property, so we should not have an editor for the value.
+        clearEditors();
+        return;
+    }
+    std::string name = text.toUtf8().constData();
     std::string group = comboBoxGroup.currentText().toUtf8().constData();
     if(name.empty() || group.empty()
        || name != Base::Tools::getIdentifier(name)
diff --git a/src/Gui/DlgAddPropertyVarSet.h b/src/Gui/DlgAddPropertyVarSet.h
index d5909e14f3..ad69082d82 100644
--- a/src/Gui/DlgAddPropertyVarSet.h
+++ b/src/Gui/DlgAddPropertyVarSet.h
@@ -87,7 +87,7 @@ private:
     bool isSupportedType(std::string& type);
     void createProperty(std::string& name, std::string& group);
 
-    void onNamePropertyDetermined();
+    void onNamePropertyDetermined(const QString& text);
     void onGroupDetermined();
     void onTypePropertyDetermined();