From df7944e7d7dd8b2bbccbd639eff0ab09745d6cc3 Mon Sep 17 00:00:00 2001 From: Gabriel de Dietrich Date: Mon, 26 Aug 2013 19:29:33 +0200 Subject: [PATCH] Cocoa: Fix QFontDialog, QColorDialog auto-tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new Cocoa event dispatcher made apparent some deficiencies in the way the dialog helpers were being hidden. In particular, we would not stop a dialog helper's modal loop when closing the dialog, resulting in the auto-tests hanging. Also, since the QApplication event loop is runnig with [NSApp run] in the stack, the previous workarounds are no longer needed. Task-number: QTBUG-24321 Change-Id: Ifba713c286638d78a699c319a15683d09714f06f Reviewed-by: Richard Moe Gustavsen Reviewed-by: Morten Johan Sørvig --- .../platforms/cocoa/qcocoacolordialoghelper.h | 2 ++ .../cocoa/qcocoacolordialoghelper.mm | 20 +++++++++++++++++++ .../platforms/cocoa/qcocoafontdialoghelper.mm | 4 ++++ src/widgets/dialogs/qcolordialog.cpp | 2 ++ src/widgets/dialogs/qfontdialog.cpp | 11 +++------- .../dialogs/qcolordialog/tst_qcolordialog.cpp | 11 ++++------ .../dialogs/qfontdialog/tst_qfontdialog.cpp | 17 +++++++--------- 7 files changed, 42 insertions(+), 25 deletions(-) diff --git a/src/plugins/platforms/cocoa/qcocoacolordialoghelper.h b/src/plugins/platforms/cocoa/qcocoacolordialoghelper.h index 59e029769d5..2b34f909d13 100644 --- a/src/plugins/platforms/cocoa/qcocoacolordialoghelper.h +++ b/src/plugins/platforms/cocoa/qcocoacolordialoghelper.h @@ -59,6 +59,8 @@ public: void setCurrentColor(const QColor&); QColor currentColor() const; + + bool event(QEvent *); }; QT_END_NAMESPACE diff --git a/src/plugins/platforms/cocoa/qcocoacolordialoghelper.mm b/src/plugins/platforms/cocoa/qcocoacolordialoghelper.mm index d90d77ec1d2..ef2b4cbcfbf 100644 --- a/src/plugins/platforms/cocoa/qcocoacolordialoghelper.mm +++ b/src/plugins/platforms/cocoa/qcocoacolordialoghelper.mm @@ -161,6 +161,10 @@ QT_NAMESPACE_ALIAS_OBJC_CLASS(QNSColorPanelDelegate); - (void)closePanel { + if (mDialogIsExecuting) { + mDialogIsExecuting = false; + [NSApp stopModal]; + } [mColorPanel close]; } @@ -488,6 +492,22 @@ QColor QCocoaColorDialogHelper::currentColor() const return sharedColorPanel()->currentColor(); } +bool QCocoaColorDialogHelper::event(QEvent *e) +{ + if (e->type() == QEvent::KeyPress) { + QKeyEvent *ke = static_cast(e); + if (ke->key() == Qt::Key_Enter || ke->key() == Qt::Key_Return) { + emit accept(); + return true; + } else if (ke->key() == Qt::Key_Escape) { + emit reject(); + return true; + } + } + + return false; +} + QT_END_NAMESPACE #endif // QT_NO_COLORDIALOG diff --git a/src/plugins/platforms/cocoa/qcocoafontdialoghelper.mm b/src/plugins/platforms/cocoa/qcocoafontdialoghelper.mm index 91fb52eb6d0..5bd0ad2a43e 100644 --- a/src/plugins/platforms/cocoa/qcocoafontdialoghelper.mm +++ b/src/plugins/platforms/cocoa/qcocoafontdialoghelper.mm @@ -203,6 +203,10 @@ QT_NAMESPACE_ALIAS_OBJC_CLASS(QNSFontPanelDelegate); - (void)closePanel { + if (mDialogIsExecuting) { + mDialogIsExecuting = false; + [NSApp stopModal]; + } [mFontPanel close]; } diff --git a/src/widgets/dialogs/qcolordialog.cpp b/src/widgets/dialogs/qcolordialog.cpp index 5da41aa0a54..c9f03c5a971 100644 --- a/src/widgets/dialogs/qcolordialog.cpp +++ b/src/widgets/dialogs/qcolordialog.cpp @@ -2166,6 +2166,8 @@ void QColorDialog::keyPressEvent(QKeyEvent *e) } e->accept(); return; + } else if (d->nativeDialogInUse && d->platformColorDialogHelper()->event(e)) { + return; } QDialog::keyPressEvent(e); } diff --git a/src/widgets/dialogs/qfontdialog.cpp b/src/widgets/dialogs/qfontdialog.cpp index b989ea7c86a..28afd91a09b 100644 --- a/src/widgets/dialogs/qfontdialog.cpp +++ b/src/widgets/dialogs/qfontdialog.cpp @@ -983,14 +983,9 @@ void QFontDialog::setVisible(bool visible) Q_D(QFontDialog); if (d->canBeNativeDialog()) d->setNativeDialogVisible(visible); - if (d->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); - } else { - d->nativeDialogInUse = false; - setAttribute(Qt::WA_DontShowOnScreen, false); - } + // Set WA_DontShowOnScreen so that QDialog::setVisible(visible) below + // updates the state correctly, but skips showing the non-native version: + setAttribute(Qt::WA_DontShowOnScreen, d->nativeDialogInUse); QDialog::setVisible(visible); } diff --git a/tests/auto/widgets/dialogs/qcolordialog/tst_qcolordialog.cpp b/tests/auto/widgets/dialogs/qcolordialog/tst_qcolordialog.cpp index d83e4729efc..06491f96abb 100644 --- a/tests/auto/widgets/dialogs/qcolordialog/tst_qcolordialog.cpp +++ b/tests/auto/widgets/dialogs/qcolordialog/tst_qcolordialog.cpp @@ -144,13 +144,10 @@ void tst_QColorDialog::postKeyReturn() { void tst_QColorDialog::testGetRgba() { -#ifdef Q_OS_MAC - QEXPECT_FAIL("", "Sending QTest::keyClick to OSX color dialog helper fails, see QTBUG-24320", Continue); -#endif - bool ok = false; - QTimer::singleShot(500, this, SLOT(postKeyReturn())); - QColorDialog::getRgba(0xffffffff, &ok); - QVERIFY(ok); + QColorDialog cd; + cd.show(); + QTimer::singleShot(0, this, SLOT(postKeyReturn())); + QTRY_COMPARE(cd.result(), int(QDialog::Accepted)); } void tst_QColorDialog::defaultOkButton() diff --git a/tests/auto/widgets/dialogs/qfontdialog/tst_qfontdialog.cpp b/tests/auto/widgets/dialogs/qfontdialog/tst_qfontdialog.cpp index 1762e40a9d1..ae70adf94b8 100644 --- a/tests/auto/widgets/dialogs/qfontdialog/tst_qfontdialog.cpp +++ b/tests/auto/widgets/dialogs/qfontdialog/tst_qfontdialog.cpp @@ -64,7 +64,7 @@ public: public slots: void postKeyReturn(); - void testGetFont(); + void testDefaultOkButton(); void testSetFont(); public slots: @@ -116,15 +116,12 @@ void tst_QFontDialog::postKeyReturn() { } } -void tst_QFontDialog::testGetFont() +void tst_QFontDialog::testDefaultOkButton() { -#ifdef Q_OS_MAC - QEXPECT_FAIL("", "Sending QTest::keyClick to OSX font dialog helper fails, see QTBUG-24321", Continue); -#endif - bool ok = false; - QTimer::singleShot(2000, this, SLOT(postKeyReturn())); - QFontDialog::getFont(&ok); - QVERIFY(ok); + QFontDialog fd; + fd.show(); + QTimer::singleShot(0, this, SLOT(postKeyReturn())); + QTRY_COMPARE(fd.result(), int(QDialog::Accepted)); } void tst_QFontDialog::runSlotWithFailsafeTimer(const char *member) @@ -144,7 +141,7 @@ void tst_QFontDialog::runSlotWithFailsafeTimer(const char *member) void tst_QFontDialog::defaultOkButton() { - runSlotWithFailsafeTimer(SLOT(testGetFont())); + runSlotWithFailsafeTimer(SLOT(testDefaultOkButton())); } void tst_QFontDialog::testSetFont()