[QTBUG-27420] Make Q{Box,Grid,Form}Layout::takeAt() unparent a nested layout

QStackedLayout doesn't have support for QLayout, only QWidget, so
the issue doesn't arise there.

Reported-by: Johannes Schaub
Task-number: QTBUG-27420

Change-Id: I71f8d10a036918c16d8f8c9197a2ec61cd76cf01
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@digia.com>

diff --git a/src/widgets/kernel/qboxlayout.cpp b/src/widgets/kernel/qboxlayout.cpp
index d16c999..62c4e52 100644 (file)
--- a/src/widgets/kernel/qboxlayout.cpp
+++ b/src/widgets/kernel/qboxlayout.cpp
@@ -729,6 +729,12 @@ QLayoutItem *QBoxLayout::takeAt(int index)
     b->item = 0;
     delete b;
 
+    if (QLayout *l = item->layout()) {
+        // sanity check in case the user passed something weird to QObject::setParent()
+        if (l->parent() == this)
+            l->setParent(0);
+    }
+
     invalidate();
     return item;
 }

diff --git a/src/widgets/kernel/qformlayout.cpp b/src/widgets/kernel/qformlayout.cpp
index b726ff8..f875de1 100644 (file)
--- a/src/widgets/kernel/qformlayout.cpp
+++ b/src/widgets/kernel/qformlayout.cpp
@@ -1409,6 +1409,13 @@ QLayoutItem *QFormLayout::takeAt(int index)
     QLayoutItem *i = item->item;
     item->item = 0;
     delete item;
+
+    if (QLayout *l = i->layout()) {
+        // sanity check in case the user passed something weird to QObject::setParent()
+        if (l->parent() == this)
+            l->setParent(0);
+    }
+
     return i;
 }
 

diff --git a/src/widgets/kernel/qgridlayout.cpp b/src/widgets/kernel/qgridlayout.cpp
index b5cd746..eda9b9d 100644 (file)
--- a/src/widgets/kernel/qgridlayout.cpp
+++ b/src/widgets/kernel/qgridlayout.cpp
@@ -156,15 +156,20 @@ public:
             return 0;
     }
     inline QLayoutItem *takeAt(int index) {
-        QLayoutItem *item = 0;
+        Q_Q(QGridLayout);
         if (index < things.count()) {
-            QGridBox *b = things.takeAt(index);
-            if (b) {
-                item = b->takeItem();
+            if (QGridBox *b = things.takeAt(index)) {
+                QLayoutItem *item = b->takeItem();
+                if (QLayout *l = item->layout()) {
+                    // sanity check in case the user passed something weird to QObject::setParent()
+                    if (l->parent() == q)
+                        l->setParent(0);
+                }
                 delete b;
+                return item;
             }
         }
-        return item;
+        return 0;
     }
 
     void getItemPosition(int index, int *row, int *column, int *rowSpan, int *columnSpan) const {

diff --git a/tests/auto/widgets/kernel/qboxlayout/tst_qboxlayout.cpp b/tests/auto/widgets/kernel/qboxlayout/tst_qboxlayout.cpp
index 3564800..4f1615a 100644 (file)
--- a/tests/auto/widgets/kernel/qboxlayout/tst_qboxlayout.cpp
+++ b/tests/auto/widgets/kernel/qboxlayout/tst_qboxlayout.cpp
@@ -66,6 +66,7 @@ private slots:
     void setStyleShouldChangeSpacing();
 
     void taskQTBUG_7103_minMaxWidthNotRespected();
+    void taskQTBUG_27420_takeAtShouldUnparentLayout();
 };
 
 class CustomLayoutStyle : public QProxyStyle
@@ -273,5 +274,26 @@ void tst_QBoxLayout::taskQTBUG_7103_minMaxWidthNotRespected()
     QCOMPARE(label->height(), height);
 }
 
+void tst_QBoxLayout::taskQTBUG_27420_takeAtShouldUnparentLayout()
+{
+    QSharedPointer<QHBoxLayout> outer(new QHBoxLayout);
+    QPointer<QVBoxLayout> inner = new QVBoxLayout;
+
+    outer->addLayout(inner);
+    QCOMPARE(outer->count(), 1);
+    QCOMPARE(inner->parent(), outer.data());
+
+    QLayoutItem *item = outer->takeAt(0);
+    QCOMPARE(item->layout(), inner.data());
+    QVERIFY(!item->layout()->parent());
+
+    outer.reset();
+
+    if (inner)
+        delete item; // success: a taken item/layout should not be deleted when the old parent is deleted
+    else
+        QVERIFY(!inner.isNull());
+}
+
 QTEST_MAIN(tst_QBoxLayout)
 #include "tst_qboxlayout.moc"

