PacketList column fixes.

Add a columnsChanged slot to PacketList and move the column update code
from redrawVisiblePackets there. Make sure we call
packet_list_model_->recreateVisibleRows, which should fix the behavior
described in bug 11324. Call columnsChanged when we, uh, change columns.

Add a sectionMoved slot to handle column reordering.

Don't rebuild the column list when we update the widgets in the column
preferences frame.  Do enable and disable the "remove" button as needed.

Try to keep the user from removing all of the columns in both the packet
list and column preferences.

Left as an exercise for the reader: The GTK+ UI also fails when you
remove all of the columns via the preferences:

  packet_list.c:377:packet_list_sort_column: assertion failed: (col)

Bug: 11324
Change-Id: Id58cf98e42cbda9aa2fc370ea06b8bcc6098c8ca
Reviewed-on: https://code.wireshark.org/review/9591
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Gerald Combs <gerald@wireshark.org>

diff --git a/ui/qt/column_preferences_frame.cpp b/ui/qt/column_preferences_frame.cpp
index cb5727382d27ed236d5bcf99ea0194983eedd680..a83111015f3262edc78bf5b2560a25e10f727200 100644 (file)
--- a/ui/qt/column_preferences_frame.cpp
+++ b/ui/qt/column_preferences_frame.cpp
@@ -70,6 +70,17 @@ ColumnPreferencesFrame::ColumnPreferencesFrame(QWidget *parent) :
     ui->columnTreeWidget->setDropIndicatorShown(true);
     ui->columnTreeWidget->setDragDropMode(QAbstractItemView::InternalMove);
 
+    for (GList *cur = g_list_first(prefs.col_list); cur != NULL && cur->data != NULL; cur = cur->next) {
+        fmt_data *cfmt = (fmt_data *) cur->data;
+        addColumn(cfmt->visible, cfmt->title, cfmt->fmt, cfmt->custom_field, cfmt->custom_occurrence);
+    }
+
+    connect(ui->columnTreeWidget, SIGNAL(itemSelectionChanged()), this, SLOT(updateWidgets()));
+
+    if (prefs.num_cols > 0) {
+        ui->columnTreeWidget->topLevelItem(0)->setSelected(true);
+    }
+
     updateWidgets();
 }
 
@@ -194,27 +205,22 @@ void ColumnPreferencesFrame::addColumn(bool visible, const char *title, int fmt,
         item->setText(custom_field_col_, custom_field);
         item->setText(custom_occurrence_col_, QString::number(custom_occurrence));
     }
+
+    updateWidgets();
 }
 
 void ColumnPreferencesFrame::updateWidgets()
 {
-    ui->columnTreeWidget->clear();
-
-    for (GList *cur = g_list_first(prefs.col_list); cur != NULL && cur->data != NULL; cur = cur->next) {
-        fmt_data *cfmt = (fmt_data *) cur->data;
-        addColumn(cfmt->visible, cfmt->title, cfmt->fmt, cfmt->custom_field, cfmt->custom_occurrence);
-    }
-
     ui->columnTreeWidget->resizeColumnToContents(visible_col_);
     ui->columnTreeWidget->resizeColumnToContents(title_col_);
     ui->columnTreeWidget->resizeColumnToContents(type_col_);
+
+    ui->deleteToolButton->setEnabled(ui->columnTreeWidget->selectedItems().count() > 0 && ui->columnTreeWidget->topLevelItemCount() > 1);
 }
 
 
-void ColumnPreferencesFrame::on_columnTreeWidget_currentItemChanged(QTreeWidgetItem *current, QTreeWidgetItem *previous)
+void ColumnPreferencesFrame::on_columnTreeWidget_currentItemChanged(QTreeWidgetItem *, QTreeWidgetItem *previous)
 {
-    ui->deleteToolButton->setEnabled(current ? true : false);
-
     if (previous && ui->columnTreeWidget->itemWidget(previous, title_col_)) {
         ui->columnTreeWidget->removeItemWidget(previous, title_col_);
     }
@@ -228,6 +234,8 @@ void ColumnPreferencesFrame::on_columnTreeWidget_currentItemChanged(QTreeWidgetI
     if (previous && ui->columnTreeWidget->itemWidget(previous, custom_occurrence_col_)) {
         ui->columnTreeWidget->removeItemWidget(previous, custom_occurrence_col_);
     }
+
+    updateWidgets();
 }
 
 void ColumnPreferencesFrame::on_columnTreeWidget_itemActivated(QTreeWidgetItem *item, int column)
@@ -380,10 +388,14 @@ void ColumnPreferencesFrame::on_newToolButton_clicked()
 
 void ColumnPreferencesFrame::on_deleteToolButton_clicked()
 {
+    if (ui->columnTreeWidget->topLevelItemCount() < 2) return;
+
     QTreeWidgetItem *item = ui->columnTreeWidget->currentItem();
     if (item) {
         ui->columnTreeWidget->invisibleRootItem()->removeChild(item);
     }
+
+    updateWidgets();
 }
 
 /*

diff --git a/ui/qt/column_preferences_frame.h b/ui/qt/column_preferences_frame.h
index adca04c0b3fb840df6576f521c9bac5fab5ccd6f..c5c45e28959393a90c44eab2a097ec40c21f24e5 100644 (file)
--- a/ui/qt/column_preferences_frame.h
+++ b/ui/qt/column_preferences_frame.h
@@ -55,10 +55,10 @@ private:
     int saved_combo_idx_;
 
     void addColumn(bool visible, const char *title, int fmt, const char *custom_field, int custom_occurrence);
-    void updateWidgets(void);
 
 private slots:
-    void on_columnTreeWidget_currentItemChanged(QTreeWidgetItem *current, QTreeWidgetItem *previous);
+    void updateWidgets(void);
+    void on_columnTreeWidget_currentItemChanged(QTreeWidgetItem *, QTreeWidgetItem *previous);
     void on_columnTreeWidget_itemActivated(QTreeWidgetItem *item, int column);
     void lineEditDestroyed();
     void comboDestroyed();

diff --git a/ui/qt/main_window.cpp b/ui/qt/main_window.cpp
index 085139cc358a39b8c0e88c81cca61ad1e2cadabd..bd60aef7d7ca6e1ac577306cab1a50dd21ae781f 100644 (file)
--- a/ui/qt/main_window.cpp
+++ b/ui/qt/main_window.cpp
@@ -408,7 +408,7 @@ MainWindow::MainWindow(QWidget *parent) :
     connect(wsApp, SIGNAL(recentFilesRead()),
             packet_list_, SLOT(applyRecentColumnWidths()));
     connect(wsApp, SIGNAL(columnsChanged()),
-            packet_list_, SLOT(redrawVisiblePackets()));
+            packet_list_, SLOT(columnsChanged()));
     connect(wsApp, SIGNAL(recentFilesRead()),
             this, SLOT(applyRecentPaneGeometry()));
     connect(wsApp, SIGNAL(packetDissectionChanged()),

diff --git a/ui/qt/main_window_slots.cpp b/ui/qt/main_window_slots.cpp
index b80b0a012ddba50b669ec8cebd086d8aab6b376c..c4b73e77bfbcd04b8c95df513362da773d4446e9 100644 (file)
--- a/ui/qt/main_window_slots.cpp
+++ b/ui/qt/main_window_slots.cpp
@@ -2402,7 +2402,7 @@ void MainWindow::on_actionAnalyzeCreateAColumn_triggered()
         colnr = column_prefs_add_custom(COL_CUSTOM, capture_file_.capFile()->finfo_selected->hfinfo->name,
                     capture_file_.capFile()->finfo_selected->hfinfo->abbrev,0);
 
-        packet_list_->redrawVisiblePackets();
+        packet_list_->columnsChanged();
         packet_list_->resizeColumnToContents(colnr);
 
         prefs_main_write();

diff --git a/ui/qt/packet_list.cpp b/ui/qt/packet_list.cpp
index 2e48b21a39992250c9809045e6adf7bd02257d7c..76377bfbf782d8f79ea2fda610a158e932511601 100644 (file)
--- a/ui/qt/packet_list.cpp
+++ b/ui/qt/packet_list.cpp
@@ -65,7 +65,6 @@
 #include <QTreeWidget>
 
 // To do:
-// - Catch column reordering and rebuild the column list accoringly.
 // - Use a timer to trigger automatic scrolling.
 
 // If we ever add the ability to open multiple capture files we might be
@@ -416,6 +415,8 @@ PacketList::PacketList(QWidget *parent) :
             this, SLOT(showHeaderMenu(QPoint)));
     connect(header(), SIGNAL(sectionResized(int,int,int)),
             this, SLOT(sectionResized(int,int,int)));
+    connect(header(), SIGNAL(sectionMoved(int,int,int)),
+            this, SLOT(sectionMoved(int,int,int)));
 
     connect(verticalScrollBar(), SIGNAL(actionTriggered(int)), this, SLOT(vScrollBarActionTriggered(int)));
 
@@ -685,8 +686,7 @@ void PacketList::initHeaderContextMenu()
     }
 }
 
-// Redraw the packet list and detail. Called from many places, including
-// columnsChanged.
+// Redraw the packet list and detail. Called from many places.
 // XXX We previously re-selected the packet here, but that seems to cause
 // automatic scrolling problems.
 void PacketList::redrawVisiblePackets() {
@@ -696,13 +696,21 @@ void PacketList::redrawVisiblePackets() {
         proto_tree_->fillProtocolTree(cap_file_->edt->tree);
     }
 
+    update();
+    header()->update();
+}
+
+// prefs.col_list has changed.
+void PacketList::columnsChanged()
+{
+    if (!cap_file_) return;
+
     prefs.num_cols = g_list_length(prefs.col_list);
     col_cleanup(&cap_file_->cinfo);
     build_column_format_array(&cap_file_->cinfo, prefs.num_cols, FALSE);
+    packet_list_model_->recreateVisibleRows(); // Calls PacketListRecord::resetColumns
     setColumnVisibility();
-
-    update();
-    header()->update();
+    redrawVisiblePackets();
 }
 
 // Column widths should
@@ -1157,6 +1165,8 @@ void PacketList::showHeaderMenu(QPoint pos)
     header_actions_[caResolveNames]->setChecked(can_resolve && get_column_resolved(header_ctx_column_));
     header_actions_[caResolveNames]->setEnabled(can_resolve);
 
+    header_actions_[caRemoveColumn]->setEnabled(header_ctx_column_ >= 0 && header()->count() > 2);
+
     foreach (QAction *action, show_hide_actions_) {
         header_ctx_menu_.removeAction(action);
         delete action;
@@ -1214,13 +1224,16 @@ void PacketList::headerMenuTriggered()
         hideColumn(header_ctx_column_);
         break;
     case caRemoveColumn:
-        column_prefs_remove_nth(header_ctx_column_);
-        if (!prefs.gui_use_pref_save) {
-            prefs_main_write();
+    {
+        if (header()->count() > 2) {
+            column_prefs_remove_nth(header_ctx_column_);
+            columnsChanged();
+            if (!prefs.gui_use_pref_save) {
+                prefs_main_write();
+            }
         }
-        setColumnVisibility();
-        redraw = true;
         break;
+    }
     default:
         break;
     }
@@ -1250,6 +1263,46 @@ void PacketList::sectionResized(int col, int, int new_width)
     }
 }
 
+// The user moved a column. Make sure prefs.col_list, the column format
+// array, and the header's visual and logical indices all agree.
+// gtk/packet_list.c:column_dnd_changed_cb
+void PacketList::sectionMoved(int, int, int)
+{
+    GList *new_col_list = NULL;
+    QList<int> saved_sizes;
+
+    // Build a new column list based on the header's logical order.
+    for (int vis_idx = 0; vis_idx < header()->count(); vis_idx++) {
+        int log_idx = header()->logicalIndex(vis_idx);
+        saved_sizes << header()->sectionSize(log_idx);
+
+        void *pref_data = g_list_nth_data(prefs.col_list, log_idx);
+        if (!pref_data) continue;
+
+        new_col_list = g_list_append(new_col_list, pref_data);
+    }
+
+    // Clear and rebuild our (and the header's) model. There doesn't appear
+    // to be another way to reset the logical index.
+    freeze();
+
+    g_list_free(prefs.col_list);
+    prefs.col_list = new_col_list;
+
+    thaw();
+
+    for (int i = 0; i < saved_sizes.length(); i++) {
+        if (saved_sizes[i] < 1) continue;
+        header()->resizeSection(i, saved_sizes[i]);
+    }
+
+    if (!prefs.gui_use_pref_save) {
+        prefs_main_write();
+    }
+
+    wsApp->emitAppSignal(WiresharkApplication::ColumnsChanged);
+}
+
 // We need to tell when the user has scrolled the packet list, either to
 // the end or anywhere other than the end.
 void PacketList::vScrollBarActionTriggered(int)

diff --git a/ui/qt/packet_list.h b/ui/qt/packet_list.h
index de08a228beccb31784022268e15b583ddbfff662..715a4ebb042c7c2d5cd53c5e00f6460cd54e040c 100644 (file)
--- a/ui/qt/packet_list.h
+++ b/ui/qt/packet_list.h
@@ -135,6 +135,7 @@ public slots:
     void setTimeReference();
     void unsetAllTimeReferences();
     void redrawVisiblePackets();
+    void columnsChanged();
     void applyRecentColumnWidths();
 
 private slots:
@@ -142,6 +143,7 @@ private slots:
     void headerMenuTriggered();
     void columnVisibilityTriggered();
     void sectionResized(int col, int, int new_width);
+    void sectionMoved(int, int, int);
     void vScrollBarActionTriggered(int);
 };