extcap: Allow starting from extcap config

Rework the changes from 428f2228533351baf6509b8fc50a075cdbc136b0
a little bit to restore the ability to start a capture from
the extcap options dialog.

When the the dialog is opened for configuration, present both the
Save and the Start button. Continue to only have Start when the
dialog was spawned because the user wanted to start a capture
but a mandatory parameter was not configured.

Use the default QDialogButtonBox "Discard/Close without Saving"
button when closing the dialog without saving the user input
for new preferences.

Fix #19199

4 files changed, 43 insertions, 41 deletions

diff --git a/ui/qt/extcap_options_dialog.cpp b/ui/qt/extcap_options_dialog.cpp
index ad8ec43d9d..78b67eb913 100644
--- a/ui/qt/extcap_options_dialog.cpp
+++ b/ui/qt/extcap_options_dialog.cpp
@@ -59,8 +59,7 @@ ExtcapOptionsDialog::ExtcapOptionsDialog(bool startCaptureOnClose, QWidget *pare
     ui(new Ui::ExtcapOptionsDialog),
     device_name(""),
     device_idx(0),
-    defaultValueIcon_(StockIcon("x-reset")),
-    start_capture_on_close_(startCaptureOnClose)
+    defaultValueIcon_(StockIcon("x-reset"))
 {
     ui->setupUi(this);
 
@@ -68,10 +67,11 @@ ExtcapOptionsDialog::ExtcapOptionsDialog(bool startCaptureOnClose, QWidget *pare
 
     ui->checkSaveOnStart->setCheckState(prefs.extcap_save_on_start ? Qt::Checked : Qt::Unchecked);
 
+    ui->buttonBox->button(QDialogButtonBox::Ok)->setText(tr("Start"));
     if (startCaptureOnClose) {
-        ui->buttonBox->button(QDialogButtonBox::Ok)->setText(tr("Start"));
-    } else {
-        ui->buttonBox->button(QDialogButtonBox::Ok)->setText(tr("Save"));
+        // This dialog was spawned because the user wanted to start a capture
+        // immediately but a mandatory parameter was not configured.
+        ui->buttonBox->button(QDialogButtonBox::Save)->hide();
     }
 }
 
@@ -118,26 +118,6 @@ ExtcapOptionsDialog::~ExtcapOptionsDialog()
     delete ui;
 }
 
-void ExtcapOptionsDialog::on_buttonBox_accepted()
-{
-    if (saveOptionToCaptureInfo()) {
-        /* Starting a new capture with those values */
-        prefs.extcap_save_on_start = ui->checkSaveOnStart->checkState() == Qt::Checked;
-
-        /* If the button says "Save" instead of "Start", unconditionally
-         * save. XXX - Why not have both buttons? (#19199)
-         *
-         * XXX - If extcap_save_on_start is the only preference that has
-         * changed, or if it changed from true to false, we should write
-         * out a new preference file with its new value, but don't.
-         */
-        if (prefs.extcap_save_on_start || !start_capture_on_close_)
-            storeValues();
-
-        accept();
-    }
-}
-
 void ExtcapOptionsDialog::anyValueChanged()
 {
     bool allowStart = true;
@@ -403,12 +383,6 @@ void ExtcapOptionsDialog::updateWidgets()
     }
 }
 
