From e3e5524f88a0919ad2491c8ce83d5104bfa9daed Mon Sep 17 00:00:00 2001
From: Amr Essam <amr.elsayed@qt.io>
Date: Fri, 11 Nov 2022 10:07:53 +0200
Subject: [PATCH] QmlDesigner: crash when dragging an effect twice to component
 in 2D

When an effect is dragged twice, same effect or other effect it caused
QDS to crash, in layer mode the effect is replaced,
otherwise effect is added upon previous effect

Task-number: QDS-8172
Change-Id: I2a3682b21492de0d0cc62bce9bd2965cedfbc826
Reviewed-by: Miikka Heikkinen <miikka.heikkinen@qt.io>
Reviewed-by: Mahmoud Badri <mahmoud.badri@qt.io>
---
 .../qmldesigner/components/formeditor/dragtool.cpp  |  7 +++----
 .../qmldesigner/designercore/include/qmlitemnode.h  |  6 +++---
 .../qmldesigner/designercore/model/qmlitemnode.cpp  | 13 +++++--------
 3 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/src/plugins/qmldesigner/components/formeditor/dragtool.cpp b/src/plugins/qmldesigner/components/formeditor/dragtool.cpp
index 1ab170fc1c3..fe345024182 100644
--- a/src/plugins/qmldesigner/components/formeditor/dragtool.cpp
+++ b/src/plugins/qmldesigner/components/formeditor/dragtool.cpp
@@ -230,8 +230,8 @@ void DragTool::dropEvent(const QList<QGraphicsItem *> &itemList, QGraphicsSceneD
         QString effectPath;
         const QStringList assetPaths = QString::fromUtf8(event->mimeData()
                                                          ->data(Constants::MIME_TYPE_ASSETS)).split(',');
-        for (auto &path : assetPaths) {
-            auto assetType = AssetsLibraryWidget::getAssetTypeAndData(path).first;
+        for (const QString &path : assetPaths) {
+            const QString assetType = AssetsLibraryWidget::getAssetTypeAndData(path).first;
             if (assetType == Constants::MIME_TYPE_ASSET_EFFECT) {
                 effectPath = path;
                 break;
@@ -266,8 +266,7 @@ void DragTool::dropEvent(const QList<QGraphicsItem *> &itemList, QGraphicsSceneD
                     return;
                 }
 
-                QmlItemNode effectNode = QmlItemNode::
-                        createQmlItemNodeForEffect(view(), parentQmlItemNode, effectName);
+                QmlItemNode::createQmlItemNodeForEffect(view(), parentQmlItemNode, effectName);
 
                 view()->setSelectedModelNodes({parentQmlItemNode});
                 view()->resetPuppet();
diff --git a/src/plugins/qmldesigner/designercore/include/qmlitemnode.h b/src/plugins/qmldesigner/designercore/include/qmlitemnode.h
index ada5887621a..61c38dd5042 100644
--- a/src/plugins/qmldesigner/designercore/include/qmlitemnode.h
+++ b/src/plugins/qmldesigner/designercore/include/qmlitemnode.h
@@ -60,9 +60,9 @@ public:
                                                  const QPointF &position,
                                                  NodeAbstractProperty parentproperty,
                                                  bool executeInTransaction = true);
-    static QmlItemNode createQmlItemNodeForEffect(AbstractView *view,
-                                                  const QmlItemNode &parentNode,
-                                                  const QString &effectName);
+    static void createQmlItemNodeForEffect(AbstractView *view,
+                                           const QmlItemNode &parentNode,
+                                           const QString &effectName);
     QList<QmlItemNode> children() const;
     QList<QmlObjectNode> resources() const;
     QList<QmlObjectNode> allDirectSubNodes() const;
diff --git a/src/plugins/qmldesigner/designercore/model/qmlitemnode.cpp b/src/plugins/qmldesigner/designercore/model/qmlitemnode.cpp
index 7a818808158..ca2bb3c5a51 100644
--- a/src/plugins/qmldesigner/designercore/model/qmlitemnode.cpp
+++ b/src/plugins/qmldesigner/designercore/model/qmlitemnode.cpp
@@ -5,6 +5,7 @@
 #include <metainfo.h>
 #include "nodelistproperty.h"
 #include "nodehints.h"
+#include "nodeproperty.h"
 #include "variantproperty.h"
 #include "bindingproperty.h"
 #include "qmlanchors.h"
@@ -161,9 +162,9 @@ static bool useLayerEffect()
     return settings->value(layerEffectEntry, true).toBool();
 }
 
-QmlItemNode QmlItemNode::createQmlItemNodeForEffect(AbstractView *view,
-                                                    const QmlItemNode &parentNode,
-                                                    const QString &effectName)
+void QmlItemNode::createQmlItemNodeForEffect(AbstractView *view,
+                                             const QmlItemNode &parentNode,
+                                             const QString &effectName)
 {
     QmlItemNode newQmlItemNode;
 
@@ -174,7 +175,7 @@ QmlItemNode QmlItemNode::createQmlItemNodeForEffect(AbstractView *view,
         if (!view->model()->hasImport(import, true, true))
             view->model()->changeImports({import}, {});
     } catch (const Exception &) {
-        QTC_ASSERT(false, return QmlItemNode());
+        QTC_ASSERT(false, return);
     }
 
     TypeName type(effectName.toUtf8());
@@ -190,10 +191,6 @@ QmlItemNode QmlItemNode::createQmlItemNodeForEffect(AbstractView *view,
     } else {
         parentNode.modelNode().variantProperty("layer.enabled").setValue(true);
     }
-
-    QTC_ASSERT(newQmlItemNode.isValid(), return QmlItemNode());
-
-    return  newQmlItemNode;
 }
 
 bool QmlItemNode::isValid() const