diff --git a/src/App/Document.cpp b/src/App/Document.cpp index ecf12cc3a7..d843343c9b 100644 --- a/src/App/Document.cpp +++ b/src/App/Document.cpp @@ -3758,6 +3758,7 @@ void Document::removeObject(const char* sName) d->objectIdMap.erase(pos->second->_Id); // Unset the bit to be on the safe side pos->second->setStatus(ObjectStatus::Remove, false); + unregisterLabel(pos->second->Label.getStrValue()); // do no transactions if we do a rollback! std::unique_ptr tobedestroyed; @@ -3775,7 +3776,6 @@ void Document::removeObject(const char* sName) } } - unregisterLabel(pos->second->Label.getStrValue()); for (std::vector::iterator obj = d->objectArray.begin(); obj != d->objectArray.end(); ++obj) { @@ -3855,6 +3855,11 @@ void Document::_removeObject(DocumentObject* pcObject) signalTransactionRemove(*pcObject, 0); breakDependency(pcObject, true); } + // TODO: Transaction::addObjectName could potentially have freed (deleted) pcObject so some of the following + // code may be dereferencing a pointer to a deleted object which is not legal. if (d->rollback) this does not occur + // and instead pcObject is deleted at the end of this function. + // This either should be fixed, perhaps by moving the following lines up in the code, + // or there should be a comment explaining why the object will never be deleted because of the logic that got us here. // remove from map pcObject->setStatus(ObjectStatus::Remove, false); // Unset the bit to be on the safe side diff --git a/src/App/PropertyStandard.cpp b/src/App/PropertyStandard.cpp index ac6fe61e51..c1c8e2c95c 100644 --- a/src/App/PropertyStandard.cpp +++ b/src/App/PropertyStandard.cpp @@ -1425,23 +1425,27 @@ PropertyString::PropertyString() = default; PropertyString::~PropertyString() = default; -void PropertyString::setValue(const char* newLabel) +void PropertyString::setValue(const char* newValue) { - if (!newLabel) { + if (!newValue) { return; } - if (_cValue == newLabel) { + if (_cValue == newValue) { return; } std::vector>> propChanges; - std::string label = newLabel; + std::string newValueStr = newValue; auto obj = dynamic_cast(getContainer()); bool commit = false; if (obj && this == &obj->Label) { - propChanges = obj->onProposedLabelChange(label); + propChanges = obj->onProposedLabelChange(newValueStr); + if (_cValue == newValueStr) { + // OnProposedLabelChange has changed the new value to what the current value is + return; + } if (!propChanges.empty() && !GetApplication().getActiveTransaction()) { commit = true; std::ostringstream str; @@ -1451,7 +1455,7 @@ void PropertyString::setValue(const char* newLabel) } aboutToSetValue(); - _cValue = label; + _cValue = newValueStr; hasSetValue(); for (auto& change : propChanges) {