Commit a800c1b3 authored by Michael Pyne's avatar Michael Pyne
Browse files

playlist: Don't rely on overridden column count during construction.

The LGTM code scanning site has coverage of JuK by virtue of its Github
mirror, and reported [1] that `Playlist` was calling a function that was
indirectly reliant on a virtual function (`columnOffset()`) that is
overridden by subclasses like `HistoryPlaylist`.

This is a bug since the C++ language specifies that a superclass
constructor will be statically bound to call its own version of a
virtual function (or an even higher superclass's), rather than whatever
the subclass reimplementation might ultimately be.

This is fixed by using the data we already have at the time of
construction to set aside the right number of columns and then just
holding onto the header labels generated rather than trying to
introspect later.

[1] https://lgtm.com/projects/g/KDE/juk?mode=list
parent 64b95b30
......@@ -124,13 +124,7 @@ Playlist::Playlist(
, m_playlistName(name)
, m_fetcher(new WebImageFetcher(this))
{
// Any added columns must precede normal ones, which are normally added
// in setup()
for(int i = 0; i < extraCols; ++i) {
addColumn(i18n("JuK")); // Placeholder text
}
setup();
setup(extraCols);
// Some subclasses need to do even more handling but will remember to
// call setupPlaylist
......@@ -1010,12 +1004,6 @@ void Playlist::takeItem(QTreeWidgetItem *item)
QTreeWidget::takeTopLevelItem(index);
}
void Playlist::addColumn(const QString &label, int)
{
m_columns.append(label);
setHeaderLabels(m_columns);
}
PlaylistItem *Playlist::createItem(const FileHandle &file, QTreeWidgetItem *after)
{
return createItem<PlaylistItem>(file, after);
......@@ -1199,26 +1187,46 @@ void Playlist::sortByColumn(int column, Qt::SortOrder order)
QTreeWidget::sortByColumn(column, order);
}
void Playlist::slotInitialize()
{
addColumn(i18n("Track Name"));
addColumn(i18n("Artist"));
addColumn(i18n("Album"));
addColumn(i18n("Cover"));
addColumn(i18nc("cd track number", "Track"));
addColumn(i18n("Genre"));
addColumn(i18n("Year"));
addColumn(i18n("Length"));
addColumn(i18n("Bitrate"));
addColumn(i18n("Comment"));
addColumn(i18n("File Name"));
addColumn(i18n("File Name (full path)"));
// This function is called during startup so it cannot rely on any virtual
// functions that might be changed by a subclass (virtual functions relying on
// superclasses are fine since the C++ runtime can statically dispatch those).
void Playlist::slotInitialize(int numColumnsToReserve)
{
// Setup the columns in the list view. We set aside room for
// subclass-specific extra columns (always added at the beginning, see
// columnOffset()) and then supplement with columns that apply to every
// playlist.
const QStringList standardColHeaders = {
i18n("Track Name"),
i18n("Artist"),
i18n("Album"),
i18n("Cover"),
i18nc("cd track number", "Track"),
i18n("Genre"),
i18n("Year"),
i18n("Length"),
i18n("Bitrate"),
i18n("Comment"),
i18n("File Name"),
i18n("File Name (full path)"),
};
QStringList allColHeaders;
allColHeaders.reserve(numColumnsToReserve + standardColHeaders.size());
std::fill_n(allColHeaders.begin(), numColumnsToReserve, i18n("JuK"));
std::copy (standardColHeaders.cbegin(), standardColHeaders.cend(),
std::back_inserter(allColHeaders));
setHeaderLabels(allColHeaders);
setAllColumnsShowFocus(true);
setSelectionMode(QTreeWidget::ExtendedSelection);
header()->setSortIndicatorShown(true);
m_columnFixedWidths.resize(columnCount());
int numColumns = columnCount();
m_columnFixedWidths.resize(numColumns);
m_weightDirty.resize(numColumns);
m_columnWeights.resize(numColumns);
//////////////////////////////////////////////////
// setup header RMB menu
......@@ -1227,11 +1235,8 @@ void Playlist::slotInitialize()
QAction *showAction;
const auto sharedSettings = SharedSettings::instance();
for(int i = 0; i < header()->count(); ++i) {
if(i - columnOffset() == PlaylistItem::FileNameColumn)
m_headerMenu->addSeparator();
showAction = new QAction(headerItem()->text(i), m_headerMenu);
for(int i = 0; i < numColumns; ++i) {
showAction = new QAction(allColHeaders[i], m_headerMenu);
showAction->setData(i);
showAction->setCheckable(true);
showAction->setChecked(sharedSettings->isColumnVisible(i));
......@@ -1270,6 +1275,9 @@ void Playlist::slotInitialize()
setDropIndicatorShown(true);
setDragEnabled(true);
// TODO: Retain last-run's sort
sortByColumn(1, Qt::AscendingOrder);
m_disableColumnWidthUpdates = false;
}
......@@ -1354,7 +1362,7 @@ void Playlist::slotPlayFromBackMenu(QAction *backAction) const
// private members
////////////////////////////////////////////////////////////////////////////////
void Playlist::setup()
void Playlist::setup(int numColumnsToReserve)
{
m_search = new PlaylistSearch(this);
......@@ -1372,8 +1380,6 @@ void Playlist::setup()
// progress.
connect(this, SIGNAL(itemSelectionChanged()), m_fetcher, SLOT(abortSearch()));
sortByColumn(1, Qt::AscendingOrder);
connect(this, &QTreeWidget::itemDoubleClicked, this, &Playlist::slotPlayCurrent);
// Use a timer to soak up the multiple dataChanged signals we're going to get
......@@ -1398,7 +1404,7 @@ void Playlist::setup()
// Explicitly call slotInitialize() so that the columns are added before
// SharedSettings::apply() sets the visible and hidden ones.
slotInitialize();
slotInitialize(numColumnsToReserve);
}
void Playlist::loadFile(const QString &fileName, const QFileInfo &fileInfo)
......
......@@ -440,8 +440,6 @@ protected:
virtual bool hasItem(const QString &file) const { return m_members.contains(file); }
virtual void addColumn(const QString &label, int width = -1);
/**
* Do some final initialization of created items. Notably ensure that they
* are shown or hidden based on the contents of the current PlaylistSearch.
......@@ -499,7 +497,7 @@ private:
PlaylistCollection *collection, const QString &iconName,
int extraCols);
void setup();
void setup(int numColumnsToReserve);
/**
* This function is called to let the user know that JuK has automatically enabled
......@@ -591,7 +589,7 @@ private slots:
* Used to be a subclass of K3ListView::polish() but the timing of the
* call is not consistent and therefore lead to crashes.
*/
void slotInitialize();
void slotInitialize(int numColumnsToReserve);
void slotUpdateColumnWidths();
......@@ -668,7 +666,6 @@ private:
/**
* The average minimum widths of columns to be used in balancing calculations.
*/
QStringList m_columns;
QVector<int> m_columnWeights;
QVector<int> m_columnFixedWidths;
QVector<int> m_weightDirty;
......
......@@ -89,9 +89,6 @@ signals:
// Minimizing/closing the JuK window will not trigger this signal.
void signalShown(bool shown);
private:
QStringList m_columnHeaders;
};
#endif
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment