Qt: Fix profile copy and delete

Delete lead to an issue with the sort model, where indeces where
no longer correctly associated with the source model.

Copy did not check if the name already existed in the dialog

Change-Id: I3668dffb1e1956f04a5b9e21c3fcdac266e0cc0c
Reviewed-on: https://code.wireshark.org/review/33911
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Roland Knall <rknall@gmail.com>

5 files changed, 71 insertions, 30 deletions

diff --git a/ui/qt/models/profile_model.cpp b/ui/qt/models/profile_model.cpp
index 1b2bc41056..98b1938b44 100644
--- a/ui/qt/models/profile_model.cpp
+++ b/ui/qt/models/profile_model.cpp
@@ -168,6 +168,8 @@ int ProfileModel::columnCount(const QModelIndex &) const
 
 QVariant ProfileModel::data(const QModelIndex &index, int role) const
 {
+    QString msg;
+
     if ( ! index.isValid() || profiles_.count() <= index.row() )
         return QVariant();
 
@@ -234,18 +236,19 @@ QVariant ProfileModel::data(const QModelIndex &index, int role) const
         return font;
     }
 
-    case Qt::BackgroundRole:
+    case Qt::BackgroundColorRole:
     {
-        QBrush bgBrush;
-
-        if ( ! profile_name_is_valid(prof->name) )
-            bgBrush.setColor(ColorUtils::fromColorT(&prefs.gui_text_invalid));
+        if ( ! ProfileModel::checkNameValidity(QString(prof->name)) )
+            return ColorUtils::fromColorT(&prefs.gui_text_invalid);
 
         if ( prof->status == PROF_STAT_DEFAULT && reset_default_ )
-            bgBrush.setColor(ColorUtils::fromColorT(&prefs.gui_text_deprecated));
+            return ColorUtils::fromColorT(&prefs.gui_text_deprecated);
 
+        QList<int> rows = const_cast<ProfileModel *>(this)->findAllByNameAndVisibility(QString(prof->name), prof->is_global);
+        if ( rows.count() > 1 )
+            return ColorUtils::fromColorT(&prefs.gui_text_invalid);
 
-        return bgBrush;
+        break;
     }
 
     case Qt::ToolTipRole:
@@ -290,11 +293,8 @@ QVariant ProfileModel::data(const QModelIndex &index, int role) const
             break;
         }
 
-        if (gchar * err_msg = profile_name_is_valid(prof->name))
-        {
-            QString msg = gchar_free_to_qstring(err_msg);
+        if ( ! ProfileModel::checkNameValidity(QString(prof->name), &msg) )
             return msg;
-        }
 
         if (prof->is_global)
             return tr("This is a system provided profile.");
@@ -409,15 +409,23 @@ int ProfileModel::findByName(QString name)
 
 int ProfileModel::findByNameAndVisibility(QString name, bool isGlobal)
 {
-    int row = -1;
-    for ( int cnt = 0; cnt < profiles_.count() && row < 0; cnt++ )
+    QList<int> result = findAllByNameAndVisibility(name, isGlobal);
+    return result.count() == 0 ? -1 : result.at(0);
+}
+
+QList<int> ProfileModel::findAllByNameAndVisibility(QString name, bool isGlobal)
+{
+    QList<int> result;
+
+    for ( int cnt = 0; cnt < profiles_.count(); cnt++ )
     {
         profile_def * prof = profiles_.at(cnt);
         if ( prof && static_cast<bool>(prof->is_global) == isGlobal && name.compare(prof->name) == 0 )
-            row = cnt;
+            result << cnt;
     }
 
-    return row;
+    return result;
+
 }
 
 QModelIndex ProfileModel::addNewProfile(QString name)
@@ -487,8 +495,8 @@ void ProfileModel::deleteEntry(QModelIndex idx)
         GList * fl_entry = entry(prof);
         emit beginRemoveRows(QModelIndex(), idx.row(), idx.row());
         remove_from_profile_list(fl_entry);
