QmlDesigner.propertyEditor: crash fix for setting expressions

The problem was that the transaction was commited outside of the try {}
catch {} block (RAII)

1. I extended to the try block arround the fxObjectNode.setVariantProperty()
to avoid similar bugs
2. I force a commit of the transaction before leaving the try block

result:
If anything is wrong with the entered expression an exception is thrown
(like before) but now is is catched and we show a messagebox.

Reviewed-by: Kai Koehne
Task-number: QTCREATORBUG-4312

diff --git a/src/plugins/qmldesigner/components/propertyeditor/propertyeditor.cpp b/src/plugins/qmldesigner/components/propertyeditor/propertyeditor.cpp
index d67c8ea..50f3272 100644 (file)
--- a/src/plugins/qmldesigner/components/propertyeditor/propertyeditor.cpp
+++ b/src/plugins/qmldesigner/components/propertyeditor/propertyeditor.cpp
@@ -464,51 +464,57 @@ void PropertyEditor::changeExpression(const QString &name)
 
     RewriterTransaction transaction = beginRewriterTransaction();
 
-    QString underscoreName(name);
-    underscoreName.replace(QLatin1Char('.'), QLatin1Char('_'));
-
-    QmlObjectNode fxObjectNode(m_selectedNode);
-    PropertyEditorValue *value = qobject_cast<PropertyEditorValue*>(QDeclarativeMetaType::toQObject(m_currentType->m_backendValuesPropertyMap.value(underscoreName)));
-
-    if (fxObjectNode.modelNode().metaInfo().isValid() && fxObjectNode.modelNode().metaInfo().hasProperty(name)) {
-        if (fxObjectNode.modelNode().metaInfo().propertyTypeName(name) == QLatin1String("QColor")) {
-            if (QColor(value->expression().remove('"')).isValid()) {
-                fxObjectNode.setVariantProperty(name, QColor(value->expression().remove('"')));
-                return;
-            }
-        } else if (fxObjectNode.modelNode().metaInfo().propertyTypeName(name) == QLatin1String("bool")) {
-            if (value->expression().compare("false", Qt::CaseInsensitive) == 0 || value->expression().compare("true", Qt::CaseInsensitive) == 0) {
-                if (value->expression().compare("true", Qt::CaseInsensitive) == 0)
-                    fxObjectNode.setVariantProperty(name, true);
-                else
-                    fxObjectNode.setVariantProperty(name, false);
-                return;
-            }
-        } else if (fxObjectNode.modelNode().metaInfo().propertyTypeName(name) == QLatin1String("int")) {
-            bool ok;
-            int intValue = value->expression().toInt(&ok);
-            if (ok) {
-                fxObjectNode.setVariantProperty(name, intValue);
-                return;
-            }
-        } else if (fxObjectNode.modelNode().metaInfo().propertyTypeName(name) == QLatin1String("qreal")) {
-            bool ok;
-            qreal realValue = value->expression().toFloat(&ok);
-            if (ok) {
-                fxObjectNode.setVariantProperty(name, realValue);
-                return;
+    try {
+        QString underscoreName(name);
+        underscoreName.replace(QLatin1Char('.'), QLatin1Char('_'));
+
+        QmlObjectNode fxObjectNode(m_selectedNode);
+        PropertyEditorValue *value = qobject_cast<PropertyEditorValue*>(QDeclarativeMetaType::toQObject(m_currentType->m_backendValuesPropertyMap.value(underscoreName)));
+
+        if (fxObjectNode.modelNode().metaInfo().isValid() && fxObjectNode.modelNode().metaInfo().hasProperty(name)) {
+            if (fxObjectNode.modelNode().metaInfo().propertyTypeName(name) == QLatin1String("QColor")) {
+                if (QColor(value->expression().remove('"')).isValid()) {
+                    fxObjectNode.setVariantProperty(name, QColor(value->expression().remove('"')));
+                    transaction.commit(); //committing in the try block
+                    return;
+                }
+            } else if (fxObjectNode.modelNode().metaInfo().propertyTypeName(name) == QLatin1String("bool")) {
+                if (value->expression().compare("false", Qt::CaseInsensitive) == 0 || value->expression().compare("true", Qt::CaseInsensitive) == 0) {
+                    if (value->expression().compare("true", Qt::CaseInsensitive) == 0)
+                        fxObjectNode.setVariantProperty(name, true);
+                    else
+                        fxObjectNode.setVariantProperty(name, false);
+                    transaction.commit(); //committing in the try block
+                    return;
+                }
+            } else if (fxObjectNode.modelNode().metaInfo().propertyTypeName(name) == QLatin1String("int")) {
+                bool ok;
+                int intValue = value->expression().toInt(&ok);
+                if (ok) {
+                    fxObjectNode.setVariantProperty(name, intValue);
+                    transaction.commit(); //committing in the try block
+                    return;
+                }
+            } else if (fxObjectNode.modelNode().metaInfo().propertyTypeName(name) == QLatin1String("qreal")) {
+                bool ok;
+                qreal realValue = value->expression().toFloat(&ok);
+                if (ok) {
+                    fxObjectNode.setVariantProperty(name, realValue);
+                    transaction.commit(); //committing in the try block
+                    return;
+                }
             }
         }
-    }
 
-    if (!value) {
-        qWarning() << "PropertyEditor::changeExpression no value for " << underscoreName;
-        return;
-    }
+        if (!value) {
+            qWarning() << "PropertyEditor::changeExpression no value for " << underscoreName;
+            return;
+        }
 
-    try {
         if (fxObjectNode.expression(name) != value->expression() || !fxObjectNode.propertyAffectedByCurrentState(name))
             fxObjectNode.setBindingProperty(name, value->expression());
+
+        transaction.commit(); //committing in the try block
     }
 
     catch (RewritingException &e) {