Qt: More byte view and proto tree fixes.
authorGerald Combs <gerald@wireshark.org>
Wed, 31 Jan 2018 19:26:18 +0000 (11:26 -0800)
committerAnders Broman <a.broman58@gmail.com>
Thu, 1 Feb 2018 14:04:25 +0000 (14:04 +0000)
47e1798762 broke byte view highlighting when selecting a proto tree
item. Switch back to emitting fieldSelected from selectionChanged. Force
a new selection in selectedFieldChanged, which does what we were trying
to do in 47e1798762.

Clear our marked byte offset in the byte view when we mark a field. Emit
byteSelected whenever we click the mouse.

Don't highlight anything when a tree item is deselected. Deselect a tree
item if we click on something that's not a byte in the byte view.

Change-Id: Ibf419ccb005d69f733b2fe12ce674e1fe504bb96
Reviewed-on: https://code.wireshark.org/review/25541
Reviewed-by: Gerald Combs <gerald@wireshark.org>
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
ui/qt/byte_view_tab.cpp
ui/qt/proto_tree.cpp
ui/qt/widgets/byte_view_text.cpp

index 2974a215ed119069d63849567da60607fdd0e00a..2f597d638c312f92c51a11617056412310425ec5 100644 (file)
@@ -255,7 +255,11 @@ void ByteViewTab::selectedFrameChanged(int frameNum)
 
 void ByteViewTab::selectedFieldChanged(FieldInformation *selected)
 {
-    ByteViewText * byte_view_text = 0;
+    // We need to handle both selection and deselection.
+    ByteViewText * byte_view_text = qobject_cast<ByteViewText *>(currentWidget());
+    int f_start = -1, f_length = -1;
+    int p_start = -1, p_length = -1;
+    int fa_start = -1, fa_length = -1;
 
     if (selected) {
         if (selected->parent() == this) {
@@ -268,29 +272,31 @@ void ByteViewTab::selectedFieldChanged(FieldInformation *selected)
         if ( fi )
             byte_view_text = findByteViewTextForTvb(fi->ds_tvb, &idx);
 
-        if (byte_view_text)
-        {
-            int f_start = -1, f_length = -1;
-
-            if (cap_file_->search_in_progress && (cap_file_->hex || (cap_file_->string && cap_file_->packet_data))) {
-                // In the hex view, only highlight the target bytes or string. The entire
-                // field can then be displayed by clicking on any of the bytes in the field.
-                f_start = cap_file_->search_pos - cap_file_->search_len + 1;
-                f_length = (int) cap_file_->search_len;
-            } else {
-                f_start = selected->position().start;
-                f_length = selected->position().length;
-            }
+        if (cap_file_->search_in_progress && (cap_file_->hex || (cap_file_->string && cap_file_->packet_data))) {
+            // In the hex view, only highlight the target bytes or string. The entire
+            // field can then be displayed by clicking on any of the bytes in the field.
+            f_start = cap_file_->search_pos - cap_file_->search_len + 1;
+            f_length = (int) cap_file_->search_len;
+        } else {
+            f_start = selected->position().start;
+            f_length = selected->position().length;
+        }
 
-            setCurrentIndex(idx);
+        setCurrentIndex(idx);
 
-            byte_view_text->markField(f_start, f_length);
-            byte_view_text->markProtocol(selected->parentField()->position().start, selected->parentField()->position().length);
-            byte_view_text->markAppendix(selected->appendix().start, selected->appendix().length);
-        }
+        p_start = selected->parentField()->position().start;
+        p_length = selected->parentField()->position().length;
+        fa_start = selected->appendix().start;
+        fa_length = selected->appendix().length;
     }
-}
 
+    if (byte_view_text)
+    {
+        byte_view_text->markField(f_start, f_length);
+        byte_view_text->markProtocol(p_start, p_length);
+        byte_view_text->markAppendix(fa_start, fa_length);
+    }
+}
 void ByteViewTab::highlightedFieldChanged(FieldInformation *highlighted)
 {
     ByteViewText * byte_view_text = qobject_cast<ByteViewText *>(currentWidget());
index ae5f0c49b65424921afe600bcc49fbfa6f067bcc..89e225e947071352728c7f8c6c8dd23d8f9e6bb3 100644 (file)
@@ -282,22 +282,6 @@ void ProtoTree::autoScrollTo(const QModelIndex &index)
         return;
     }
 