-        loadProfiles();
         emit endRemoveRows();
+        loadProfiles();
     }
 }
 
@@ -544,6 +552,32 @@ bool ProfileModel::setData(const QModelIndex &idx, const QVariant &value, int ro
     return true;
 }
 
+bool ProfileModel::checkNameValidity(QString name, QString *msg)
+{
+    QString message;
+
+    QString invalid_dir_chars("\\/:*?\"<>|");
+    bool invalid = false;
+    for ( int cnt = 0; cnt < invalid_dir_chars.length() && ! invalid; cnt++ )
+    {
+        if ( name.contains(invalid_dir_chars[cnt]) )
+            invalid = true;
+    }
+    if ( invalid )
+        message = tr("A profile must not contain any of the following characters: %1").arg(invalid_dir_chars);
+
+    if ( message.isEmpty() && ( name.startsWith('.') || name.endsWith('.') ) )
+        message = tr("A profile must not start or end with a period (.)");
+
+    if (! message.isEmpty()) {
+        if (msg)
+            msg->append(message);
+        return false;
+    }
+
+    return true;
+}
+
 /*
  * Editor modelines
  *
diff --git a/ui/qt/models/profile_model.h b/ui/qt/models/profile_model.h
index ab3320ef4e..b49b593377 100644
--- a/ui/qt/models/profile_model.h
+++ b/ui/qt/models/profile_model.h
@@ -71,6 +71,7 @@ public:
     virtual int rowCount(const QModelIndex & parent = QModelIndex()) const;
     virtual int columnCount(const QModelIndex & parent = QModelIndex()) const;
     virtual QVariant data(const QModelIndex & idx, int role = Qt::DisplayRole) const;
+    virtual bool setData(const QModelIndex &index, const QVariant &value, int role);
     virtual QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const;
     virtual Qt::ItemFlags flags(const QModelIndex &index) const;
 
@@ -87,6 +88,9 @@ public:
 
     GList * at(int row) const;
 
+    static bool checkNameValidity(QString name, QString *msg = Q_NULLPTR);
+    QList<int> findAllByNameAndVisibility(QString name, bool isGlobal = false);
+
 private:
     QList<profile_def *> profiles_;
     QString set_profile_;
@@ -96,10 +100,6 @@ private:
     GList * entry(profile_def *) const;
 
     int findByNameAndVisibility(QString name, bool isGlobal = false);
-
-    // QAbstractItemModel interface
-public:
-    virtual bool setData(const QModelIndex &index, const QVariant &value, int role);
 };
 
 #endif
diff --git a/ui/qt/profile_dialog.cpp b/ui/qt/profile_dialog.cpp
index f929e9e411..158b6322b2 100644
--- a/ui/qt/profile_dialog.cpp
+++ b/ui/qt/profile_dialog.cpp
@@ -70,8 +70,8 @@ ProfileDialog::ProfileDialog(QWidget *parent) :
     connect(pd_ui_->profileTreeView, &ProfileTreeView::currentItemChanged,
             this, &ProfileDialog::currentItemChanged);
 
-    connect(pd_ui_->profileTreeView->itemDelegate(), SIGNAL(closeEditor(QWidget*, QAbstractItemDelegate::EndEditHint)),
-            this, SLOT(editingFinished()));
+    connect(pd_ui_->profileTreeView, &ProfileTreeView::itemUpdated,
+            this, &ProfileDialog::editingFinished);
 
     /* Select the row for the currently selected profile or the first row if non is selected*/
     selectProfile();
@@ -155,9 +155,8 @@ void ProfileDialog::updateWidgets()
             QModelIndex idx = model_->index(row, ProfileModel::COL_NAME);
             QString name = idx.data().toString();
 
-            if (gchar * err_msg = profile_name_is_valid(name.toLatin1().data()))
+            if ( ! ProfileModel::checkNameValidity(name) )
             {
-                pd_ui_->infoLabel->setText(gchar_free_to_qstring(err_msg));
                 enable_ok = false;
                 continue;
             }
@@ -165,13 +164,12 @@ void ProfileDialog::updateWidgets()
             if ( idx != index && idx.data().toString().compare(index.data().toString()) == 0 )
             {
                 if (idx.data(ProfileModel::DATA_IS_GLOBAL).toBool() == index.data(ProfileModel::DATA_IS_GLOBAL).toBool())
-                {
-                    int status = index.data(ProfileModel::DATA_STATUS).toInt();
-                    if (status != PROF_STAT_DEFAULT && status != PROF_STAT_EXISTS)
-                        pd_ui_->infoLabel->setText(tr("A profile already exists with this name"));
                     enable_ok = false;
-                }
             }
+
+            QList<int> rows = model_->findAllByNameAndVisibility(name, idx.data(ProfileModel::DATA_IS_GLOBAL).toBool());
+            if ( rows.count() > 1 )
+                enable_ok = false;
         }
     }
 
@@ -198,6 +196,7 @@ void ProfileDialog::on_newToolButton_clicked()
     {
         pd_ui_->profileTreeView->setCurrentIndex(ridx);
         pd_ui_->profileTreeView->edit(ridx);
+        updateWidgets();
     }
 }
 
@@ -206,6 +205,7 @@ void ProfileDialog::on_deleteToolButton_clicked()
     QModelIndex index = sort_model_->mapToSource(pd_ui_->profileTreeView->currentIndex());
 
     model_->deleteEntry(index);
+    updateWidgets();
 }
 
 void ProfileDialog::on_copyToolButton_clicked()
@@ -223,6 +223,7 @@ void ProfileDialog::on_copyToolButton_clicked()
     {
         pd_ui_->profileTreeView->setCurrentIndex(sort_model_->mapFromSource(ridx));
         pd_ui_->profileTreeView->edit(sort_model_->mapFromSource(ridx));
+        updateWidgets();
     }
 }
 
diff --git a/ui/qt/widgets/profile_tree_view.cpp b/ui/qt/widgets/profile_tree_view.cpp
index 47c8fa310b..6d00719f70 100644
--- a/ui/qt/widgets/profile_tree_view.cpp
+++ b/ui/qt/widgets/profile_tree_view.cpp
@@ -46,12 +46,14 @@ void ProfileTreeEditDelegate::setEditorData(QWidget *editor, const QModelIndex &
 ProfileTreeView::ProfileTreeView(QWidget *parent) :
     QTreeView (parent)
 {
-    setItemDelegateForColumn(ProfileModel::COL_NAME, new ProfileTreeEditDelegate());
+    delegate_ = new ProfileTreeEditDelegate();
+    setItemDelegateForColumn(ProfileModel::COL_NAME, delegate_);
     setItemDelegateForColumn(ProfileModel::COL_PATH, new ProfileUrlLinkDelegate());
 
     resizeColumnToContents(ProfileModel::COL_NAME);
 
     connect(this, &QAbstractItemView::clicked, this, &ProfileTreeView::clicked);
+    connect(delegate_, SIGNAL(commitData(QWidget *)), this, SIGNAL(itemUpdated()));
 }
 
 void ProfileTreeView::selectionChanged(const QItemSelection &selected, const QItemSelection &deselected)
diff --git a/ui/qt/widgets/profile_tree_view.h b/ui/qt/widgets/profile_tree_view.h
index 3c83ec5278..a0c34c8743 100644
--- a/ui/qt/widgets/profile_tree_view.h
+++ b/ui/qt/widgets/profile_tree_view.h
@@ -44,6 +44,7 @@ public:
 
 Q_SIGNALS:
     void currentItemChanged();
+    void itemUpdated();
 
     // QAbstractItemView interface
 protected slots:
@@ -51,6 +52,9 @@ protected slots:
     virtual void currentChanged(const QModelIndex &current, const QModelIndex &previous);
 
     virtual void clicked(const QModelIndex &index);
+
+private:
+    ProfileTreeEditDelegate *delegate_;
 };
 
 #endif