diff --git a/tests/auto/widgets/kernel/qformlayout/tst_qformlayout.cpp b/tests/auto/widgets/kernel/qformlayout/tst_qformlayout.cpp
index 3aa0ea0..b2cdb87 100644 (file)
--- a/tests/auto/widgets/kernel/qformlayout/tst_qformlayout.cpp
+++ b/tests/auto/widgets/kernel/qformlayout/tst_qformlayout.cpp
@@ -51,6 +51,7 @@
 #include <QtWidgets/QLineEdit>
 #include <QtWidgets/QPushButton>
 #include <QStyleFactory>
+#include <QSharedPointer>
 
 #include <qformlayout.h>
 
@@ -123,6 +124,8 @@ private slots:
     Qt::Orientations expandingDirections() const;
 */
 
+    void taskQTBUG_27420_takeAtShouldUnparentLayout();
+
 };
 
 tst_QFormLayout::tst_QFormLayout()
@@ -901,6 +904,27 @@ void tst_QFormLayout::layoutAlone()
     QTest::qWait(500);
 }
 
+void tst_QFormLayout::taskQTBUG_27420_takeAtShouldUnparentLayout()
+{
+    QSharedPointer<QFormLayout> outer(new QFormLayout);
+    QPointer<QFormLayout> inner = new QFormLayout;
+
+    outer->addRow(inner);
+    QCOMPARE(outer->count(), 1);
+    QCOMPARE(inner->parent(), outer.data());
+
+    QLayoutItem *item = outer->takeAt(0);
+    QCOMPARE(item->layout(), inner.data());
+    QVERIFY(!item->layout()->parent());
+
+    outer.reset();
+
+    if (inner)
+        delete item; // success: a taken item/layout should not be deleted when the old parent is deleted
+    else
+        QVERIFY(!inner.isNull());
+}
+
 QTEST_MAIN(tst_QFormLayout)
 
 #include "tst_qformlayout.moc"

diff --git a/tests/auto/widgets/kernel/qgridlayout/tst_qgridlayout.cpp b/tests/auto/widgets/kernel/qgridlayout/tst_qgridlayout.cpp
index 34b38c1..e52ff3f 100644 (file)
--- a/tests/auto/widgets/kernel/qgridlayout/tst_qgridlayout.cpp
+++ b/tests/auto/widgets/kernel/qgridlayout/tst_qgridlayout.cpp
@@ -52,6 +52,7 @@
 #include <QtWidgets/QLineEdit>
 #include <QtWidgets/QRadioButton>
 #include <QStyleFactory>
+#include <QSharedPointer>
 
 class tst_QGridLayout : public QObject
 {
@@ -89,6 +90,8 @@ private slots:
     void contentsRect();
     void distributeMultiCell();
 
+    void taskQTBUG_27420_takeAtShouldUnparentLayout();
+
 private:
     QWidget *testWidget;
     QGridLayout *testLayout;
@@ -1605,5 +1608,26 @@ void tst_QGridLayout::distributeMultiCell()
     QCOMPARE(w.sizeHint().height(), 11 + 57 + 11);
 }
 
+void tst_QGridLayout::taskQTBUG_27420_takeAtShouldUnparentLayout()
+{
+    QSharedPointer<QGridLayout> outer(new QGridLayout);
+    QPointer<QGridLayout> inner = new QGridLayout;
+
+    outer->addLayout(inner, 0, 0);
+    QCOMPARE(outer->count(), 1);
+    QCOMPARE(inner->parent(), outer.data());
+
+    QLayoutItem *item = outer->takeAt(0);
+    QCOMPARE(item->layout(), inner.data());
+    QVERIFY(!item->layout()->parent());
+
+    outer.reset();
+
+    if (inner)
+        delete item; // success: a taken item/layout should not be deleted when the old parent is deleted
+    else
+        QVERIFY(!inner.isNull());
+}
+
 QTEST_MAIN(tst_QGridLayout)
 #include "tst_qgridlayout.moc"