1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Dialogs: clean up native dialogs when object gets destroyed

Browse Source 
QWidget::setVisible is virtual, and called via hide() by both the
QDialog and the QWidget destructor. A dialog that becomes invisible
when getting destroyed will at most execute the QDialog override.
Subclassing QDialog and overriding setVisible() to update the state
of the native platform dialog will not work, unless we explicitly
call hide() in the respective subclass's destructor.

Since e0bb9e81ab1a9d71f2893844ea82, QDialogPrivate::setVisible is
also virtual, and gets called by QDialog::setVisible. So the clean
solution is to move the implementation of the native dialog status
update into an override of QDialogPrivate::setVisible.

Add test that verifies that the transient parent of the dialog
becomes inactive when the (native) dialog shows (and skip if that
fails), and then becomes active again when the (native) dialog is
closed through the destructor of the Q*Dialog class. The test of
QFileDialog has to be skipped on Android for the same reason as the
widgetlessNativeDialog.

Fixes: QTBUG-116277
Change-Id: Ie3f93980d8653b8d933bf70aac3ef90de606f0ef
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
(cherry picked from commit 505ed52cd4dcef081d9868424057451bd1dce497)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Volker Hilsheimer 2023-08-30 16:49:16 +02:00 committed by Qt Cherry-pick Bot
parent 864544eb1a
commit 9044a8e035
10 changed files with 224 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
src/widgets/dialogs/qcolordialog.cpp
View File

