----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125208/ -----------------------------------------------------------
Review request for KDE Frameworks, David Faure and Jean-Baptiste Mardelle. Repository: frameworkintegration Description ------- 1. The reason of 350758: https://bugreports.qt.io/browse/QTBUG-48248 , I just tried to reproduce it with pure Qt code. If a dialog called against show, hide, and exec one immediately after another, the dialog may not show up. 2. Why hide() is needed in the first place? If one runs dialog.show() and dialog.exec(), it usually works. So there is something happened in between KDEPlatformFileDialogHelper::show() and KDEPlatformFileDialogHelper::exec(). The real reason is the platform dialog helper doesn't expect that user may use "Qt" dialog to implement it. https://github.com/qtproject/qtbase/blob/5bfac9d653357c906946563b9494d7ae69cdad92/src/widgets/dialogs/qfiledialog.cpp#L845 here it sets Qt::WA_DontShowOnScreen on the actual dialog. https://github.com/qtproject/qtbase/blob/5bfac9d653357c906946563b9494d7ae69cdad92/src/widgets/dialogs/qfiledialog.cpp#L868 and call QDialog::setVisible() here https://github.com/qtproject/qtbase/blob/17d6b2d1f02e5f679008d97036befd713025a0f2/src/widgets/dialogs/qdialog.cpp#L713 Thus it will by pass the first check, and QDialog will actually show an invisible qdialog with it. I assume https://git.reviewboard.kde.org/r/123335/ tries to solve this issue by "steal" the focus from that invisible dialog by hide it first. It makes some sense for Qt with "real" native dialog, because this invisble dialog helps Qt to aware that there's a modal dialog, but doesn't work for our case. 3. To avoid calling hide() in exec() I'd rather to see that KDEPlatformFileDialogHelper::show() is not called in QDialog::exec(), but that's not gonna happen. So here I used a trick with QTimer to delegate the show to next event when calling to KDEPlatformFileDialogHelper::show(). I uses QTimer instead of QMetaObject::invoke here, because we can discard the "m_dialog->show()" if hide() or exec() is called immediately. (Actually we don't need to discard the one in exec(), but we can save a call to m_dialog->show() if we discard it.) Before this change, the calling sequence on m_dialog is m_dialog->show(); The dummy dialog setVisible(true); m_dialog->hide(); m_dialog->exec(); After this change, it becomes The dummy dialog setVisible(true); m_dialog->exec(); 4. Required changes to autotest QTest::qWaitForWindowExposed(fw->window()) is added to some necessary places. Diffs ----- autotests/kfiledialog_unittest.cpp 0d4c816 autotests/kfiledialogqml_unittest.cpp f805ef2 src/platformtheme/kdeplatformfiledialogbase.cpp 8e696bd src/platformtheme/kdeplatformfiledialogbase_p.h 5936dfb src/platformtheme/kdeplatformfiledialoghelper.h dfbbed1 src/platformtheme/kdeplatformfiledialoghelper.cpp 94f2059 Diff: https://git.reviewboard.kde.org/r/125208/diff/ Testing ------- not able to reproduce the bug. Thanks, Xuetian Weng
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