-    // Find and highlight the protocol bytes. select above won't call
-    // selectionChanged if the current and selected indexes are the same
-    // so we do this here.
-    FieldInformation finfo(proto_tree_model_->protoNodeFromIndex(index).protoNode(), this);
-    if (finfo.isValid()) {
-        QModelIndex parent = index;
-        while (parent.isValid() && parent.parent().isValid()) {
-            parent = parent.parent();
-        }
-        if (parent.isValid()) {
-            FieldInformation parent_finfo(proto_tree_model_->protoNodeFromIndex(parent).protoNode());
-            finfo.setParentField(parent_finfo.fieldInfo());
-        }
-        emit fieldSelected(&finfo);
-    }
-
     ScrollHint scroll_hint = PositionAtTop;
     if (prefs.gui_auto_scroll_percentage > 66) {
         scroll_hint = PositionAtBottom;
@@ -317,10 +301,29 @@ void ProtoTree::goToHfid(int hfid)
 void ProtoTree::selectionChanged(const QItemSelection &selected, const QItemSelection &deselected)
 {
     QTreeView::selectionChanged(selected, deselected);
-    if (selected.isEmpty()) return;
+    if (selected.isEmpty()) {
+        emit fieldSelected(0);
+        return;
+    }
 
     QModelIndex index = selected.indexes().first();
     saveSelectedField(index);
+
+    // Find and highlight the protocol bytes. select above won't call
+    // selectionChanged if the current and selected indexes are the same
+    // so we do this here.
+    FieldInformation finfo(proto_tree_model_->protoNodeFromIndex(index).protoNode(), this);
+    if (finfo.isValid()) {
+        QModelIndex parent = index;
+        while (parent.isValid() && parent.parent().isValid()) {
+            parent = parent.parent();
+        }
+        if (parent.isValid()) {
+            FieldInformation parent_finfo(proto_tree_model_->protoNodeFromIndex(parent).protoNode());
+            finfo.setParentField(parent_finfo.fieldInfo());
+        }
+        emit fieldSelected(&finfo);
+    }
 }
 
 void ProtoTree::syncExpanded(const QModelIndex &index) {
@@ -431,12 +434,19 @@ void ProtoTree::itemDoubleClicked(const QModelIndex &index) {
 
 void ProtoTree::selectedFieldChanged(FieldInformation *finfo)
 {
-    QModelIndex index = proto_tree_model_->findFieldInformation(finfo);
-    if (!index.isValid() || finfo->parent() == this) {
-        // We only want valid, inbound signals.
+    if (finfo && finfo->parent() == this) {
+        // We only want inbound signals.
         return;
     }
+
+    QModelIndex index = proto_tree_model_->findFieldInformation(finfo);
+    setUpdatesEnabled(false);
+    // The new finfo might match the current index. Clear our selection
+    // so that we force a fresh item selection, so that fieldSelected
+    // will in turn be emitted.
+    selectionModel()->clearSelection();
     autoScrollTo(index);
+    setUpdatesEnabled(true);
 }
 
 // Remember the currently focussed field based on:
index ed16261391be52ad7619cbd8d2aa8f2576b95700..cd6eada7f2cd5c93016b63bff5d7c4e092db9468 100644 (file)
@@ -147,6 +147,9 @@ void ByteViewText::markField(int start, int length, bool scroll_to)
 {
     field_start_ = start;
     field_len_ = length;
+    // This might be called as a result of (de)selecting a proto tree
+    // item, so take us out of marked mode.
+    marked_byte_offset_ = -1;
     if (scroll_to) {
         scrollToByte(start);
     }
@@ -264,17 +267,26 @@ void ByteViewText::mousePressEvent (QMouseEvent *event) {
         return;
     }
 
-    if (marked_byte_offset_ < 0) {
-        // Marked mode.
-        marked_byte_offset_ = byteOffsetAtPixel(event->pos());
+    // byteSelected does the following:
+    // - Triggers selectedFieldChanged in ProtoTree, which clears the
+    //   selection and selects the corresponding (or no) item.
+    // - The new tree selection triggers markField, which clobbers
+    //   marked_byte_offset_.
+
+    const bool hover_mode = marked_byte_offset_ < 0;
+    const int byte_offset = byteOffsetAtPixel(event->pos());
+    setUpdatesEnabled(false);
+    emit byteSelected(byte_offset);
+    if (hover_mode && byte_offset >= 0) {
+        // Switch to marked mode.
         hovered_byte_offset_ = -1;
-        emit byteSelected(marked_byte_offset_);
+        marked_byte_offset_ = byte_offset;
         viewport()->update();
     } else {
         // Back to hover mode.
-        marked_byte_offset_ = -1;
         mouseMoveEvent(event);
     }
+    setUpdatesEnabled(true);
 }
 
 void ByteViewText::mouseMoveEvent(QMouseEvent *event)
@@ -290,7 +302,8 @@ void ByteViewText::mouseMoveEvent(QMouseEvent *event)
 
 void ByteViewText::leaveEvent(QEvent *event)
 {
-    emit byteHovered(-1);
+    hovered_byte_offset_ = -1;
+    emit byteHovered(hovered_byte_offset_);
 
     viewport()->update();
     QAbstractScrollArea::leaveEvent(event);