Make component.createObject require a parent argument

For graphical objects (the common case) a common mistake is to not
parent a dynamically created item. Since you almost always want to add a
parent, and it's hard for a beginner to diagnose this problem, a parent
is now a required argument and dealt with by the createObject function.

Task-number: QTBUG-10110

13 files changed, 39 insertions, 28 deletions

diff --git a/demos/declarative/samegame/SamegameCore/samegame.js b/demos/declarative/samegame/SamegameCore/samegame.js
index cc0a70d..f9c6184 100755
--- a/demos/declarative/samegame/SamegameCore/samegame.js
+++ b/demos/declarative/samegame/SamegameCore/samegame.js
@@ -176,14 +176,13 @@ function createBlock(column,row){
     // not be ready immediately. There is a statusChanged signal on the
     // component you could use if you want to wait to load remote files.
     if(component.status == Component.Ready){
-        var dynamicObject = component.createObject();
+        var dynamicObject = component.createObject(gameCanvas);
         if(dynamicObject == null){
             console.log("error creating block");
             console.log(component.errorsString());
             return false;
         }
         dynamicObject.type = Math.floor(Math.random() * 3);
-        dynamicObject.parent = gameCanvas;
         dynamicObject.x = column*gameCanvas.blockSize;
         dynamicObject.targetX = column*gameCanvas.blockSize;
         dynamicObject.targetY = row*gameCanvas.blockSize;
diff --git a/demos/declarative/snake/content/snake.js b/demos/declarative/snake/content/snake.js
index 102bd87..f5c231e 100644
--- a/demos/declarative/snake/content/snake.js
+++ b/demos/declarative/snake/content/snake.js
@@ -59,8 +59,7 @@ function startNewGame()
                     console.log("Still loading linkComponent");
                 continue;//TODO: Better error handling?
             }
-            var link = linkComponent.createObject();
-            link.parent = playfield;
+            var link = linkComponent.createObject(playfield);
             link.z = numRows * numColumns + 1 - i;
             link.type = i == 0 ? 2 : 0;
             link.spawned = false;
@@ -300,8 +299,7 @@ function createCookie(value) {
             console.log("Still loading cookieComponent");
         return;//TODO: Better error handling?
     }
-    cookie = cookieComponent.createObject();
-    cookie.parent = head.parent;
+    cookie = cookieComponent.createObject(head.parent);
     cookie.value = value;
     cookie.row = row;
     cookie.column = column;
diff --git a/doc/src/declarative/dynamicobjects.qdoc b/doc/src/declarative/dynamicobjects.qdoc
index dc0277d..2688ee5 100644
--- a/doc/src/declarative/dynamicobjects.qdoc
+++ b/doc/src/declarative/dynamicobjects.qdoc
@@ -69,7 +69,9 @@ This function takes the URL of the QML file as its only argument and returns
 a component object which can be used to create and load that QML file.
 
 Once you have a component you can use its \l {Component::createObject()}{createObject()} method to create an instance of
-the component. 
+the component. This function takes exactly one argument, which is the parent for the new item. Since graphical items will
+not appear on the scene without a parent, it is recommended that you set the parent this way. However, if you wish to set
+the parent later you can safely pass null to this function.
 
 Here is an example. Here is a \c Sprite.qml, which defines a simple QML component:
 
diff --git a/doc/src/declarative/globalobject.qdoc b/doc/src/declarative/globalobject.qdoc
index 7c27ae4..bc9830a 100644
--- a/doc/src/declarative/globalobject.qdoc
+++ b/doc/src/declarative/globalobject.qdoc
@@ -258,7 +258,10 @@ The methods and properties of the Component element are defined in its own
 page, but when using it dynamically only two methods are usually used.
 \c Component.createObject() returns the created object or \c null if there is an error.
 If there is an error, \l {Component::errorsString()}{Component.errorsString()} describes
-the error that occurred.
+the error that occurred. Note that createObject() takes exactly one argument, which is set
+to the parent of the created object. Graphical objects without a parent will not appear
+on the scene, but if you do not wish to parent the item at this point you can safely pass
+in null.
 
 If you want to just create an arbitrary string of QML, instead of
 loading a QML file, consider the \l{Qt.createQmlObject(string qml, object parent, string filepath)}{Qt.createQmlObject()} function.
diff --git a/doc/src/snippets/declarative/componentCreation.js b/doc/src/snippets/declarative/componentCreation.js
index be928f0..f6fb379 100644
--- a/doc/src/snippets/declarative/componentCreation.js
+++ b/doc/src/snippets/declarative/componentCreation.js
@@ -4,11 +4,10 @@ var sprite;
 
 function finishCreation() {
     if (component.status == Component.Ready) {
-        sprite = component.createObject();
+        sprite = component.createObject(appWindow);
         if (sprite == null) {
             // Error Handling
         } else {
-            sprite.parent = appWindow;
             sprite.x = 100;
             sprite.y = 100;
             // ...
@@ -32,13 +31,12 @@ else
 
 //![2]
 component = Qt.createComponent("Sprite.qml");
-sprite = component.createObject();
+sprite = component.createObject(appWindow);
 
 if (sprite == null) {
     // Error Handling
     console.log("Error loading component:", component.errorsString());
 } else {
-    sprite.parent = appWindow;
     sprite.x = 100;
     sprite.y = 100;
     // ...
@@ -47,5 +45,3 @@ if (sprite == null) {
 
 }
 
-createSpriteObjects();
-
diff --git a/doc/src/snippets/declarative/dynamicObjects.qml b/doc/src/snippets/declarative/dynamicObjects.qml
index dd55d78..6a8c927 100644
--- a/doc/src/snippets/declarative/dynamicObjects.qml
+++ b/doc/src/snippets/declarative/dynamicObjects.qml
@@ -21,8 +21,7 @@ Rectangle {
     }
 
     function createRectangle() {
-        var object = rectComponent.createObject();
-        object.parent = rootItem;
+        var object = rectComponent.createObject(rootItem);
     } 
 
     Component.onCompleted: createRectangle()
diff --git a/examples/declarative/dynamic/qml/itemCreation.js b/examples/declarative/dynamic/qml/itemCreation.js
index 3c1b975..08f5320 100644
--- a/examples/declarative/dynamic/qml/itemCreation.js
+++ b/examples/declarative/dynamic/qml/itemCreation.js
@@ -42,8 +42,7 @@ function loadComponent() {
 
 function createItem() {
     if (itemComponent.status == Component.Ready && draggedItem == null) {
-        draggedItem = itemComponent.createObject();
-        draggedItem.parent = window;
+        draggedItem = itemComponent.createObject(window);
         draggedItem.image = itemButton.image;
         draggedItem.x = xOffset;
         draggedItem.y = yOffset;
diff --git a/examples/declarative/tutorials/samegame/samegame2/samegame.js b/examples/declarative/tutorials/samegame/samegame2/samegame.js
index bcfb5b6..0dbe6a6 100644
--- a/examples/declarative/tutorials/samegame/samegame2/samegame.js
+++ b/examples/declarative/tutorials/samegame/samegame2/samegame.js
@@ -41,13 +41,12 @@ function createBlock(column, row) {
     // Loading and we should wait for the component's statusChanged() signal to
     // know when the file is downloaded and ready before calling createObject().
     if (component.status == Component.Ready) {
-        var dynamicObject = component.createObject();
+        var dynamicObject = component.createObject(background);
         if (dynamicObject == null) {
             console.log("error creating block");
             console.log(component.errorsString());
             return false;
         }
-        dynamicObject.parent = background;
         dynamicObject.x = column * blockSize;
         dynamicObject.y = row * blockSize;
         dynamicObject.width = blockSize;
diff --git a/examples/declarative/tutorials/samegame/samegame3/samegame.js b/examples/declarative/tutorials/samegame/samegame3/samegame.js
index 4256aee..e5ba6e0 100644
--- a/examples/declarative/tutorials/samegame/samegame3/samegame.js
+++ b/examples/declarative/tutorials/samegame/samegame3/samegame.js
@@ -38,14 +38,13 @@ function createBlock(column, row) {
     // Loading and we should wait for the component's statusChanged() signal to
     // know when the file is downloaded and ready before calling createObject().
     if (component.status == Component.Ready) {
-        var dynamicObject = component.createObject();
+        var dynamicObject = component.createObject(gameCanvas);
         if (dynamicObject == null) {
             console.log("error creating block");
             console.log(component.errorsString());
             return false;
         }
         dynamicObject.type = Math.floor(Math.random() * 3);
-        dynamicObject.parent = gameCanvas;
         dynamicObject.x = column * gameCanvas.blockSize;
         dynamicObject.y = row * gameCanvas.blockSize;
         dynamicObject.width = gameCanvas.blockSize;
diff --git a/examples/declarative/tutorials/samegame/samegame4/content/samegame.js b/examples/declarative/tutorials/samegame/samegame4/content/samegame.js
index 961cd66..159c9a7 100755
--- a/examples/declarative/tutorials/samegame/samegame4/content/samegame.js
+++ b/examples/declarative/tutorials/samegame/samegame4/content/samegame.js
@@ -49,14 +49,13 @@ function createBlock(column, row) {
     // Loading and we should wait for the component's statusChanged() signal to
     // know when the file is downloaded and ready before calling createObject().
     if (component.status == Component.Ready) {
-        var dynamicObject = component.createObject();
+        var dynamicObject = component.createObject(gameCanvas);
         if (dynamicObject == null) {
             console.log("error creating block");
             console.log(component.errorsString());
             return false;
         }
         dynamicObject.type = Math.floor(Math.random() * 3);
-        dynamicObject.parent = gameCanvas;
         dynamicObject.x = column * gameCanvas.blockSize;
         dynamicObject.targetX = column * gameCanvas.blockSize;
         dynamicObject.targetY = row * gameCanvas.blockSize;
diff --git a/src/declarative/qml/qdeclarativecomponent.cpp b/src/declarative/qml/qdeclarativecomponent.cpp
index 3ca0707..e7b9c9e 100644
--- a/src/declarative/qml/qdeclarativecomponent.cpp
+++ b/src/declarative/qml/qdeclarativecomponent.cpp
@@ -59,6 +59,7 @@
 #include <QFileInfo>
 #include <QtCore/qdebug.h>
 #include <QApplication>
+#include <QGraphicsObject>
 
 QT_BEGIN_NAMESPACE
 
@@ -557,8 +558,11 @@ QDeclarativeComponent::QDeclarativeComponent(QDeclarativeComponentPrivate &dd, Q
 /*!
     \internal
     A version of create which returns a scriptObject, for use in script
+
+    Sets graphics object parent because forgetting to do this is a frequent
+    and serious problem.
 */
-QScriptValue QDeclarativeComponent::createObject()
+QScriptValue QDeclarativeComponent::createObject(QObject* parent)
 {
     Q_D(QDeclarativeComponent);
     QDeclarativeContext* ctxt = creationContext();
@@ -567,6 +571,20 @@ QScriptValue QDeclarativeComponent::createObject()
     QObject* ret = create(ctxt);
     if (!ret)
         return QScriptValue(QScriptValue::NullValue);
+
+    QGraphicsObject* gobj = qobject_cast<QGraphicsObject*>(ret);
+    bool needParent = (gobj != 0);
+    if(parent){
+        ret->setParent(parent);
+        QGraphicsObject* gparent = qobject_cast<QGraphicsObject*>(parent);
+        if(gparent){
+            gobj->setParentItem(gparent);
+            needParent = false;
+        }
+    }
+    if(needParent)
+        qWarning("QDeclarativeComponent: Created graphical object was not placed in the graphics scene.");
+
     QDeclarativeEnginePrivate *priv = QDeclarativeEnginePrivate::get(d->engine);
     QDeclarativeData::get(ret, true)->setImplicitDestructible();
     return priv->objectClass->newQObject(ret, QMetaType::QObjectStar);
diff --git a/src/declarative/qml/qdeclarativecomponent.h b/src/declarative/qml/qdeclarativecomponent.h
index b078174..688e233 100644
--- a/src/declarative/qml/qdeclarativecomponent.h
+++ b/src/declarative/qml/qdeclarativecomponent.h
@@ -109,7 +109,7 @@ Q_SIGNALS:
 
 protected:
     QDeclarativeComponent(QDeclarativeComponentPrivate &dd, QObject* parent);
-    Q_INVOKABLE QScriptValue createObject();
+    Q_INVOKABLE QScriptValue createObject(QObject* parent);
 
 private:
     QDeclarativeComponent(QDeclarativeEngine *, QDeclarativeCompiledData *, int, int, QObject *parent);
diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/dynamicCreation.qml b/tests/auto/declarative/qdeclarativeecmascript/data/dynamicCreation.qml
index 3047e9b..7b132e1 100644
--- a/tests/auto/declarative/qdeclarativeecmascript/data/dynamicCreation.qml
+++ b/tests/auto/declarative/qdeclarativeecmascript/data/dynamicCreation.qml
@@ -11,7 +11,7 @@ MyQmlObject{
     function createTwo()
     {
         var component = Qt.createComponent('dynamicCreation.helper.qml');
-        obj.objectProperty = component.createObject();
+        obj.objectProperty = component.createObject(obj);
     }
 
     function createThree()
@@ -22,6 +22,6 @@ MyQmlObject{
     function dontCrash()
     {
         var component = Qt.createComponent('file-doesnt-exist.qml');
-        obj.objectProperty = component.createObject();
+        obj.objectProperty = component.createObject(obj);
     }
 }