Commit 571461e4 authored by Scott Wheeler's avatar Scott Wheeler

Finally was able to reproduce this one. Add lots of paranoia to check for

null items before dereferencing them and make sure that item deletion signals
are propogated to various places even when items are being removed in
lists other than the current one.

CCMAIL:66540-done@bugs.kde.org

svn path=/trunk/kdemultimedia/juk/; revision=267363
parent 23356f1c
......@@ -299,7 +299,7 @@ void Playlist::clearItem(PlaylistItem *item, bool emitChanged)
{
emit signalAboutToRemove(item);
m_members.remove(item->absFilePath());
if (!m_randomList.isEmpty() && !m_visibleChanged)
if(!m_randomList.isEmpty() && !m_visibleChanged)
m_randomList.remove(item);
item->deleteLater();
if(emitChanged)
......@@ -346,9 +346,9 @@ PlaylistItemList Playlist::selectedItems()
switch(m_selectedCount) {
case 0:
break;
case 1:
list.append(m_lastSelected);
break;
// case 1:
// list.append(m_lastSelected);
// break;
default:
list = items(QListViewItemIterator::IteratorFlag(QListViewItemIterator::Selected |
QListViewItemIterator::Visible));
......@@ -602,7 +602,6 @@ void Playlist::removeFromDisk(const PlaylistItemList &items)
if(KMessageBox::questionYesNoList(this, message, files) == KMessageBox::Yes) {
for(PlaylistItemList::ConstIterator it = items.begin(); it != items.end(); ++it) {
if(QFile::remove((*it)->filePath())) {
emit signalAboutToRemove(*it);
if(!m_randomList.isEmpty() && !m_visibleChanged)
m_randomList.remove(*it);
CollectionList::instance()->clearItem((*it)->collectionItem());
......
......@@ -680,7 +680,7 @@ ItemType *Playlist::createItem(const QFileInfo &file, const QString &absFilePath
m_randomList.append(i);
emit signalCountChanged(this);
connect(item, SIGNAL(destroyed()), i, SLOT(deleteLater()));
connect(item, SIGNAL(signalAboutToDelete()), i, SLOT(slotClear()));
if(emitChanged)
emit signalCountChanged(this);
......@@ -704,7 +704,7 @@ void Playlist::createItems(const QValueList<SiblingType *> &siblings)
for(; it != siblings.end(); ++it) {
if(!m_members.insert(resolveSymLinks((*it)->absFilePath())) || m_allowDuplicates) {
previous = new ItemType((*it)->collectionItem(), this, previous);
connect((*it)->collectionItem(), SIGNAL(destroyed()), (*it), SLOT(deleteLater()));
connect((*it)->collectionItem(), SIGNAL(signalAboutToDelete()), (*it), SLOT(slotClear()));
}
}
......
......@@ -33,6 +33,7 @@
PlaylistItem::~PlaylistItem()
{
emit signalAboutToDelete();
m_data->deleteUser();
}
......@@ -192,6 +193,12 @@ void PlaylistItem::slotRefreshFromDisk()
slotRefresh();
}
void PlaylistItem::slotClear()
{
// deleteLater();
static_cast<Playlist *>(listView())->clearItem(this);
}
////////////////////////////////////////////////////////////////////////////////
// PlaylistItem protected methods
////////////////////////////////////////////////////////////////////////////////
......
......@@ -122,6 +122,11 @@ public slots:
*/
virtual void slotRefreshFromDisk();
/**
* Asks the item's playlist to remove the item (which uses deleteLater()).
*/
virtual void slotClear();
protected:
/**
* Items should always be created using Playlist::createItem() or through a
......@@ -162,6 +167,7 @@ protected slots:
signals:
void signalRefreshed();
void signalColumnWidthChanged(int column);
void signalAboutToDelete();
private:
void setup(CollectionListItem *item, Playlist *parent);
......
......@@ -39,7 +39,8 @@
// public members
////////////////////////////////////////////////////////////////////////////////
TagEditor::TagEditor(QWidget *parent, const char *name) : QWidget(parent, name)
TagEditor::TagEditor(QWidget *parent, const char *name) : QWidget(parent, name),
m_currentPlaylist(0)
{
setupLayout();
readConfig();
......@@ -58,7 +59,21 @@ TagEditor::~TagEditor()
void TagEditor::slotSetItems(const PlaylistItemList &list)
{
saveChangesPrompt();
if(m_currentPlaylist) {
disconnect(m_currentPlaylist, SIGNAL(signalAboutToRemove(PlaylistItem *)),
this, SLOT(slotItemRemoved(PlaylistItem *)));
}
m_currentPlaylist = list.isEmpty() ? 0 : static_cast<Playlist *>(list.first()->listView());
if(m_currentPlaylist) {
connect(m_currentPlaylist, SIGNAL(signalAboutToRemove(PlaylistItem *)),
this, SLOT(slotItemRemoved(PlaylistItem *)));
}
m_items = list;
if(isVisible())
slotRefresh();
}
......@@ -70,7 +85,7 @@ void TagEditor::slotRefresh()
// the most common case -- is to just process the first item. Then we
// check after that to see if there are other m_items and adjust accordingly.
if(m_items.isEmpty()) {
if(m_items.isEmpty() || !m_items.first()->tag()) {
slotClear();
setEnabled(false);
return;
......@@ -139,47 +154,51 @@ void TagEditor::slotRefresh()
else {
for(; it != m_items.end(); ++it) {
tag = (*it)->tag();
if(m_artistNameBox->currentText() != tag->artist() &&
m_enableBoxes.contains(m_artistNameBox))
{
m_artistNameBox->lineEdit()->clear();
m_enableBoxes[m_artistNameBox]->setChecked(false);
}
if(m_trackNameBox->text() != tag->title() &&
m_enableBoxes.contains(m_trackNameBox))
{
m_trackNameBox->clear();
m_enableBoxes[m_trackNameBox]->setChecked(false);
}
if(m_albumNameBox->currentText() != tag->album() &&
m_enableBoxes.contains(m_albumNameBox))
{
m_albumNameBox->lineEdit()->clear();
m_enableBoxes[m_albumNameBox]->setChecked(false);
}
if(m_genreBox->currentText() != tag->genre() &&
m_enableBoxes.contains(m_genreBox))
{
m_genreBox->lineEdit()->clear();
m_enableBoxes[m_genreBox]->setChecked(false);
}
if(m_trackSpin->value() != tag->track() &&
m_enableBoxes.contains(m_trackSpin))
{
m_trackSpin->setValue(0);
m_enableBoxes[m_trackSpin]->setChecked(false);
}
if(m_yearSpin->value() != tag->year() &&
m_enableBoxes.contains(m_yearSpin))
{
m_yearSpin->setValue(0);
m_enableBoxes[m_yearSpin]->setChecked(false);
}
if(m_commentBox->text() != tag->comment() &&
m_enableBoxes.contains(m_commentBox))
{
m_commentBox->clear();
m_enableBoxes[m_commentBox]->setChecked(false);
if(tag) {
if(m_artistNameBox->currentText() != tag->artist() &&
m_enableBoxes.contains(m_artistNameBox))
{
m_artistNameBox->lineEdit()->clear();
m_enableBoxes[m_artistNameBox]->setChecked(false);
}
if(m_trackNameBox->text() != tag->title() &&
m_enableBoxes.contains(m_trackNameBox))
{
m_trackNameBox->clear();
m_enableBoxes[m_trackNameBox]->setChecked(false);
}
if(m_albumNameBox->currentText() != tag->album() &&
m_enableBoxes.contains(m_albumNameBox))
{
m_albumNameBox->lineEdit()->clear();
m_enableBoxes[m_albumNameBox]->setChecked(false);
}
if(m_genreBox->currentText() != tag->genre() &&
m_enableBoxes.contains(m_genreBox))
{
m_genreBox->lineEdit()->clear();
m_enableBoxes[m_genreBox]->setChecked(false);
}
if(m_trackSpin->value() != tag->track() &&
m_enableBoxes.contains(m_trackSpin))
{
m_trackSpin->setValue(0);
m_enableBoxes[m_trackSpin]->setChecked(false);
}
if(m_yearSpin->value() != tag->year() &&
m_enableBoxes.contains(m_yearSpin))
{
m_yearSpin->setValue(0);
m_enableBoxes[m_yearSpin]->setChecked(false);
}
if(m_commentBox->text() != tag->comment() &&
m_enableBoxes.contains(m_commentBox))
{
m_commentBox->clear();
m_enableBoxes[m_commentBox]->setChecked(false);
}
}
}
}
......@@ -427,7 +446,8 @@ void TagEditor::save(const PlaylistItemList &list)
// If not we'll append it to errorFiles to tell the user which
// files we couldn't write to.
if((newFile.isWritable() || (!newFile.exists() && directory.isWritable())) &&
if(item->tag() &&
(newFile.isWritable() || (!newFile.exists() && directory.isWritable())) &&
item->isWritable())
{
......@@ -495,23 +515,23 @@ void TagEditor::save(const PlaylistItemList &list)
void TagEditor::saveChangesPrompt()
{
if(isVisible() && m_dataChanged && !m_items.isEmpty()) {
QStringList files;
for(PlaylistItemList::Iterator it = m_items.begin(); it != m_items.end(); it++)
files.append((*it)->fileName());
if(KMessageBox::questionYesNoList(this,
i18n("Do you want to save your changes to:\n"),
files,
i18n("Save Changes"),
KStdGuiItem::yes(),
KStdGuiItem::no(),
"tagEditor_showSaveChangesBox") == KMessageBox::Yes)
{
save(m_items);
}
if(!isVisible() || !m_dataChanged || m_items.isEmpty())
return;
QStringList files;
for(PlaylistItemList::Iterator it = m_items.begin(); it != m_items.end(); it++)
files.append((*it)->fileName());
if(KMessageBox::questionYesNoList(this,
i18n("Do you want to save your changes to:\n"),
files,
i18n("Save Changes"),
KStdGuiItem::yes(),
KStdGuiItem::no(),
"tagEditor_showSaveChangesBox") == KMessageBox::Yes)
{
save(m_items);
}
}
......@@ -567,4 +587,11 @@ void TagEditor::slotDataChanged(bool c)
m_dataChanged = c;
}
void TagEditor::slotItemRemoved(PlaylistItem *item)
{
m_items.remove(item);
if(m_items.isEmpty())
slotRefresh();
}
#include "tageditor.moc"
......@@ -30,6 +30,7 @@ class KPushButton;
class QCheckBox;
class QBoxLayout;
class Playlist;
class PlaylistItem;
typedef QValueList<PlaylistItem *> PlaylistItemList;
......@@ -69,6 +70,7 @@ private:
private slots:
void slotDataChanged(bool c = true);
void slotItemRemoved(PlaylistItem *item);
private:
typedef QMap<QWidget *, QCheckBox *> BoxMap;
......@@ -90,6 +92,7 @@ private:
QValueList<QWidget *> m_hideList;
PlaylistItemList m_items;
Playlist *m_currentPlaylist;
bool m_dataChanged;
};
......
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