Don't use exec() on dialogs
authorHannah von Reth <hannah.vonreth@owncloud.com>
Thu, 15 Oct 2020 13:12:16 +0000 (15:12 +0200)
committerKevin Ottens <kevin.ottens@nextcloud.com>
Tue, 15 Dec 2020 09:59:25 +0000 (10:59 +0100)
src/gui/accountsettings.cpp
src/gui/accountsettings.h
src/gui/wizard/owncloudadvancedsetuppage.cpp

index e524f9dda75e38b6acc9db8b9c668cba2ac55414..b3e37937e734367c6aeef63964023d5643b839ff 100644 (file)
@@ -659,8 +659,7 @@ void AccountSettings::slotFolderWizardRejected()
 
 void AccountSettings::slotRemoveCurrentFolder()
 {
-    FolderMan *folderMan = FolderMan::instance();
-    auto folder = folderMan->folder(selectedFolderAlias());
+    auto folder = FolderMan::instance()->folder(selectedFolderAlias());
     QModelIndex selected = _ui->_folderList->selectionModel()->currentIndex();
     if (selected.isValid() && folder) {
         int row = selected.row();
@@ -668,28 +667,27 @@ void AccountSettings::slotRemoveCurrentFolder()
         qCInfo(lcAccountSettings) << "Remove Folder alias " << folder->alias();
         QString shortGuiLocalPath = folder->shortGuiLocalPath();
 
-        QMessageBox messageBox(QMessageBox::Question,
+        auto messageBox = new QMessageBox(QMessageBox::Question,
             tr("Confirm Folder Sync Connection Removal"),
             tr("<p>Do you really want to stop syncing the folder <i>%1</i>?</p>"
                "<p><b>Note:</b> This will <b>not</b> delete any files.</p>")
                 .arg(shortGuiLocalPath),
             QMessageBox::NoButton,
             this);
+        messageBox->setAttribute(Qt::WA_DeleteOnClose);
         QPushButton *yesButton =
-            messageBox.addButton(tr("Remove Folder Sync Connection"), QMessageBox::YesRole);
-        messageBox.addButton(tr("Cancel"), QMessageBox::NoRole);
-
-        messageBox.exec();
-        if (messageBox.clickedButton() != yesButton) {
-            return;
-        }
-
-        folderMan->removeFolder(folder);
-        _model->removeRow(row);
-
-        // single folder fix to show add-button and hide remove-button
-
-        emit folderChanged();
+            messageBox->addButton(tr("Remove Folder Sync Connection"), QMessageBox::YesRole);
+        messageBox->addButton(tr("Cancel"), QMessageBox::NoRole);
+        connect(messageBox, &QMessageBox::finished, this, [messageBox, yesButton, folder, row, this]{
+            if (messageBox->clickedButton() == yesButton) {
+                FolderMan::instance()->removeFolder(folder);
+                _model->removeRow(row);
+
+                // single folder fix to show add-button and hide remove-button
+                emit folderChanged();
+            }
+        });
+        messageBox->open();
     }
 }
 
@@ -884,7 +882,7 @@ void AccountSettings::showConnectionLabel(const QString &message, QStringList er
     _ui->accountStatus->setVisible(!message.isEmpty());
 }
 
-void AccountSettings::slotEnableCurrentFolder()
+void AccountSettings::slotEnableCurrentFolder(bool terminate)
 {
     auto alias = selectedFolderAlias();
 
@@ -892,7 +890,6 @@ void AccountSettings::slotEnableCurrentFolder()
         FolderMan *folderMan = FolderMan::instance();
 
         qCInfo(lcAccountSettings) << "Application: enable folder with alias " << alias;
-        bool terminate = false;
         bool currentlyPaused = false;
 
         // this sets the folder status to disabled but does not interrupt it.
@@ -901,25 +898,19 @@ void AccountSettings::slotEnableCurrentFolder()
             return;
         }
         currentlyPaused = f->syncPaused();
-        if (!currentlyPaused) {
+        if (!currentlyPaused && !terminate) {
             // check if a sync is still running and if so, ask if we should terminate.
             if (f->isBusy()) { // its still running
-#if defined(Q_OS_MAC)
-                QWidget *parent = this;
-                Qt::WindowFlags flags = Qt::Sheet;
-#else
-                QWidget *parent = nullptr;
-                Qt::WindowFlags flags = Qt::Dialog | Qt::MSWindowsFixedSizeDialogHint; // default flags
-#endif
-                QMessageBox msgbox(QMessageBox::Question, tr("Sync Running"),
+                auto msgbox = new QMessageBox(QMessageBox::Question, tr("Sync Running"),
                     tr("The syncing operation is running.<br/>Do you want to terminate it?"),
-                    QMessageBox::Yes | QMessageBox::No, parent, flags);
-                msgbox.setDefaultButton(QMessageBox::Yes);
-                int reply = msgbox.exec();
-                if (reply == QMessageBox::Yes)
-                    terminate = true;
-                else
-                    return; // do nothing
+                    QMessageBox::Yes | QMessageBox::No, this);
+                msgbox->setAttribute(Qt::WA_DeleteOnClose);
+                msgbox->setDefaultButton(QMessageBox::Yes);
+                connect(msgbox, &QMessageBox::accepted, this, [this]{
+                    slotEnableCurrentFolder(true);
+                });
+                msgbox->open();
+                return;
             }
         }
 
@@ -1273,33 +1264,27 @@ void AccountSettings::slotDeleteAccount()
 {
     // Deleting the account potentially deletes 'this', so
     // the QMessageBox should be destroyed before that happens.
-    {
-        QMessageBox messageBox(QMessageBox::Question,
-            tr("Confirm Account Removal"),
-            tr("<p>Do you really want to remove the connection to the account <i>%1</i>?</p>"
-               "<p><b>Note:</b> This will <b>not</b> delete any files.</p>")
-                .arg(_accountState->account()->displayName()),
-            QMessageBox::NoButton,
-            this);
-        QPushButton *yesButton =
-            messageBox.addButton(tr("Remove connection"), QMessageBox::YesRole);
-        messageBox.addButton(tr("Cancel"), QMessageBox::NoRole);
-
-        messageBox.exec();
-        if (messageBox.clickedButton() != yesButton) {
-            return;
+    auto messageBox = new QMessageBox(QMessageBox::Question,
+        tr("Confirm Account Removal"),
+        tr("<p>Do you really want to remove the connection to the account <i>%1</i>?</p>"
+           "<p><b>Note:</b> This will <b>not</b> delete any files.</p>")
+            .arg(_accountState->account()->displayName()),
+        QMessageBox::NoButton,
+        this);
+    auto yesButton = messageBox->addButton(tr("Remove connection"), QMessageBox::YesRole);
+    messageBox->addButton(tr("Cancel"), QMessageBox::NoRole);
+    messageBox->setAttribute(Qt::WA_DeleteOnClose);
+    connect(messageBox, &QMessageBox::finished, this, [this, messageBox, yesButton]{
+        if (messageBox->clickedButton() == yesButton) {
+            // Else it might access during destruction. This should be better handled by it having a QSharedPointer
+            _model->setAccountState(nullptr);
+
+            auto manager = AccountManager::instance();
+            manager->deleteAccount(_accountState);
+            manager->save();
         }
-    }
-
-    // Else it might access during destruction. This should be better handled by it having a QSharedPointer
-    _model->setAccountState(nullptr);
-
-    auto manager = AccountManager::instance();
-    manager->deleteAccount(_accountState);
-    manager->save();
-
-    // IMPORTANT: "this" is deleted from this point on. We should probably remove this synchronous
-    // .exec() QMessageBox magic above as it recurses into the event loop.
+    });
+    messageBox->open();
 }
 
 bool AccountSettings::event(QEvent *e)
index ef37c44b75dc89c95e6ccaba836c1d7e76e54492..ccb986e74262f3173e194824bd83be538d1ef793 100644 (file)
@@ -77,7 +77,7 @@ public slots:
 
 protected slots:
     void slotAddFolder();
-    void slotEnableCurrentFolder();
+    void slotEnableCurrentFolder(bool terminate = false);
     void slotScheduleCurrentFolder();
     void slotScheduleCurrentFolderForceRemoteDiscovery();
     void slotForceSyncCurrentFolder();
index dba276ec85efe3db9b0d3b01f3d0b20af556a81b..6128f843b34fa7d87b8d7eda8928ed2a188b082e 100644 (file)
@@ -347,45 +347,41 @@ void OwncloudAdvancedSetupPage::slotSelectiveSyncClicked()
 {
     AccountPtr acc = static_cast<OwncloudWizard *>(wizard())->account();
     auto *dlg = new SelectiveSyncDialog(acc, _remoteFolder, _selectiveSyncBlacklist, this);
+    dlg->setAttribute(Qt::WA_DeleteOnClose);
+
+    connect(dlg, &SelectiveSyncDialog::finished, this, [this, dlg]{
+        const int result = dlg->result();
+        bool updateBlacklist = false;
+
+        // We need to update the selective sync blacklist either when the dialog
+        // was accepted in that
+        // case the stub blacklist of / was expanded to the actual list of top
+        // level folders by the selective sync dialog.
+        if (result == QDialog::Accepted) {
+            _selectiveSyncBlacklist = dlg->createBlackList();
+            updateBlacklist = true;
+        } else if (result == QDialog::Rejected && _selectiveSyncBlacklist == QStringList("/")) {
+            _selectiveSyncBlacklist = dlg->oldBlackList();
+            updateBlacklist = true;
+        }
 
-    const int result = dlg->exec();
-    bool updateBlacklist = false;
-
-    // We need to update the selective sync blacklist either when the dialog
-    // was accepted, or when it was used in conjunction with the
-    // wizardSelectiveSyncDefaultNothing feature and was cancelled - in that
-    // case the stub blacklist of / was expanded to the actual list of top
-    // level folders by the selective sync dialog.
-    if (result == QDialog::Accepted) {
-        _selectiveSyncBlacklist = dlg->createBlackList();
-        updateBlacklist = true;
-    } else if (result == QDialog::Rejected && _selectiveSyncBlacklist == QStringList("/")) {
-        _selectiveSyncBlacklist = dlg->oldBlackList();
-        updateBlacklist = true;
-    }
-
-    if (updateBlacklist) {
-        if (!_selectiveSyncBlacklist.isEmpty()) {
-            _ui.rSelectiveSync->blockSignals(true);
-            setRadioChecked(_ui.rSelectiveSync);
-            _ui.rSelectiveSync->blockSignals(false);
-            auto s = dlg->estimatedSize();
-            if (s > 0) {
-                _ui.lSelectiveSyncSizeLabel->setText(tr("(%1)").arg(Utility::octetsToString(s)));
-
-                _rSelectedSize = s;
-                QString errorStr = checkLocalSpace(_rSelectedSize);
-                setErrorString(errorStr);
-
+        if (updateBlacklist) {
+            if (!_selectiveSyncBlacklist.isEmpty()) {
+                auto s = dlg->estimatedSize();
+                if (s > 0) {
+                    _ui.lSelectiveSyncSizeLabel->setText(tr("(%1)").arg(Utility::octetsToString(s)));
+                } else {
+                    _ui.lSelectiveSyncSizeLabel->setText(QString());
+                }
             } else {
+                setRadioChecked(_ui.rSyncEverything);
                 _ui.lSelectiveSyncSizeLabel->setText(QString());
             }
-        } else {
-            setRadioChecked(_ui.rSyncEverything);
-            _ui.lSelectiveSyncSizeLabel->setText(QString());
+            wizard()->setProperty("blacklist", _selectiveSyncBlacklist);
         }
-        wizard()->setProperty("blacklist", _selectiveSyncBlacklist);
-    }
+
+    });
+    dlg->open();
 }
 
 void OwncloudAdvancedSetupPage::slotVirtualFileSyncClicked()