[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [frameworkintegration] /: Fix random file dialog not showing up problem.
From:       Weng Xuetian <wengxt () gmail ! com>
Date:       2015-09-30 18:04:14
Message-ID: E1ZhLji-0001aQ-Td () scm ! kde ! org
[Download RAW message or body]

Git commit 6ae74671ca68e9444db618207d662ad5d1c16f31 by Weng Xuetian.
Committed on 30/09/2015 at 18:03.
Pushed by xuetianweng into branch 'master'.

Fix random file dialog not showing up problem.

When using QDialog to implement QPlatformDialog there will be a issue,
that QDialog will use a dummy invisible to make Qt aware that a modal
dialog exists. But for our usecase, this invisible dialog will show up
after our own qdialog and cause our dialog not accept any input.

The original workaround is to hide our dialog and show it again, but
this will hit QTBUG-48248 and our dialog will not show up at all
randomly. To avoid this, we delegate the call to show() on our
QDialog with a timer.

BUG: 350758
REVIEW: 125208

M  +10   -0    autotests/kfiledialog_unittest.cpp
M  +4    -0    autotests/kfiledialogqml_unittest.cpp
M  +15   -1    src/platformtheme/kdeplatformfiledialogbase.cpp
M  +5    -0    src/platformtheme/kdeplatformfiledialogbase_p.h
M  +23   -12   src/platformtheme/kdeplatformfiledialoghelper.cpp
M  +1    -0    src/platformtheme/kdeplatformfiledialoghelper.h

http://commits.kde.org/frameworkintegration/6ae74671ca68e9444db618207d662ad5d1c16f31

diff --git a/autotests/kfiledialog_unittest.cpp b/autotests/kfiledialog_unittest.cpp
index 0d4c816..6abd98a 100644
--- a/autotests/kfiledialog_unittest.cpp
+++ b/autotests/kfiledialog_unittest.cpp
@@ -92,6 +92,8 @@ private Q_SLOTS:
 
             KFileWidget *fw = findFileWidget();
             QVERIFY(fw);
+            // real show() is delayed to next event.
+            QTest::qWaitForWindowExposed(fw->window());
             KDirOperator *op = fw->dirOperator();
             QCOMPARE(fileViewToString(op->viewMode()), \
fileViewToString(KFile::Tree));  fw->setViewMode(KFile::Simple);
@@ -120,6 +122,8 @@ private Q_SLOTS:
 
             KFileWidget *fw = findFileWidget();
             QVERIFY(fw);
+            // real show() is delayed to next event.
+            QTest::qWaitForWindowExposed(fw->window());
             QCOMPARE(fw->isVisible(), true);
             fw->slotCancel();
         }
@@ -133,6 +137,8 @@ private Q_SLOTS:
 
             KFileWidget *fw = findFileWidget();
             QVERIFY(fw);
+            // real show() is delayed to next event.
+            QTest::qWaitForWindowExposed(fw->window());
             QCOMPARE(fw->isVisible(), true);
             fw->slotCancel();
         }
@@ -147,6 +153,8 @@ private Q_SLOTS:
 
             KFileWidget *fw = findFileWidget();
             QVERIFY(fw);
+            // real show() is delayed to next event.
+            QTest::qWaitForWindowExposed(fw->window());
             QCOMPARE(fw->isVisible(), true);
             fw->slotCancel();
         }
@@ -160,6 +168,8 @@ private Q_SLOTS:
 
             KFileWidget *fw = findFileWidget();
             QVERIFY(fw);
+            // real show() is delayed to next event.
+            QTest::qWaitForWindowExposed(fw->window());
             QCOMPARE(fw->isVisible(), true);
             fw->slotCancel();
         }
diff --git a/autotests/kfiledialogqml_unittest.cpp \
b/autotests/kfiledialogqml_unittest.cpp index f805ef2..eb7dfc5 100644
--- a/autotests/kfiledialogqml_unittest.cpp
+++ b/autotests/kfiledialogqml_unittest.cpp
@@ -48,6 +48,8 @@ private Q_SLOTS:
 
             fw = findFileWidget();
             QVERIFY(fw);
+            // real show() is delayed to next event.
+            QTest::qWaitForWindowExposed(fw->window());
             QCOMPARE(fw->isVisible(), true);
             fw->slotCancel();
         }
@@ -65,6 +67,8 @@ private Q_SLOTS:
 
             fw = findFileWidget();
             QVERIFY(fw);
+            // real show() is delayed to next event.
+            QTest::qWaitForWindowExposed(fw->window());
             QCOMPARE(fw->isVisible(), true);
             fw->slotCancel();
         }
diff --git a/src/platformtheme/kdeplatformfiledialogbase.cpp \
b/src/platformtheme/kdeplatformfiledialogbase.cpp index 8e696bd..856c052 100644
--- a/src/platformtheme/kdeplatformfiledialogbase.cpp
+++ b/src/platformtheme/kdeplatformfiledialogbase.cpp
@@ -22,6 +22,20 @@
 
 KDEPlatformFileDialogBase::KDEPlatformFileDialogBase()
 {
+    m_timer.setInterval(0);
+    m_timer.setSingleShot(true);
+    connect(&m_timer, &QTimer::timeout, this, &KDEPlatformFileDialogBase::show);
+}
+
+void KDEPlatformFileDialogBase::delayedShow()
+{
+    m_timer.start();
+}
+
+void KDEPlatformFileDialogBase::discardDelayedShow()
+{
+    // this is used when hide() is called before timer triggers.
+    m_timer.stop();
 }
 
 void KDEPlatformFileDialogBase::closeEvent(QCloseEvent *e)