-// Not sure why we have to do this manually.
-void ExtcapOptionsDialog::on_buttonBox_rejected()
-{
-    reject();
-}
-
 void ExtcapOptionsDialog::on_buttonBox_helpRequested()
 {
     interface_t *device;
@@ -491,8 +465,41 @@ bool ExtcapOptionsDialog::saveOptionToCaptureInfo()
 void ExtcapOptionsDialog::on_buttonBox_clicked(QAbstractButton *button)
 {
     /* Only the save button has the ActionRole */
-    if (ui->buttonBox->buttonRole(button) == QDialogButtonBox::ResetRole)
+    switch (ui->buttonBox->buttonRole(button)) {
+    case QDialogButtonBox::ResetRole:
         resetValues();
+        break;
+    case QDialogButtonBox::RejectRole:
+    case QDialogButtonBox::DestructiveRole:
+        /* entries are only saved if saveOptionToCaptureInfo() is called,
+         * so do nothing. */
+        reject();
+        break;
+    case QDialogButtonBox::AcceptRole:
+        if (saveOptionToCaptureInfo()) {
+            /* Starting a new capture with those values */
+            prefs.extcap_save_on_start = ui->checkSaveOnStart->checkState() == Qt::Checked;
+
+            /* XXX - If extcap_save_on_start is the only preference that has
+             * changed, or if it changed from true to false, we should write
+             * out a new preference file with its new value, but don't.
+             */
+            if (ui->buttonBox->standardButton(button) == QDialogButtonBox::Save) {
+                storeValues();
+                /* Reject the dialog, because we don't want to start a capture. */
+                reject();
+            } else {
+                /* Start */
+                if (prefs.extcap_save_on_start) {
+                    storeValues();
+                }
+                accept();
+            }
+        }
+        break;
+    default:
+        break;
+    }
 }
 
 void ExtcapOptionsDialog::resetValues()
diff --git a/ui/qt/extcap_options_dialog.h b/ui/qt/extcap_options_dialog.h
index bb1d8e895a..a38808e84a 100644
--- a/ui/qt/extcap_options_dialog.h
+++ b/ui/qt/extcap_options_dialog.h
@@ -40,8 +40,6 @@ public:
     ExtcapValueList loadValuesFor(int argNum, QString call, QString parent = "");
 
 private Q_SLOTS:
-    void on_buttonBox_accepted();
-    void on_buttonBox_rejected();
     void on_buttonBox_clicked(QAbstractButton *button);
     void on_buttonBox_helpRequested();
     void updateWidgets();
@@ -54,7 +52,6 @@ private:
     QString device_name;
     guint device_idx;
     QIcon defaultValueIcon_;
-    bool start_capture_on_close_;
 
     ExtcapArgumentList extcapArguments;
 
diff --git a/ui/qt/extcap_options_dialog.ui b/ui/qt/extcap_options_dialog.ui
index 337a0c9df9..1c6f0d7baf 100644
--- a/ui/qt/extcap_options_dialog.ui
+++ b/ui/qt/extcap_options_dialog.ui
@@ -40,7 +40,7 @@
    <item>
     <widget class="QDialogButtonBox" name="buttonBox">
      <property name="standardButtons">
-      <set>QDialogButtonBox::Close|QDialogButtonBox::Help|QDialogButtonBox::Ok|QDialogButtonBox::RestoreDefaults</set>
+      <set>QDialogButtonBox::Discard|QDialogButtonBox::Help|QDialogButtonBox::Ok|QDialogButtonBox::Save|QDialogButtonBox::RestoreDefaults</set>
      </property>
     </widget>
    </item>
diff --git a/ui/qt/wireshark_main_window_slots.cpp b/ui/qt/wireshark_main_window_slots.cpp
index 60de62efe0..dcef5af6b1 100644
--- a/ui/qt/wireshark_main_window_slots.cpp
+++ b/ui/qt/wireshark_main_window_slots.cpp
@@ -3965,12 +3965,10 @@ void WiresharkMainWindow::showExtcapOptionsDialog(QString &device_name, bool sta
     if (extcap_options_dialog) {
         extcap_options_dialog->setModal(true);
         extcap_options_dialog->setAttribute(Qt::WA_DeleteOnClose);
-        if (startCaptureOnClose) {
-            connect(extcap_options_dialog, SIGNAL(finished(int)),
-                        this, SLOT(extcap_options_finished(int)));
-        }
+        connect(extcap_options_dialog, SIGNAL(finished(int)),
+                    this, SLOT(extcap_options_finished(int)));
 #ifdef HAVE_LIBPCAP
-        if (capture_options_dialog_ && startCaptureOnClose) {
+        if (capture_options_dialog_) {
             /* Allow capture options dialog to close */
             connect(extcap_options_dialog, SIGNAL(accepted()),
                     capture_options_dialog_, SLOT(accept()));