@ -120,6 +120,7 @@ public:
bool handleColorPickingKeyPress(QKeyEvent *e);
bool canBeNativeDialog() const override;
void setVisible(bool visible) override;
QWellArray *custom;
QWellArray *standard;
@ -2136,30 +2137,42 @@ QColorDialog::ColorDialogOptions QColorDialog::options() const
*/
void QColorDialog::setVisible(bool visible)
{
Q_D(QColorDialog);
// will call QColorDialogPrivate::setVisible override
QDialog::setVisible(visible);
}
/*!
\internal
The implementation of QColorDialog::setVisible() has to live here so that the call
to hide() in ~QDialog calls this function; it wouldn't call the override of
QDialog::setVisible().
*/
void QColorDialogPrivate::setVisible(bool visible)
{
Q_Q(QColorDialog);
if (visible){
if (testAttribute(Qt::WA_WState_ExplicitShowHide) && !testAttribute(Qt::WA_WState_Hidden))
if (q->testAttribute(Qt::WA_WState_ExplicitShowHide) && !q->testAttribute(Qt::WA_WState_Hidden))
return;
} else if (testAttribute(Qt::WA_WState_ExplicitShowHide) && testAttribute(Qt::WA_WState_Hidden))
} else if (q->testAttribute(Qt::WA_WState_ExplicitShowHide) && q->testAttribute(Qt::WA_WState_Hidden))
return;
if (visible)
d->selectedQColor = QColor();
selectedQColor = QColor();
if (d->nativeDialogInUse) {
if (d->setNativeDialogVisible(visible)) {
if (nativeDialogInUse) {
if (setNativeDialogVisible(visible)) {
// Set WA_DontShowOnScreen so that QDialog::setVisible(visible) below
// updates the state correctly, but skips showing the non-native version:
setAttribute(Qt::WA_DontShowOnScreen);
q->setAttribute(Qt::WA_DontShowOnScreen);
} else {
d->initWidgets();
initWidgets();
}
} else {
setAttribute(Qt::WA_DontShowOnScreen, false);
q->setAttribute(Qt::WA_DontShowOnScreen, false);
}
QDialog::setVisible(visible);
QDialogPrivate::setVisible(visible);
}
/*!
@ -2207,7 +2220,6 @@ QColor QColorDialog::getColor(const QColor &initial, QWidget *parent, const QStr
QColorDialog::~QColorDialog()
{
}
/*!

48
src/widgets/dialogs/qfiledialog.cpp
View File

@ -835,41 +835,53 @@ void QFileDialog::open(QObject *receiver, const char *member)
*/
void QFileDialog::setVisible(bool visible)
{
Q_D(QFileDialog);
// will call QFileDialogPrivate::setVisible override
QDialog::setVisible(visible);
}
/*!
\internal
The logic has to live here so that the call to hide() in ~QDialog calls
this function; it wouldn't call an override of QDialog::setVisible().
*/
void QFileDialogPrivate::setVisible(bool visible)
{
Q_Q(QFileDialog);
if (visible){
if (testAttribute(Qt::WA_WState_ExplicitShowHide) && !testAttribute(Qt::WA_WState_Hidden))
if (q->testAttribute(Qt::WA_WState_ExplicitShowHide) && !q->testAttribute(Qt::WA_WState_Hidden))
return;
} else if (testAttribute(Qt::WA_WState_ExplicitShowHide) && testAttribute(Qt::WA_WState_Hidden))
} else if (q->testAttribute(Qt::WA_WState_ExplicitShowHide) && q->testAttribute(Qt::WA_WState_Hidden))
return;
if (d->canBeNativeDialog()){
if (d->setNativeDialogVisible(visible)){
// Set WA_DontShowOnScreen so that QDialog::setVisible(visible) below
if (canBeNativeDialog()){
if (setNativeDialogVisible(visible)){
// Set WA_DontShowOnScreen so that QDialogPrivate::setVisible(visible) below
// updates the state correctly, but skips showing the non-native version:
setAttribute(Qt::WA_DontShowOnScreen);
q->setAttribute(Qt::WA_DontShowOnScreen);
#if QT_CONFIG(fscompleter)
// So the completer doesn't try to complete and therefore show a popup
if (!d->nativeDialogInUse)
d->completer->setModel(nullptr);
if (!nativeDialogInUse)
completer->setModel(nullptr);
#endif
} else {
d->createWidgets();
setAttribute(Qt::WA_DontShowOnScreen, false);
createWidgets();
q->setAttribute(Qt::WA_DontShowOnScreen, false);
#if QT_CONFIG(fscompleter)
if (!d->nativeDialogInUse) {
if (d->proxyModel != nullptr)
d->completer->setModel(d->proxyModel);
if (!nativeDialogInUse) {
if (proxyModel != nullptr)
completer->setModel(proxyModel);
else
d->completer->setModel(d->model);
completer->setModel(model);
}
#endif
}
}
if (visible && d->usingWidgets())
d->qFileDialogUi->fileNameEdit->setFocus();
if (visible && usingWidgets())
qFileDialogUi->fileNameEdit->setFocus();
QDialog::setVisible(visible);
QDialogPrivate::setVisible(visible);
}
/*!

1
src/widgets/dialogs/qfiledialog_p.h
View File

@ -221,6 +221,7 @@ public:
// dialog. Returning false means that a non-native dialog must be
// used instead.
bool canBeNativeDialog() const override;
void setVisible(bool visible) override;
inline bool usingWidgets() const;
inline void setDirectory_sys(const QUrl &directory);

29
src/widgets/dialogs/qfontdialog.cpp
View File

@ -960,19 +960,32 @@ void QFontDialog::open(QObject *receiver, const char *member)
*/
void QFontDialog::setVisible(bool visible)
{
if (testAttribute(Qt::WA_WState_ExplicitShowHide) && testAttribute(Qt::WA_WState_Hidden) != visible)
// will call QFontDialogPrivate::setVisible
QDialog::setVisible(visible);
}
/*!
\internal
The implementation of QFontDialog::setVisible() has to live here so that the call
to hide() in ~QDialog calls this function; it wouldn't call the override of
QDialog::setVisible().
*/
void QFontDialogPrivate::setVisible(bool visible)
{
Q_Q(QFontDialog);
if (q->testAttribute(Qt::WA_WState_ExplicitShowHide) && q->testAttribute(Qt::WA_WState_Hidden) != visible)
return;
Q_D(QFontDialog);
if (d->canBeNativeDialog())
d->setNativeDialogVisible(visible);
if (d->nativeDialogInUse) {
if (canBeNativeDialog())
setNativeDialogVisible(visible);
if (nativeDialogInUse) {
// Set WA_DontShowOnScreen so that QDialog::setVisible(visible) below
// updates the state correctly, but skips showing the non-native version:
setAttribute(Qt::WA_DontShowOnScreen, true);
q->setAttribute(Qt::WA_DontShowOnScreen, true);
} else {
setAttribute(Qt::WA_DontShowOnScreen, false);
q->setAttribute(Qt::WA_DontShowOnScreen, false);
}
QDialog::setVisible(visible);
QDialogPrivate::setVisible(visible);
}
/*!

1
src/widgets/dialogs/qfontdialog_p.h
View File

@ -105,6 +105,7 @@ public:
QByteArray memberToDisconnectOnClose;
bool canBeNativeDialog() const override;
void setVisible(bool visible) override;
void _q_runNativeAppModalPanel();
private:

17
tests/auto/printsupport/dialogs/qabstractprintdialog/tst_qabstractprintdialog.cpp
View File

@ -146,9 +146,26 @@ void tst_QAbstractPrintDialog::hideNativeByDestruction()
QWidget window;
QWidget *child = new QWidget(&window);
QPointer<QPrintDialog> dialog = new QPrintDialog(child);
// Make it application modal so that we don't end up with a sheet on macOS
dialog->setWindowModality(Qt::ApplicationModal);
window.show();
QVERIFY(QTest::qWaitForWindowActive(&window));
dialog->open();
// We test that the dialog opens and closes by watching the activation of the
// transient parent window. If it doesn't deactivate, then we have to skip.
const auto windowActive = [&window]{ return window.isActiveWindow(); };
const auto windowInactive = [&window]{ return !window.isActiveWindow(); };
if (!QTest::qWaitFor(windowInactive, 2000))
QSKIP("Dialog didn't activate");
// This should destroy the dialog and close the native window
child->deleteLater();
QTRY_VERIFY(!dialog);
// If the native window is still open, then the transient parent can't become
// active
window.activateWindow();
QVERIFY(QTest::qWaitFor(windowActive));
}
#endif

29
tests/auto/widgets/dialogs/qcolordialog/tst_qcolordialog.cpp
View File

@ -29,6 +29,8 @@ private slots:
void QTBUG_43548_initialColor();
void hexColor_data();
void hexColor();
void hideNativeByDestruction();
};
class TestNativeDialog : public QColorDialog
@ -179,5 +181,32 @@ void tst_QColorDialog::hexColor()
QCOMPARE(color.name(QColor::HexRgb), expectedHexColor.toLower());
}
void tst_QColorDialog::hideNativeByDestruction()
{
QWidget window;
QWidget *child = new QWidget(&window);
QPointer<QColorDialog> dialog = new QColorDialog(child);
// Make it application modal so that we don't end up with a sheet on macOS
dialog->setWindowModality(Qt::ApplicationModal);
window.show();
QVERIFY(QTest::qWaitForWindowActive(&window));
dialog->open();
// We test that the dialog opens and closes by watching the activation of the
// transient parent window. If it doesn't deactivate, then we have to skip.
const auto windowActive = [&window]{ return window.isActiveWindow(); };
const auto windowInactive = [&window]{ return !window.isActiveWindow(); };
if (!QTest::qWaitFor(windowInactive, 2000))
QSKIP("Dialog didn't activate");
// This should destroy the dialog and close the native window
child->deleteLater();
QTRY_VERIFY(!dialog);
// If the native window is still open, then the transient parent can't become
// active
window.activateWindow();
QVERIFY(QTest::qWaitFor(windowActive));
}
QTEST_MAIN(tst_QColorDialog)
#include "tst_qcolordialog.moc"

48
tests/auto/widgets/dialogs/qfiledialog/tst_qfiledialog.cpp
View File

@ -128,9 +128,10 @@ private slots:
void QTBUG49600_nativeIconProviderCrash();
void focusObjectDuringDestruction();
// NOTE: Please keep widgetlessNativeDialog() as the LAST test!
// NOTE: Please keep widgetlessNativeDialog() and
// hideNativeByDestruction() as the LAST tests!
//
// widgetlessNativeDialog() is the only test function that creates
// widgetlessNativeDialog() are the only test functions that create
// a native file dialog instance. GTK+ versions prior 3.15.5 have
// a nasty bug (https://bugzilla.gnome.org/show_bug.cgi?id=725164)
// in GtkFileChooserWidget, which makes it leak its folder change
@ -141,6 +142,7 @@ private slots:
// The crash has been fixed in GTK+ 3.15.5, but the RHEL 7.2 CI has
// GTK+ 3.14.13 installed (QTBUG-55276).
void widgetlessNativeDialog();
void hideNativeByDestruction();
private:
void cleanupSettingsFile();
@ -1437,7 +1439,7 @@ void tst_QFiledialog::widgetlessNativeDialog()
QSKIP("This platform always uses widgets to realize its QFileDialog, instead of the native file dialog.");
#ifdef Q_OS_ANDROID
// QTBUG-101194
QSKIP("Android: This keeeps the window open. Figure out why.");
QSKIP("Android: This keeps the window open. Figure out why.");
#endif
QApplication::setAttribute(Qt::AA_DontUseNativeDialogs, false);
QFileDialog fd;
@ -1451,6 +1453,46 @@ void tst_QFiledialog::widgetlessNativeDialog()
QApplication::setAttribute(Qt::AA_DontUseNativeDialogs, true);
}
void tst_QFiledialog::hideNativeByDestruction()
{
if (!QGuiApplicationPrivate::platformTheme()->usePlatformNativeDialog(QPlatformTheme::FileDialog))
QSKIP("This platform always uses widgets to realize its QFileDialog, instead of the native file dialog.");
#ifdef Q_OS_ANDROID
// QTBUG-101194
QSKIP("Android: This keeps the native window open. Figure out why.");
#endif
QApplication::setAttribute(Qt::AA_DontUseNativeDialogs, false);
auto resetAttribute = qScopeGuard([]{
QApplication::setAttribute(Qt::AA_DontUseNativeDialogs, true);
});
QWidget window;
QWidget *child = new QWidget(&window);
QPointer<QFileDialog> dialog = new QFileDialog(child);
// Make it application modal so that we don't end up with a sheet on macOS
dialog->setWindowModality(Qt::ApplicationModal);
window.show();
QVERIFY(QTest::qWaitForWindowActive(&window));
dialog->open();
// We test that the dialog opens and closes by watching the activation of the
// transient parent window. If it doesn't deactivate, then we have to skip.
const auto windowActive = [&window]{ return window.isActiveWindow(); };
const auto windowInactive = [&window]{ return !window.isActiveWindow(); };
if (!QTest::qWaitFor(windowInactive, 2000))
QSKIP("Dialog didn't activate");
// This should destroy the dialog and close the native window
child->deleteLater();
QTRY_VERIFY(!dialog);
// If the native window is still open, then the transient parent can't become
// active
window.activateWindow();
QVERIFY(QTest::qWaitFor(windowActive, 2000));
}
void tst_QFiledialog::selectedFilesWithoutWidgets()
{
// Test for a crash when widgets are not instantiated yet.

28
tests/auto/widgets/dialogs/qfontdialog/tst_qfontdialog.cpp
View File

@ -45,6 +45,7 @@ private slots:
void qtbug_41513_stylesheetStyle();
#endif
void hideNativeByDestruction();
private:
void runSlotWithFailsafeTimer(const char *member);
@ -238,5 +239,32 @@ void tst_QFontDialog::testNonStandardFontSize()
qWarning("Fail using a non-standard font size.");
}
void tst_QFontDialog::hideNativeByDestruction()
{
QWidget window;
QWidget *child = new QWidget(&window);
QPointer<QFontDialog> dialog = new QFontDialog(child);
// Make it application modal so that we don't end up with a sheet on macOS
dialog->setWindowModality(Qt::ApplicationModal);
window.show();
QVERIFY(QTest::qWaitForWindowActive(&window));
dialog->open();
// We test that the dialog opens and closes by watching the activation of the
// transient parent window. If it doesn't deactivate, then we have to skip.
const auto windowActive = [&window]{ return window.isActiveWindow(); };
const auto windowInactive = [&window]{ return !window.isActiveWindow(); };
if (!QTest::qWaitFor(windowInactive, 2000))
QSKIP("Dialog didn't activate");
// This should destroy the dialog and close the native window
child->deleteLater();
QTRY_VERIFY(!dialog);
// If the native window is still open, then the transient parent can't become
// active
window.activateWindow();
QVERIFY(QTest::qWaitFor(windowActive));
}
QTEST_MAIN(tst_QFontDialog)
#include "tst_qfontdialog.moc"

29
tests/auto/widgets/dialogs/qmessagebox/tst_qmessagebox.cpp
View File

@ -61,6 +61,8 @@ private slots:
void overrideDone_data();
void overrideDone();
void hideNativeByDestruction();
void cleanup();
};
@ -799,5 +801,32 @@ void tst_QMessageBox::acceptedRejectedSignals_data()
addCustomButtonsData();
}
void tst_QMessageBox::hideNativeByDestruction()
{
QWidget window;
QWidget *child = new QWidget(&window);
QPointer<QMessageBox> dialog = new QMessageBox(child);
// Make it application modal so that we don't end up with a sheet on macOS
dialog->setWindowModality(Qt::ApplicationModal);
window.show();
QVERIFY(QTest::qWaitForWindowActive(&window));
dialog->open();
// We test that the dialog opens and closes by watching the activation of the
// transient parent window. If it doesn't deactivate, then we have to skip.
const auto windowActive = [&window]{ return window.isActiveWindow(); };
const auto windowInactive = [&window]{ return !window.isActiveWindow(); };
if (!QTest::qWaitFor(windowInactive, 2000))
QSKIP("Dialog didn't activate");
// This should destroy the dialog and close the native window
child->deleteLater();
QTRY_VERIFY(!dialog);
// If the native window is still open, then the transient parent can't become
// active
window.activateWindow();
QVERIFY(QTest::qWaitFor(windowActive));
}
QTEST_MAIN(tst_QMessageBox)
#include "tst_qmessagebox.moc"