@@ -30,4 +44,4 @@ void KDEPlatformFileDialogBase::closeEvent(QCloseEvent *e)
     QDialog::closeEvent(e);
 }
 
-#include "moc_kdeplatformfiledialogbase_p.cpp"
\ No newline at end of file
+#include "moc_kdeplatformfiledialogbase_p.cpp"
diff --git a/src/platformtheme/kdeplatformfiledialogbase_p.h \
b/src/platformtheme/kdeplatformfiledialogbase_p.h index 5936dfb..3191027 100644
--- a/src/platformtheme/kdeplatformfiledialogbase_p.h
+++ b/src/platformtheme/kdeplatformfiledialogbase_p.h
@@ -23,6 +23,7 @@
 
 #include <QDialog>
 #include <QUrl>
+#include <QTimer>
 
 class KFileWidget;
 class QDialogButtonBox;
@@ -40,6 +41,9 @@ public:
     virtual QString selectedNameFilter() = 0;
     virtual QList<QUrl> selectedFiles() = 0;
 
+    void delayedShow();
+    void discardDelayedShow();
+
 Q_SIGNALS:
     void closed();
     void fileSelected(const QUrl &file);
@@ -51,6 +55,7 @@ Q_SIGNALS:
 protected:
     void closeEvent(QCloseEvent *e) Q_DECL_OVERRIDE;
     QDialogButtonBox *m_buttons;
+    QTimer m_timer;
 };
 
 #endif
diff --git a/src/platformtheme/kdeplatformfiledialoghelper.cpp \
b/src/platformtheme/kdeplatformfiledialoghelper.cpp index 94f2059..11e7efb 100644
--- a/src/platformtheme/kdeplatformfiledialoghelper.cpp
+++ b/src/platformtheme/kdeplatformfiledialoghelper.cpp
@@ -37,7 +37,6 @@
 #include <QWindow>
 
 #include <QTextStream>
-#include <QEventLoop>
 
 namespace
 {
@@ -281,19 +280,17 @@ void KDEPlatformFileDialogHelper::initializeDialog()
 
 void KDEPlatformFileDialogHelper::exec()
 {
-    m_dialog->hide(); // ensure dialog is not shown (exec would block input)
-    m_dialog->winId(); // ensure there's a window created
-    KSharedConfig::Ptr conf = KSharedConfig::openConfig();
-    KWindowConfig::restoreWindowSize(m_dialog->windowHandle(), \
                conf->group("FileDialogSize"));
-    // NOTICE: QWindow::setGeometry() does NOT impact the backing QWidget geometry \
                even if the platform
-    // window was created -> QTBUG-40584. We therefore copy the size here.
-    // TODO: remove once this was resolved in QWidget QPA
-    m_dialog->resize(m_dialog->windowHandle()->size());
+    restoreSize();
+    // KDEPlatformFileDialog::show() will always be called during \
QFileDialog::exec() +    // discard the delayed show() it and use exec() and it will \
call show() for us. +    // We can't hide and show it here because of \
https://bugreports.qt.io/browse/QTBUG-48248 +    m_dialog->discardDelayedShow();
     m_dialog->exec();
 }
 
 void KDEPlatformFileDialogHelper::hide()
 {
+    m_dialog->discardDelayedShow();
     m_dialog->hide();
 }
 
@@ -304,15 +301,29 @@ void KDEPlatformFileDialogHelper::saveSize()
     KWindowConfig::saveWindowSize(m_dialog->windowHandle(), group);
 }
 
+void KDEPlatformFileDialogHelper::restoreSize()
+{
+    m_dialog->winId(); // ensure there's a window created
+    KSharedConfig::Ptr conf = KSharedConfig::openConfig();
+    KWindowConfig::restoreWindowSize(m_dialog->windowHandle(), \
conf->group("FileDialogSize")); +    // NOTICE: QWindow::setGeometry() does NOT \
impact the backing QWidget geometry even if the platform +    // window was created \
-> QTBUG-40584. We therefore copy the size here. +    // TODO: remove once this was \
resolved in QWidget QPA +    m_dialog->resize(m_dialog->windowHandle()->size());
+}
+
 bool KDEPlatformFileDialogHelper::show(Qt::WindowFlags windowFlags, \
Qt::WindowModality windowModality, QWindow *parent)  {
     Q_UNUSED(parent)
     initializeDialog();
     m_dialog->setWindowFlags(windowFlags);
     m_dialog->setWindowModality(windowModality);
-    m_dialog->show();
-    KSharedConfig::Ptr conf = KSharedConfig::openConfig();
-    KWindowConfig::restoreWindowSize(m_dialog->windowHandle(), \
conf->group("FileDialogSize")); +    restoreSize();
+    // Use a delayed show here to delay show() after the internal Qt invisible \
QDialog. +    // The delayed call shouldn't matter, because for other "real" native \
QPlatformDialog +    // implementation like Mac and Windows, the native dialog is not \
necessarily +    // show up immediately.
+    m_dialog->delayedShow();
     return true;
 }
 
diff --git a/src/platformtheme/kdeplatformfiledialoghelper.h \
b/src/platformtheme/kdeplatformfiledialoghelper.h index dfbbed1..f80b8c7 100644
--- a/src/platformtheme/kdeplatformfiledialoghelper.h
+++ b/src/platformtheme/kdeplatformfiledialoghelper.h
@@ -74,6 +74,7 @@ private Q_SLOTS:
     void saveSize();
 
 private:
+    void restoreSize();
     KDEPlatformFileDialogBase *m_dialog;
 };
 


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic