Qt: Fix File Path editor in table

The current situation in UatDelegate as well as PathChooserDelegate leads to issues, where Wireshark crashes if the button is clicked. This is due to the UI not correctly positioning the button inside the cell.

This change implements a widget, which will serve as cell content, handling all relations with choosing the file, but also properly handling the size and geometry of said cell content, therefore no longer leading to crashes and cleaning up code at the same time, as duplicate methods are being removed.

Fixes #17789, #17819, #18088
This commit is contained in:
Roland Knall 2022-05-16 09:54:08 +00:00
parent 52054046c1
commit acdda8eb6b
10 changed files with 266 additions and 179 deletions

View File

@ -40,6 +40,7 @@ set(WIRESHARK_WIDGET_HEADERS
../qt/widgets/label_stack.h
../qt/widgets/overlay_scroll_bar.h
../qt/widgets/packet_list_header.h
../qt/widgets/path_selection_edit.h
../qt/widgets/pref_module_view.h
../qt/widgets/profile_tree_view.h
../qt/widgets/range_syntax_lineedit.h
@ -105,7 +106,7 @@ set(WIRESHARK_MODEL_HEADERS
../qt/models/numeric_value_chooser_delegate.h
../qt/models/packet_list_model.h
../qt/models/packet_list_record.h
../qt/models/path_chooser_delegate.h
../qt/models/path_selection_delegate.h
../qt/models/percent_bar_delegate.h
../qt/models/pref_delegate.h
../qt/models/pref_models.h
@ -262,6 +263,7 @@ set(WIRESHARK_WIDGET_SRCS
../qt/widgets/label_stack.cpp
../qt/widgets/overlay_scroll_bar.cpp
../qt/widgets/packet_list_header.cpp
../qt/widgets/path_selection_edit.cpp
../qt/widgets/pref_module_view.cpp
../qt/widgets/profile_tree_view.cpp
../qt/widgets/range_syntax_lineedit.cpp
@ -324,7 +326,7 @@ set(WIRESHARK_MODEL_SRCS
../qt/models/numeric_value_chooser_delegate.cpp
../qt/models/packet_list_model.cpp
../qt/models/packet_list_record.cpp
../qt/models/path_chooser_delegate.cpp
../qt/models/path_selection_delegate.cpp
../qt/models/percent_bar_delegate.cpp
../qt/models/pref_delegate.cpp
../qt/models/pref_models.cpp

View File

@ -40,6 +40,7 @@ set(WIRESHARK_WIDGET_HEADERS
widgets/label_stack.h
widgets/overlay_scroll_bar.h
widgets/packet_list_header.h
widgets/path_selection_edit.h
widgets/pref_module_view.h
widgets/profile_tree_view.h
widgets/range_syntax_lineedit.h
@ -103,7 +104,7 @@ set(WIRESHARK_MODEL_HEADERS
models/numeric_value_chooser_delegate.h
models/packet_list_model.h
models/packet_list_record.h
models/path_chooser_delegate.h
models/path_selection_delegate.h
models/percent_bar_delegate.h
models/pref_delegate.h
models/pref_models.h
@ -287,6 +288,7 @@ set(WIRESHARK_WIDGET_SRCS
widgets/label_stack.cpp
widgets/overlay_scroll_bar.cpp
widgets/packet_list_header.cpp
widgets/path_selection_edit.cpp
widgets/pref_module_view.cpp
widgets/profile_tree_view.cpp
widgets/range_syntax_lineedit.cpp
@ -347,7 +349,7 @@ set(WIRESHARK_MODEL_SRCS
models/numeric_value_chooser_delegate.cpp
models/packet_list_model.cpp
models/packet_list_record.cpp
models/path_chooser_delegate.cpp
models/path_selection_delegate.cpp
models/percent_bar_delegate.cpp
models/pref_delegate.cpp
models/pref_models.cpp

View File

@ -37,7 +37,7 @@
#include "ui/capture_ui_utils.h"
#include <ui/qt/models/path_chooser_delegate.h>
#include <ui/qt/models/path_selection_delegate.h>
#include <QCheckBox>
#include <QHBoxLayout>
@ -49,7 +49,7 @@
// To do:
// - Check the validity of pipes and remote interfaces and provide feedback
// via hintLabel.
// - We might want to move PathChooserDelegate to its own module and use it in
// - We might want to move PathSelectionDelegate to its own module and use it in
// other parts of the application such as the general preferences and UATs.
// Qt Creator has a much more elaborate version from which we might want
// to draw inspiration.
@ -176,9 +176,7 @@ ManageInterfacesDialog::ManageInterfacesDialog(QWidget *parent) :
ui->pipeView->setModel(pipeProxyModel);
ui->delPipe->setEnabled(pipeProxyModel->rowCount() > 0);
ui->pipeView->setItemDelegateForColumn(
pipeProxyModel->mapSourceToColumn(IFTREE_COL_PIPE_PATH), new PathChooserDelegate()
);
ui->pipeView->setItemDelegateForColumn(pipeProxyModel->mapSourceToColumn(IFTREE_COL_PIPE_PATH), new PathSelectionDelegate());
connect(ui->pipeView->selectionModel(), &QItemSelectionModel::selectionChanged, this, [=](const QItemSelection &sel, const QItemSelection &) {
ui->delPipe->setEnabled(sel.count() > 0);
});

View File

@ -1,128 +0,0 @@
/* path_chooser_delegate.cpp
* Delegate to select a file path for a treeview entry
*
* Wireshark - Network traffic analyzer
* By Gerald Combs <gerald@wireshark.org>
* Copyright 1998 Gerald Combs
*
* SPDX-License-Identifier: GPL-2.0-or-later
*/
#include "config.h"
#include "epan/prefs.h"
#include "ui/last_open_dir.h"
#include <ui/qt/models/path_chooser_delegate.h>
#include "ui/qt/widgets/wireshark_file_dialog.h"
#include <QHBoxLayout>
#include <QPushButton>
#include <QWidget>
#include <QLineEdit>
PathChooserDelegate::PathChooserDelegate(QObject *parent)
: QStyledItemDelegate(parent)
{
}
QWidget* PathChooserDelegate::createEditor(QWidget *parent, const QStyleOptionViewItem &option, const QModelIndex &) const
{
QWidget * pathEditor = new QWidget(parent);
QHBoxLayout *hbox = new QHBoxLayout(pathEditor);
pathEditor->setLayout(hbox);
QLineEdit * lineEdit = new QLineEdit(pathEditor);
QPushButton *btnBrowse = new QPushButton(pathEditor);
btnBrowse->setText(tr("Browse"));
hbox->setContentsMargins(0, 0, 0, 0);
hbox->addWidget(lineEdit);
hbox->addWidget(btnBrowse);
hbox->setSizeConstraint(QLayout::SetMinimumSize);
// Grow the item to match the editor. According to the QAbstractItemDelegate
// documenation we're supposed to reimplement sizeHint but this seems to work.
QSize size = option.rect.size();
size.setHeight(qMax(option.rect.height(), hbox->sizeHint().height()));
lineEdit->selectAll();
pathEditor->setFocusProxy(lineEdit);
pathEditor->setFocusPolicy(lineEdit->focusPolicy());
connect(btnBrowse, &QPushButton::clicked, this, &PathChooserDelegate::browseButtonClicked);
return pathEditor;
}
void PathChooserDelegate::updateEditorGeometry(QWidget *editor, const QStyleOptionViewItem &option, const QModelIndex &) const
{
QRect rect = option.rect;
// Make sure the editor doesn't get squashed.
editor->adjustSize();
rect.setHeight(qMax(option.rect.height(), editor->height()));
editor->setGeometry(rect);
}
void PathChooserDelegate::browseButtonClicked()
{
char *open_dir = NULL;
switch (prefs.gui_fileopen_style)
{
case FO_STYLE_LAST_OPENED:
open_dir = get_last_open_dir();
break;
case FO_STYLE_SPECIFIED:
if (prefs.gui_fileopen_dir[0] != '\0')
open_dir = prefs.gui_fileopen_dir;
break;
}
QWidget * qw = new QWidget();
QString file_name = WiresharkFileDialog::getOpenFileName(qw, tr("Open Pipe"), open_dir);
if (!file_name.isEmpty())
{
QWidget * parent = ((QPushButton *)sender())->parentWidget();
QLineEdit * lineEdit = parent->findChild<QLineEdit*>();
if (lineEdit)
{
lineEdit->setText(file_name);
emit commitData(parent);
}
}
delete(qw);
}
void PathChooserDelegate::setEditorData(QWidget *editor, const QModelIndex &idx) const
{
if (idx.isValid())
{
QString content = idx.data().toString();
QLineEdit * lineEdit = editor->findChild<QLineEdit*>();
if (lineEdit)
{
lineEdit->setText(content);
}
}
else
QStyledItemDelegate::setEditorData(editor, idx);
}
void PathChooserDelegate::setModelData(QWidget *editor, QAbstractItemModel * model, const QModelIndex &idx) const
{
if (idx.isValid())
{
QLineEdit * lineEdit = editor->findChild<QLineEdit*>();
if (lineEdit)
{
model->setData(idx, lineEdit->text());
}
}
else
{
QStyledItemDelegate::setModelData(editor, model, idx);
}
}

View File

@ -0,0 +1,63 @@
/* path_chooser_delegate.cpp
* Delegate to select a file path for a treeview entry
*
* Wireshark - Network traffic analyzer
* By Gerald Combs <gerald@wireshark.org>
* Copyright 1998 Gerald Combs
*
* SPDX-License-Identifier: GPL-2.0-or-later
*/
#include <ui/qt/models/path_selection_delegate.h>
#include <ui/qt/widgets/path_selection_edit.h>
PathSelectionDelegate::PathSelectionDelegate(QObject *parent)
: QStyledItemDelegate(parent)
{
}
QWidget* PathSelectionDelegate::createEditor(QWidget *parent, const QStyleOptionViewItem &, const QModelIndex &) const
{
PathSelectionEdit * editor = new PathSelectionEdit(tr("Open a pipe"), QString(), true, parent);
connect(editor, &PathSelectionEdit::pathChanged, this, &PathSelectionDelegate::pathHasChanged);
return editor;
}
void PathSelectionDelegate::pathHasChanged(QString)
{
PathSelectionEdit * editor = qobject_cast<PathSelectionEdit *>(sender());
if (editor)
emit commitData(editor);
}
void PathSelectionDelegate::updateEditorGeometry(QWidget *editor, const QStyleOptionViewItem &option, const QModelIndex &) const
{
editor->setGeometry(option.rect);
}
void PathSelectionDelegate::setEditorData(QWidget *editor, const QModelIndex &idx) const
{
if (idx.isValid() && qobject_cast<PathSelectionEdit *>(editor) != nullptr)
{
PathSelectionEdit * edit = qobject_cast<PathSelectionEdit *>(editor);
edit->setPath(idx.data().toString());
}
else
QStyledItemDelegate::setEditorData(editor, idx);
}
void PathSelectionDelegate::setModelData(QWidget *editor, QAbstractItemModel * model, const QModelIndex &idx) const
{
if (idx.isValid() && qobject_cast<PathSelectionEdit *>(editor) != nullptr)
{
PathSelectionEdit * edit = qobject_cast<PathSelectionEdit *>(editor);
model->setData(idx, edit->path());
}
else
{
QStyledItemDelegate::setModelData(editor, model, idx);
}
}

View File

@ -9,26 +9,27 @@
* SPDX-License-Identifier: GPL-2.0-or-later
*/
#ifndef PATH_CHOOSER_DELEGATE_H_
#define PATH_CHOOSER_DELEGATE_H_
#ifndef PATH_SELECTION_DELEGATE_H_
#define PATH_SELECTION_DELEGATE_H_
#include <QStyledItemDelegate>
class PathChooserDelegate : public QStyledItemDelegate
class PathSelectionDelegate : public QStyledItemDelegate
{
Q_OBJECT
public:
PathChooserDelegate(QObject *parent = 0);
PathSelectionDelegate(QObject *parent = 0);
protected:
QWidget *createEditor(QWidget *parent, const QStyleOptionViewItem &option, const QModelIndex &idx) const;
void updateEditorGeometry (QWidget * editor, const QStyleOptionViewItem & option, const QModelIndex & idx) const;
void setEditorData(QWidget *editor, const QModelIndex &idx) const;
void setModelData(QWidget *editor, QAbstractItemModel *model, const QModelIndex &idx) const;
QWidget *createEditor(QWidget *parent, const QStyleOptionViewItem &option, const QModelIndex &idx) const override;
void updateEditorGeometry (QWidget * editor, const QStyleOptionViewItem & option, const QModelIndex & idx) const override;
void setEditorData(QWidget *editor, const QModelIndex &idx) const override;
void setModelData(QWidget *editor, QAbstractItemModel *model, const QModelIndex &idx) const override;
protected slots:
void pathHasChanged(QString newPath);
private slots:
void browseButtonClicked();
};
#endif /* PATH_CHOOSER_DELEGATE_H_ */
#endif /* PATH_SELECTION_DELEGATE_H_ */

View File

@ -24,6 +24,7 @@
#include <ui/qt/widgets/display_filter_edit.h>
#include <ui/qt/widgets/field_filter_edit.h>
#include <ui/qt/widgets/editor_file_dialog.h>
#include <ui/qt/widgets/path_selection_edit.h>
// The Qt docs suggest overriding updateEditorGeometry, but the
// defaults seem sane.
@ -46,31 +47,14 @@ QWidget *UatDelegate::createEditor(QWidget *parent, const QStyleOptionViewItem &
switch (field->mode) {
case PT_TXTMOD_DIRECTORYNAME:
if (index.isValid()) {
QString filename_old = index.model()->data(index, Qt::EditRole).toString();
EditorFileDialog* fileDialog = new EditorFileDialog(index, EditorFileDialog::Directory, parent, QString(field->title), filename_old);
// Use signals to accept data from cell
connect(fileDialog, &EditorFileDialog::acceptEdit, this, &UatDelegate::applyFilename);
// Don't fall through and set setAutoFillBackground(true)
return fileDialog;
}
break;
case PT_TXTMOD_FILENAME:
if (index.isValid()) {
QString filename_old = index.model()->data(index, Qt::EditRole).toString();
EditorFileDialog* fileDialog = new EditorFileDialog(index, EditorFileDialog::ExistingFile, parent, QString(field->title), filename_old);
fileDialog->setOption(QFileDialog::DontConfirmOverwrite);
// Use signals to accept data from cell
connect(fileDialog, &EditorFileDialog::acceptEdit, this, &UatDelegate::applyFilename);
// Don't fall through and set setAutoFillBackground(true)
return fileDialog;
PathSelectionEdit * pathEdit = new PathSelectionEdit(field->title, QString(), field->mode != PT_TXTMOD_DIRECTORYNAME, parent);
connect(pathEdit, &PathSelectionEdit::pathChanged, this, &UatDelegate::pathHasChanged);
return pathEdit;
}
break;
case PT_TXTMOD_COLOR:
if (index.isValid()) {
QColor color(index.model()->data(index, Qt::DecorationRole).toString());
@ -145,6 +129,11 @@ void UatDelegate::setEditorData(QWidget *editor, const QModelIndex &index) const
uat_field_t *field = indexToField(index);
switch (field->mode) {
case PT_TXTMOD_DIRECTORYNAME:
case PT_TXTMOD_FILENAME:
if (index.isValid() && qobject_cast<PathSelectionEdit *>(editor))
qobject_cast<PathSelectionEdit *>(editor)->setPath(index.model()->data(index, Qt::EditRole).toString());
break;
case PT_TXTMOD_ENUM:
{
QComboBox *combobox = static_cast<QComboBox *>(editor);
@ -174,6 +163,11 @@ void UatDelegate::setModelData(QWidget *editor, QAbstractItemModel *model,
uat_field_t *field = indexToField(index);
switch (field->mode) {
case PT_TXTMOD_DIRECTORYNAME:
case PT_TXTMOD_FILENAME:
if (index.isValid() && qobject_cast<PathSelectionEdit *>(editor))
const_cast<QAbstractItemModel *>(index.model())->setData(index, qobject_cast<PathSelectionEdit *>(editor)->path(), Qt::EditRole);
break;
case PT_TXTMOD_ENUM:
{
QComboBox *combobox = static_cast<QComboBox *>(editor);
@ -195,10 +189,25 @@ void UatDelegate::setModelData(QWidget *editor, QAbstractItemModel *model,
}
}
void UatDelegate::applyFilename(const QModelIndex& index)
void UatDelegate::updateEditorGeometry(QWidget *editor,
const QStyleOptionViewItem &option,
const QModelIndex &index) const
{
if (index.isValid()) {
EditorFileDialog* fileDialog = static_cast<EditorFileDialog*>(sender());
const_cast<QAbstractItemModel *>(index.model())->setData(index, fileDialog->text(), Qt::EditRole);
uat_field_t *field = indexToField(index);
switch (field->mode) {
case PT_TXTMOD_DIRECTORYNAME:
case PT_TXTMOD_FILENAME:
editor->setGeometry(option.rect);
break;
default:
QStyledItemDelegate::updateEditorGeometry(editor, option, index);
}
}
void UatDelegate::pathHasChanged(QString)
{
PathSelectionEdit * editor = qobject_cast<PathSelectionEdit *>(sender());
if (editor)
emit commitData(editor);
}

View File

@ -28,14 +28,13 @@ class UatDelegate : public QStyledItemDelegate
public:
UatDelegate(QObject *parent = 0);
QWidget *createEditor(QWidget *parent, const QStyleOptionViewItem &option,
const QModelIndex &index) const;
void setEditorData(QWidget *editor, const QModelIndex &index) const;
void setModelData(QWidget *editor, QAbstractItemModel *model,
const QModelIndex &index) const;
QWidget *createEditor(QWidget *parent, const QStyleOptionViewItem &option, const QModelIndex &index) const override;
void setEditorData(QWidget *editor, const QModelIndex &index) const override;
void setModelData(QWidget *editor, QAbstractItemModel *model, const QModelIndex &index) const override;
void updateEditorGeometry(QWidget *editor, const QStyleOptionViewItem &option, const QModelIndex &index) const override;
private slots:
void applyFilename(const QModelIndex& index);
protected slots:
void pathHasChanged(QString newPath);
private:
uat_field_t *indexToField(const QModelIndex &index) const;

View File

@ -0,0 +1,93 @@
/* path_chooser_delegate.cpp
* Delegate to select a file path for a treeview entry
*
* Wireshark - Network traffic analyzer
* By Gerald Combs <gerald@wireshark.org>
* Copyright 1998 Gerald Combs
*
* SPDX-License-Identifier: GPL-2.0-or-later
*/
#include "config.h"
#include "epan/prefs.h"
#include "ui/last_open_dir.h"
#include <ui/qt/widgets/path_selection_edit.h>
#include "ui/qt/widgets/wireshark_file_dialog.h"
#include <QHBoxLayout>
#include <QToolButton>
#include <QWidget>
#include <QLineEdit>
PathSelectionEdit::PathSelectionEdit(QString title, QString path, bool selectFile, QWidget *parent) :
_title(title),
_path(path),
_selectFile(selectFile),
QWidget(parent)
{
_edit = new QLineEdit(this);
_edit->setText(_path);
connect(_edit, &QLineEdit::textChanged, this, &PathSelectionEdit::setPath);
_button = new QToolButton(this);
_button->setText(tr("Browse"));
connect(_button, &QToolButton::clicked, this, &PathSelectionEdit::browseForPath);
setContentsMargins(0, 0, 0, 0);
QHBoxLayout *hbox = new QHBoxLayout(this);
hbox->setContentsMargins(0, 0, 0, 0);
hbox->addWidget(_edit);
hbox->addWidget(_button);
hbox->setSizeConstraint(QLayout::SetMinimumSize);
setLayout(hbox);
setFocusProxy(_edit);
setFocusPolicy(_edit->focusPolicy());
}
PathSelectionEdit::PathSelectionEdit(QWidget *parent) :
PathSelectionEdit(tr("Select a path"), QString(), true, parent)
{}
void PathSelectionEdit::setPath(QString newPath)
{
_path = newPath;
if (!sender()) {
_edit->blockSignals(true);
_edit->setText(newPath);
_edit->blockSignals(false);
} else {
emit pathChanged(newPath);
}
}
QString PathSelectionEdit::path() const
{
return _path;
}
void PathSelectionEdit::browseForPath()
{
QString openDir = _path;
if (openDir.isEmpty()) {
if (prefs.gui_fileopen_style == FO_STYLE_LAST_OPENED) {
openDir = QString(get_last_open_dir());
} else if (prefs.gui_fileopen_style == FO_STYLE_SPECIFIED) {
openDir = QString(prefs.gui_fileopen_dir);
}
}
QString newPath;
if ( _selectFile )
newPath = WiresharkFileDialog::getOpenFileName(this, _title, openDir);
else
newPath = WiresharkFileDialog::getExistingDirectory(this, _title, openDir);
if (!newPath.isEmpty()) {
_edit->setText(newPath);
}
}

View File

@ -0,0 +1,48 @@
/* path_chooser_delegate.cpp
* Delegate to select a file path for a treeview entry
*
* Wireshark - Network traffic analyzer
* By Gerald Combs <gerald@wireshark.org>
* Copyright 1998 Gerald Combs
*
* SPDX-License-Identifier: GPL-2.0-or-later
*/
#ifndef PATH_SELECTOR_EDIT_H
#define PATH_SELECTOR_EDIT_H
#include <QWidget>
#include <QString>
#include <QLineEdit>
#include <QToolButton>
class PathSelectionEdit : public QWidget
{
Q_OBJECT
public:
PathSelectionEdit(QString title, QString path, bool selectFile, QWidget *parent = 0);
PathSelectionEdit(QWidget *parent = 0);
QString path() const;
public slots:
void setPath(QString newPath = QString());
signals:
void pathChanged(QString newPath);
protected slots:
void browseForPath();
private:
QString _path;
QString _title;
bool _selectFile;
QLineEdit * _edit;
QToolButton * _button;
};
#endif // PATH_SELECTOR_EDIT_H