From 223066d4319d611e724da69089c8f49d525a2566 Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Fri, 24 Sep 2021 23:33:27 +0200 Subject: [PATCH] Don't clear focus if setParent doesn't change the parent QWidget::setParent might be called to change the window flags, without changing the parent. For those cases, we don't have to clear the focus. Decouple the newParent state from the wasCreated flag. In most places where newParent was tested, wasCreated was either tested previously and can't be false anyway, or the code executed is irrelevant for widgets that are not yet created (there can't be a paint manager). In the remaining case, test wasCreated explicitly to maintain existing logic. Add test for the cases where the previous code broke the focus, both for QWidget and QDialog. Fixes: QTBUG-93005 Pick-to: 6.2 Change-Id: I39dc179c2d348054de3927aa8b69eecef4935511 Reviewed-by: Richard Moe Gustavsen Reviewed-by: Doris Verria --- src/widgets/kernel/qwidget.cpp | 12 ++-- .../widgets/dialogs/qdialog/tst_qdialog.cpp | 18 +++++ .../widgets/kernel/qwidget/tst_qwidget.cpp | 72 +++++++++++++++++++ 3 files changed, 97 insertions(+), 5 deletions(-) diff --git a/src/widgets/kernel/qwidget.cpp b/src/widgets/kernel/qwidget.cpp index a92454547a..580e71ff22 100644 --- a/src/widgets/kernel/qwidget.cpp +++ b/src/widgets/kernel/qwidget.cpp @@ -10543,8 +10543,8 @@ void QWidget::setParent(QWidget *parent, Qt::WindowFlags f) }); #endif - bool resized = testAttribute(Qt::WA_Resized); - bool wasCreated = testAttribute(Qt::WA_WState_Created); + const bool resized = testAttribute(Qt::WA_Resized); + const bool wasCreated = testAttribute(Qt::WA_WState_Created); QWidget *oldtlw = window(); if (f & Qt::Window) // Frame geometry likely changes, refresh. @@ -10553,7 +10553,7 @@ void QWidget::setParent(QWidget *parent, Qt::WindowFlags f) QWidget *desktopWidget = nullptr; if (parent && parent->windowType() == Qt::Desktop) desktopWidget = parent; - bool newParent = (parent != parentWidget()) || !wasCreated || desktopWidget; + bool newParent = (parent != parentWidget()) || desktopWidget; if (newParent && parent && !desktopWidget) { if (testAttribute(Qt::WA_NativeWindow) && !QCoreApplication::testAttribute(Qt::AA_DontCreateNativeWidgetSiblings)) @@ -10572,7 +10572,9 @@ void QWidget::setParent(QWidget *parent, Qt::WindowFlags f) QCoreApplication::sendEvent(this, &e); } } - if (newParent && isAncestorOf(focusWidget())) + // If we get parented into another window, children will be folded + // into the new parent's focus chain, so clear focus now. + if (newParent && isAncestorOf(focusWidget()) && !(f & Qt::Window)) focusWidget()->clearFocus(); d->setParent_sys(parent, f); @@ -10619,7 +10621,7 @@ void QWidget::setParent(QWidget *parent, Qt::WindowFlags f) // event to handle recreation/rebinding of the GL context, hence the // (f & Qt::MSWindowsOwnDC) clause (which is set on QGLWidgets on all // platforms). - if (newParent + if (newParent || !wasCreated #if QT_CONFIG(opengles2) || (f & Qt::MSWindowsOwnDC) #endif diff --git a/tests/auto/widgets/dialogs/qdialog/tst_qdialog.cpp b/tests/auto/widgets/dialogs/qdialog/tst_qdialog.cpp index a509f1135a..e585835c3d 100644 --- a/tests/auto/widgets/dialogs/qdialog/tst_qdialog.cpp +++ b/tests/auto/widgets/dialogs/qdialog/tst_qdialog.cpp @@ -86,6 +86,7 @@ private slots: void virtualsOnClose(); void deleteOnDone(); void quitOnDone(); + void focusWidgetAfterOpen(); }; // Testing get/set functions @@ -738,5 +739,22 @@ void tst_QDialog::quitOnDone() QCOMPARE(quitSpy.count(), 1); } +void tst_QDialog::focusWidgetAfterOpen() +{ + QDialog dialog; + dialog.setLayout(new QVBoxLayout); + + QPushButton *pb1 = new QPushButton; + QPushButton *pb2 = new QPushButton; + dialog.layout()->addWidget(pb1); + dialog.layout()->addWidget(pb2); + + pb2->setFocus(); + QCOMPARE(dialog.focusWidget(), static_cast(pb2)); + + dialog.open(); + QCOMPARE(dialog.focusWidget(), static_cast(pb2)); +} + QTEST_MAIN(tst_QDialog) #include "tst_qdialog.moc" diff --git a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp index 2d77e4af93..8acba6748c 100644 --- a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp +++ b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp @@ -431,6 +431,9 @@ private slots: void deleteWindowInCloseEvent(); void quitOnClose(); + void setParentChangesFocus_data(); + void setParentChangesFocus(); + private: bool ensureScreenSize(int width, int height); @@ -12188,5 +12191,74 @@ void tst_QWidget::quitOnClose() QCOMPARE(quitSpy.count(), 2); } +void tst_QWidget::setParentChangesFocus_data() +{ + QTest::addColumn("initialType"); + QTest::addColumn("initialParent"); + QTest::addColumn("targetType"); + QTest::addColumn("targetParent"); + QTest::addColumn("reparentBeforeShow"); + QTest::addColumn("focusWidget"); + + for (const bool before : {true, false}) { + const char *tag = before ? "before" : "after"; + QTest::addRow("give dialog parent, %s", tag) + << Qt::Dialog << false << Qt::Dialog << true << before << "lineEdit"; + QTest::addRow("make dialog parentless, %s", tag) + << Qt::Dialog << true << Qt::Dialog << false << before << "lineEdit"; + QTest::addRow("dialog to sheet, %s", tag) + << Qt::Dialog << true << Qt::Sheet << true << before << "lineEdit"; + QTest::addRow("window to widget, %s", tag) + << Qt::Window << true << Qt::Widget << true << before << "windowEdit"; + QTest::addRow("widget to window, %s", tag) + << Qt::Widget << true << Qt::Window << true << before << "lineEdit"; + } +} + +void tst_QWidget::setParentChangesFocus() +{ + QFETCH(Qt::WindowType, initialType); + QFETCH(bool, initialParent); + QFETCH(Qt::WindowType, targetType); + QFETCH(bool, targetParent); + QFETCH(bool, reparentBeforeShow); + QFETCH(QString, focusWidget); + + QWidget window; + window.setObjectName("window"); + QLineEdit *windowEdit = new QLineEdit(&window); + windowEdit->setObjectName("windowEdit"); + windowEdit->setFocus(); + + std::unique_ptr secondary(new QWidget(initialParent ? &window : nullptr, initialType)); + secondary->setObjectName("secondary"); + QLineEdit *lineEdit = new QLineEdit(secondary.get()); + lineEdit->setObjectName("lineEdit"); + QPushButton *pushButton = new QPushButton(secondary.get()); + pushButton->setObjectName("pushButton"); + lineEdit->setFocus(); + + window.show(); + QVERIFY(QTest::qWaitForWindowActive(&window)); + + if (reparentBeforeShow) { + secondary->setParent(targetParent ? &window : nullptr, targetType); + // making a widget into a window doesn't set a focusWidget until shown + if (secondary->focusWidget()) + QCOMPARE(secondary->focusWidget()->objectName(), focusWidget); + } + secondary->show(); + QApplication::setActiveWindow(secondary.get()); + QVERIFY(QTest::qWaitForWindowActive(secondary.get())); + + if (!reparentBeforeShow) { + secondary->setParent(targetParent ? &window : nullptr, targetType); + secondary->show(); // reparenting hides, so show again + QApplication::setActiveWindow(secondary.get()); + QVERIFY(QTest::qWaitForWindowActive(secondary.get())); + } + QCOMPARE(QApplication::focusWidget()->objectName(), focusWidget); +} + QTEST_MAIN(tst_QWidget) #include "tst_qwidget.moc"