diff options
author | Laszlo Agocs <laszlo.agocs@qt.io> | 2022-06-28 15:30:28 +0200 |
---|---|---|
committer | Laszlo Agocs <laszlo.agocs@qt.io> | 2022-07-01 22:38:45 +0000 |
commit | dab0ef367089869910c38fe772f31da14fd5386e (patch) | |
tree | 43b55e71181f08b9c31618a4ec2cb32734aa9f2a /tests/auto/widgets/kernel/qwidget_window/tst_qwidget_window.cpp | |
parent | 9ea2f7f4b1192f3429aa2d3e278097008bc773bb (diff) |
Harden drag and drop handling in widget window
User code in an event handler can do arbitrary things, including
operations that lead to destroying the QWidgetWindow. An example is
what the autotest does: reparenting the top-level widget to under
another top-level upon the drop. Internally this leads to destroying
the drop target's QWidgetWindow as the widget is now a child, not a
top-level.
In fact some of the existing drag and drop handling code seems to be
prepared to handle the case of having the drag target widget destroyed
in the user's event handler during a drag-move. But none of it is
prepared for having the QWidgetWindow destroyed upon returning from
forwardEvent().
The associated bug report has the same root cause, it is just popping up
now via the new 6.4 behavior: adding a QOpenGLWidget to a widget
hierarchy upon a drop leads to getting a new QWidgetWindow (if the
window only had regular raster widgets before).
To solve this, avoid touching members on 'this' after the
forwardEvent(). It looks like the handlers for mouse events follow
this pattern already, no member data is touched after forwarding events
(not sure if that is intentional or just incidental but it is the safe
solution, even if this is not feasible everywhere, but ideally input
events should take this into account).
Fixes: QTBUG-104596
Pick-to: 6.4 6.3 6.2
Change-Id: I96c704cadcd799fc5619b776e939dfdf313a27dd
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Diffstat (limited to 'tests/auto/widgets/kernel/qwidget_window/tst_qwidget_window.cpp')
-rw-r--r-- | tests/auto/widgets/kernel/qwidget_window/tst_qwidget_window.cpp | 73 |
1 files changed, 73 insertions, 0 deletions
diff --git a/tests/auto/widgets/kernel/qwidget_window/tst_qwidget_window.cpp b/tests/auto/widgets/kernel/qwidget_window/tst_qwidget_window.cpp index 689a65518e..09956e69a2 100644 --- a/tests/auto/widgets/kernel/qwidget_window/tst_qwidget_window.cpp +++ b/tests/auto/widgets/kernel/qwidget_window/tst_qwidget_window.cpp @@ -82,6 +82,7 @@ private slots: void tst_dnd(); void tst_dnd_events(); void tst_dnd_propagation(); + void tst_dnd_destroyOnDrop(); #endif void tst_qtbug35600(); @@ -917,6 +918,78 @@ void tst_QWidget_window::tst_dnd_propagation() QCOMPARE(target.mDndEvents, "enter leave enter drop "); } + +class ReparentSelfOnDropWidget : public QWidget +{ +public: + ReparentSelfOnDropWidget(QWidget *newFutureParent) + : m_newFutureParent(newFutureParent) + { + setAcceptDrops(true); + + const QRect availableGeometry = QGuiApplication::primaryScreen()->availableGeometry(); + auto width = availableGeometry.width() / 6; + auto height = availableGeometry.height() / 4; + + setGeometry(availableGeometry.x() + 200, availableGeometry.y() + 200, width, height); + + QLabel *label = new QLabel(QStringLiteral("Test"), this); + label->setGeometry(40, 40, 60, 60); + label->setAcceptDrops(true); + } + + void dragEnterEvent(QDragEnterEvent *event) override + { + event->accept(); + } + + void dragMoveEvent(QDragMoveEvent *event) override + { + event->acceptProposedAction(); + } + + void dropEvent(QDropEvent *event) override + { + event->accept(); + // Turn 'this' from a top-level widget to a child widget. + // This destroys the QWidgetWindow since the widget is no longer top-level. + setParent(m_newFutureParent); + } + +private: + QWidget *m_newFutureParent; +}; + +void tst_QWidget_window::tst_dnd_destroyOnDrop() +{ + if (QGuiApplication::platformName().startsWith(QLatin1String("wayland"), Qt::CaseInsensitive)) + QSKIP("Wayland: This fails. Figure out why."); + + QMimeData mimeData; + mimeData.setText(QLatin1String("testmimetext")); + + QWidget newParent; + newParent.resize(400, 400); + newParent.show(); + QVERIFY(QTest::qWaitForWindowActive(&newParent)); + + ReparentSelfOnDropWidget *target = new ReparentSelfOnDropWidget(&newParent); + target->show(); + QVERIFY(QTest::qWaitForWindowActive(target)); + + Qt::DropActions supportedActions = Qt::DropAction::CopyAction; + QWindow *window = target->windowHandle(); + + auto posInsideDropTarget = QHighDpi::toNativePixels(QPoint(20, 20), window->screen()); + auto posInsideLabel = QHighDpi::toNativePixels(QPoint(60, 60), window->screen()); + + QWindowSystemInterface::handleDrag(window, &mimeData, posInsideDropTarget, supportedActions, {}, {}); + QWindowSystemInterface::handleDrag(window, &mimeData, posInsideLabel, supportedActions, {}, {}); + QWindowSystemInterface::handleDrop(window, &mimeData, posInsideLabel, supportedActions, {}, {}); + + QGuiApplication::processEvents(); +} + #endif void tst_QWidget_window::tst_qtbug35600() |