-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125208/
-----------------------------------------------------------
(Updated Sept. 30, 2015, 6:04 p.m.)
Status
------
This change has been marked as submitted.
Review request for KDE Frameworks, David Faure and Jean-Baptiste Mardelle.
Changes
-------
Submitted with commit 6ae74671ca68e9444db618207d662ad5d1c16f31 by Weng Xuetian
to branch master.
Bugs: 350758
https://bugs.kde.org/show_bug.cgi?id=350758
Repository: frameworkintegration
Description
-------
1. The directly reason of 350758 is
https://bugreports.qt.io/browse/QTBUG-48248, which seems to be a Qt problem.
See the code example. One possible to avoid this is to remove the
m_dialog->hide().
2. Why hide() is needed in the first place?
If one runs dialog.show() and dialog.exec(), it should works. So there is
something happened in between the calls to 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 check, and QDialog will actually show an invisible
qdialog. I assume https://git.reviewboard.kde.org/r/123335/ tries to solve this
issue by "steal" the focus from that invisible dialog by hide our dialog and
show it again.
It makes some sense for Qt with "real" native dialog which is not controlled by
Qt, because this invisble dialog helps Qt to aware that there's a modal dialog,
but doesn't work for our case since our dialog is also a QDialog.
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 anytime soon. So here I used a
trick with QTimer to delegate the call to show() to next event.
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 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
