Git commit f3373660e48ffd38c123afddaabe6b7a7e733aaa by Jan Kundrát.
Committed on 21/09/2016 at 00:09.
Pushed by gerrit into branch 'master'.
GUI: Reduce indirect QDialog::exec calls
QDialog::exec is dangerous because it re-enters the event loop. We've
got a report of a crash that can be traced back to
MainWindow::cacheError which led to further event processing up to
Gui::MainWindow::slotPluginsChanged which tried to manipulate QActins
which weren't really instantiated at that time.
Reported on Haiku (!) by Zoltán Mizsei ("miqlas" on IRC).
Change-Id: Ibd24040c67341961b69cd058253fceadf5a6ea2a
M +3 -2 src/Gui/ComposeWidget.cpp
M +3 -3 src/Gui/SettingsDialog.cpp
M +5 -2 src/Gui/TagListWidget.cpp
M +23 -0 src/Gui/Util.cpp
M +3 -0 src/Gui/Util.h
M +21 -21 src/Gui/Window.cpp
http://commits.kde.org/trojita/f3373660e48ffd38c123afddaabe6b7a7e733aaa
diff --git a/src/Gui/ComposeWidget.cpp b/src/Gui/ComposeWidget.cpp
index b84449d..153c2a0 100644
--- a/src/Gui/ComposeWidget.cpp
+++ b/src/Gui/ComposeWidget.cpp
@@ -439,8 +439,9 @@ ComposeWidget *ComposeWidget::createReply(MainWindow
*mainWindow, const Composer
"You might want to use the \"Reply All\" function and
trim the list of addresses manually.");
break;
}
- if (!err.isEmpty())
- QMessageBox::warning(w, tr("Cannot Determine Recipients"), err);
+ if (!err.isEmpty()) {
+ Gui::Util::messageBoxWarning(w, tr("Cannot Determine Recipients"),
err);
+ }
}
w->placeOnMainWindow();
w->show();
diff --git a/src/Gui/SettingsDialog.cpp b/src/Gui/SettingsDialog.cpp
index 5e6ccf0..ae5e913 100644
--- a/src/Gui/SettingsDialog.cpp
+++ b/src/Gui/SettingsDialog.cpp
@@ -211,9 +211,9 @@ void SettingsDialog::slotAccept()
}
}
if (!passwordFailures.isEmpty()) {
- QMessageBox::warning(this, tr("Saving passwords failed"),
- tr("<p>Couldn't save passwords. These were the
error messages:</p>\n<p>%1</p>")
-
.arg(passwordFailures.join(QStringLiteral("<br/>"))));
+ Gui::Util::messageBoxWarning(this, tr("Saving passwords failed"),
+ tr("<p>Couldn't save passwords. These
were the error messages:</p>\n<p>%1</p>")
+
.arg(passwordFailures.join(QStringLiteral("<br/>"))));
}
buttons->setEnabled(true);
diff --git a/src/Gui/TagListWidget.cpp b/src/Gui/TagListWidget.cpp
index fa2df43..ed71cbb 100644
--- a/src/Gui/TagListWidget.cpp
+++ b/src/Gui/TagListWidget.cpp
@@ -32,6 +32,7 @@
#include "TagListWidget.h"
#include "FlowLayout.h"
#include "TagWidget.h"
+#include "Gui/Util.h"
#include "Imap/Model/SpecialFlagNames.h"
namespace Gui
@@ -103,8 +104,10 @@ void TagListWidget::newTagsRequested()
}
}
if (!reservedTagsList.isEmpty()) {
- QMessageBox::warning(this, tr("Disallowed tag value"),
- tr("No tags were set because the following given
tag(s) are reserved and have been disallowed from being set in this way:
%1.").arg(reservedTagsList.join(QStringLiteral(", "))));
+ Gui::Util::messageBoxWarning(this, tr("Disallowed tag value"),
+ tr("No tags were set because the
following given tag(s) are reserved "
+ "and have been disallowed from being
set in this way: %1.")
+
.arg(reservedTagsList.join(QStringLiteral(", "))));
return;
}
diff --git a/src/Gui/Util.cpp b/src/Gui/Util.cpp
index 2cf7c95..b9b0620 100644
--- a/src/Gui/Util.cpp
+++ b/src/Gui/Util.cpp
@@ -36,6 +36,17 @@
#include "Util.h"
#include "Window.h"
+namespace {
+
+void messageBoxImpl(QWidget *parent, const QString &title, const QString
&message, const QMessageBox::Icon icon)
+{
+ Q_ASSERT(parent);
+ auto box = new QMessageBox(icon, title, message, QMessageBox::Ok, parent);
+ box->open();
+}
+
+}
+
namespace Gui {
namespace Util {
@@ -94,6 +105,18 @@ QString resizedImageAsDataUrl(const QString &fileName,
const int extent)
return QLatin1String("data:image/png;base64,") +
QString::fromUtf8(bdata.toBase64());
}
+/** @short QMessageBox::critical, but without reentering the event loop */
+void messageBoxCritical(QWidget *parent, const QString &title, const QString
&message)
+{
+ messageBoxImpl(parent, title, message, QMessageBox::Critical);
+}
+
+/** @short QMessageBox::warning, but without reentering the event loop */
+void messageBoxWarning(QWidget *parent, const QString &title, const QString
&message)
+{
+ messageBoxImpl(parent, title, message, QMessageBox::Warning);
+}
+
} // namespace Util
} // namespace Gui
diff --git a/src/Gui/Util.h b/src/Gui/Util.h
index 9713c3e..09883bc 100644
--- a/src/Gui/Util.h
+++ b/src/Gui/Util.h
@@ -46,6 +46,9 @@ int askForSomethingUnlessTold(const QString &title, const
QString &message, cons
QString resizedImageAsDataUrl(const QString &fileName, const int extent);
+void messageBoxCritical(QWidget *parent, const QString &title, const QString
&message);
+void messageBoxWarning(QWidget *parent, const QString &title, const QString
&message);
+
} // namespace Util
} // namespace Gui
diff --git a/src/Gui/Window.cpp b/src/Gui/Window.cpp
index 4ab6596..24715f5 100644
--- a/src/Gui/Window.cpp
+++ b/src/Gui/Window.cpp
@@ -117,11 +117,11 @@ MainWindow::MainWindow(QSettings *settings):
QMainWindow(), m_imapAccess(0), m_m
Common::SettingsNames::addressbookPlugin,
Common::SettingsNames::passwordPlugin);
connect(m_pluginManager, &Plugins::PluginManager::pluginsChanged, this,
&MainWindow::slotPluginsChanged);
connect(m_pluginManager, &Plugins::PluginManager::pluginError, this,
[this](const QString &errorMessage) {
- QMessageBox::warning(this, tr("Plugin Error"),
- //: The %1 placeholder is a full error message as
provided by Qt, ready for human consumption.
- trUtf8("A plugin failed to load, therefore some
functionality might be lost. "
- "You might want to update your system or
report a bug to your vendor."
- "\n\n%1").arg(errorMessage));
+ Gui::Util::messageBoxWarning(this, tr("Plugin Error"),
+ //: The %1 placeholder is a full error
message as provided by Qt, ready for human consumption.
+ trUtf8("A plugin failed to load,
therefore some functionality might be lost. "
+ "You might want to update your
system or report a bug to your vendor."
+ "\n\n%1").arg(errorMessage));
});
#ifdef TROJITA_HAVE_CRYPTO_MESSAGES
Plugins::PluginManager::MimePartReplacers replacers;
@@ -1199,12 +1199,12 @@ void MainWindow::slotResyncMbox()
void MainWindow::alertReceived(const QString &message)
{
//: "ALERT" is a special warning which we're required to show to the user
- QMessageBox::warning(this, tr("IMAP Alert"), message);
+ Gui::Util::messageBoxWarning(this, tr("IMAP Alert"), message);
}
void MainWindow::imapError(const QString &message)
{
- QMessageBox::critical(this, tr("IMAP Protocol Error"), message);
+ Gui::Util::messageBoxCritical(this, tr("IMAP Protocol Error"), message);
// Show the IMAP logger -- maybe some user will take that as a hint that
they shall include it in the bug report.
// </joke>
showImapLogger->setChecked(true);
@@ -1233,10 +1233,10 @@ void MainWindow::networkError(const QString &message)
void MainWindow::cacheError(const QString &message)
{
- QMessageBox::critical(this, tr("IMAP Cache Error"),
- tr("The caching subsystem managing a cache of the
data already "
- "downloaded from the IMAP server is having
troubles. "
- "All caching will be
disabled.\n\n%1").arg(message));
+ Gui::Util::messageBoxCritical(this, tr("IMAP Cache Error"),
+ tr("The caching subsystem managing a cache
of the data already "
+ "downloaded from the IMAP server is
having troubles. "
+ "All caching will be
disabled.\n\n%1").arg(message));
}
void MainWindow::networkPolicyOffline()
@@ -1291,8 +1291,8 @@ void MainWindow::slotShowSettings()
QString method =
m_settings->value(Common::SettingsNames::imapMethodKey).toString();
if (method != Common::SettingsNames::methodTCP && method !=
Common::SettingsNames::methodSSL &&
method != Common::SettingsNames::methodProcess ) {
- QMessageBox::critical(this, tr("No Configuration"),
- trUtf8("No IMAP account is configured. Trojitá
cannot do much without one."));
+ Gui::Util::messageBoxCritical(this, tr("No Configuration"),
+ trUtf8("No IMAP account is configured.
Trojitá cannot do much without one."));
}
applySizesAndState();
}
@@ -1808,20 +1808,20 @@ MSA::MSAFactory *MainWindow::msaFactory()
void MainWindow::slotMailboxDeleteFailed(const QString &mailbox, const QString
&msg)
{
- QMessageBox::warning(this, tr("Can't delete mailbox"),
- tr("Deleting mailbox \"%1\" failed with the following
message:\n%2").arg(mailbox, msg));
+ Gui::Util::messageBoxWarning(this, tr("Can't delete mailbox"),
+ tr("Deleting mailbox \"%1\" failed with the
following message:\n%2").arg(mailbox, msg));
}
void MainWindow::slotMailboxCreateFailed(const QString &mailbox, const QString
&msg)
{
- QMessageBox::warning(this, tr("Can't create mailbox"),
- tr("Creating mailbox \"%1\" failed with the following
message:\n%2").arg(mailbox, msg));
+ Gui::Util::messageBoxWarning(this, tr("Can't create mailbox"),
+ tr("Creating mailbox \"%1\" failed with the
following message:\n%2").arg(mailbox, msg));
}
void MainWindow::slotMailboxSyncFailed(const QString &mailbox, const QString
&msg)
{
- QMessageBox::warning(this, tr("Can't open mailbox"),
- tr("Opening mailbox \"%1\" failed with the following
message:\n%2").arg(mailbox, msg));
+ Gui::Util::messageBoxWarning(this, tr("Can't open mailbox"),
+ tr("Opening mailbox \"%1\" failed with the
following message:\n%2").arg(mailbox, msg));
}
void MainWindow::slotMailboxChanged(const QModelIndex &mailbox)
@@ -1969,8 +1969,8 @@ void MainWindow::slotSaveCurrentMessageBody()
void MainWindow::slotDownloadTransferError(const QString &errorString)
{
- QMessageBox::critical(this, tr("Can't save into file"),
- tr("Unable to save into file.
Error:\n%1").arg(errorString));
+ Gui::Util::messageBoxCritical(this, tr("Can't save into file"),
+ tr("Unable to save into file.
Error:\n%1").arg(errorString));
}
void MainWindow::slotDownloadMessageFileNameRequested(QString *fileName)
