From 72427192723869535fb938425ddf8c5c4568ab5c Mon Sep 17 00:00:00 2001
From: Gerald Combs <gerald@wireshark.org>
Date: Tue, 29 Nov 2016 09:32:47 -0800
Subject: [PATCH] Qt: Fix simple_dialog formatting.

Make sure that simple_dialog displays plain text. Trim whitespace
and remove excessive newlines in order to improve message formatting.
Add a comment about simple_dialog's behavior in Qt and GTK+ and how it
might be improved.

Bug: 13178
Change-Id: Ic6ff3cecd5ef1d76ec095d7a409f38e602b41ce2
Reviewed-on: https://code.wireshark.org/review/18985
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Jaap Keuter <jaap.keuter@xs4all.nl>
Reviewed-by: Gerald Combs <gerald@wireshark.org>
---
 dumpcap.c               |  2 +-
 ui/qt/simple_dialog.cpp | 21 +++++++++------------
 ui/simple_dialog.h      | 31 ++++++++++++++++++++++++++-----
 3 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/dumpcap.c b/dumpcap.c
index be96c173a7..632dda33f8 100644
--- a/dumpcap.c
+++ b/dumpcap.c
@@ -644,7 +644,7 @@ get_capture_device_open_failure_messages(const char *open_err_str,
                  "Please check that \"%s\" is the proper interface.\n"
                  "\n"
                  "\n"
-                 "Help can be found at:\n"
+                 "Help can be found on the following pages:\n"
                  "\n"
                  "       https://wiki.wireshark.org/WinPcap\n"
                  "       https://wiki.wireshark.org/CaptureSetup\n",
diff --git a/ui/qt/simple_dialog.cpp b/ui/qt/simple_dialog.cpp
index 78a2adb259..6c1c7becfd 100644
--- a/ui/qt/simple_dialog.cpp
+++ b/ui/qt/simple_dialog.cpp
@@ -29,13 +29,14 @@
 #include "wireshark_application.h"
 
 #include <QMessageBox>
+#include <QRegExp>
 #include <QTextCodec>
 
 /* Simple dialog function - Displays a dialog box with the supplied message
  * text.
  *
  * This is meant to be used as a backend for the functions defined in
- *  ui/simple_dialog.h. Qt code should use QMessageBox directly.
+ * ui/simple_dialog.h. Qt code should use QMessageBox directly.
  *
  * Args:
  * type       : One of ESD_TYPE_*.
@@ -62,14 +63,7 @@ simple_dialog_primary_end(void) {
 char *
 simple_dialog_format_message(const char *msg)
 {
-    char *str;
-
-    if (msg) {
-        str = xml_escape(msg);
-    } else {
-        str = NULL;
-    }
-    return str;
+    return g_strdup(msg);
 }
 
 /*
@@ -95,10 +89,13 @@ SimpleDialog::SimpleDialog(QWidget *parent, ESD_TYPE_E type, int btn_mask, const
     message = QTextCodec::codecForLocale()->toUnicode(vmessage);
     g_free(vmessage);
 
-    setTextFormat(Qt::RichText);
+    setTextFormat(Qt::PlainText);
+
     MessagePair msg_pair = splitMessage(message);
-    QString primary = msg_pair.first;
-    QString secondary = msg_pair.second;
+    // Remove leading and trailing whitespace along with excessive newline runs.
+    QString primary = msg_pair.first.trimmed();
+    QString secondary = msg_pair.second.trimmed();
+    secondary.replace(QRegExp("\n\n+"), "\n\n");
 
     if (primary.isEmpty()) {
         return;
diff --git a/ui/simple_dialog.h b/ui/simple_dialog.h
index ed4139a8a3..d603d069f2 100644
--- a/ui/simple_dialog.h
+++ b/ui/simple_dialog.h
@@ -80,11 +80,32 @@ typedef enum {
 
 /** Create and show a simple dialog.
  *
- * @param type type of dialog
- * @param btn_mask the buttons to display
- * @param msg_format printf like message format
- * @param ... printf like parameters
- * @return the newly created dialog
+ * @param Type type of dialog, e.g. ESD_TYPE_WARN
+ * @param btn_mask The buttons to display, e.g. ESD_BTNS_OK_CANCEL
+ * @param msg_format Printf like message format. Text must be plain.
+ * @param ... Printf like parameters
+ * @return The newly created dialog
+ */
+/*
+ * XXX This is a bit clunky. We typically pass in:
+ * - simple_dialog_primary_start
+ * - The primary message
+ * - simple_dialog_primary_end
+ * - Optionally, the secondary message.
+ *
+ * In the GTK+ UI primary_start and primary_end make up a <span> that adds
+ * text formatting. The whole string is then shoved into a GtkLabel.
+ *
+ * In the Qt UI we use primary_start and _end to split the primary and
+ * secondary messages. They are then added to a QMessageBox via setText and
+ * setInformativeText respectively. No formatting is applied.
+ *
+ * Callers are responsible for wrapping the primary message and formatting
+ * the message text.
+ *
+ * Explicitly passing in separate primary and secondary messages would let us
+ * get rid of primary_start and primary_end and reduce the amount of
+ * gymnastics we have to to in the Qt UI.
  */
 extern gpointer simple_dialog(ESD_TYPE_E type, gint btn_mask,
     const gchar *msg_format, ...)