refactoring the code that handles common UI album cover related actions into a common controller

fixes a bug where cancelling the 'cover from URL' dialog would set the cover to a previously chosen URL

src/CMakeLists.txt

@@ -195,6 +195,7 @@ set(SOURCES
 
   ui/about.cpp
   ui/addstreamdialog.cpp
+  ui/albumcoverchoicecontroller.cpp
   ui/coverfromurldialog.cpp
   ui/edittagdialog.cpp
   ui/equalizer.cpp
@@ -374,6 +375,7 @@ set(HEADERS
 
   ui/about.h
   ui/addstreamdialog.h
+  ui/albumcoverchoicecontroller.h
   ui/coverfromurldialog.h
   ui/edittagdialog.h
   ui/equalizer.h

src/ui/albumcoverchoicecontroller.cpp

@@ -0,0 +1,127 @@
+/* This file is part of Clementine.
+
+   Copyright 2010, David Sansome <me@davidsansome.com>
+
+   Clementine is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   Clementine is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with Clementine.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "core/albumcoverfetcher.h"
+#include "core/albumcoverloader.h"
+#include "ui/albumcoverchoicecontroller.h"
+#include "ui/albumcovermanager.h"
+#include "ui/albumcoversearcher.h"
+#include "ui/coverfromurldialog.h"
+#include "ui/iconloader.h"
+
+#include <QDialog>
+#include <QFileDialog>
+#include <QLabel>
+
+const char* AlbumCoverChoiceController::kImageFileFilter =
+  QT_TR_NOOP("Images (*.png *.jpg *.jpeg *.bmp *.gif *.xpm *.pbm *.pgm *.ppm *.xbm *.tiff)");
+const char* AlbumCoverChoiceController::kAllFilesFilter =
+  QT_TR_NOOP("All files (*)");
+
+AlbumCoverChoiceController::AlbumCoverChoiceController(QWidget* parent)
+  : QWidget(parent),
+#ifdef HAVE_LIBLASTFM
+    cover_searcher_(new AlbumCoverSearcher(QIcon(":/nocover.png"), this)),
+    cover_fetcher_(new AlbumCoverFetcher(this)),
+#endif
+    cover_from_url_dialog_(NULL)
+{
+#ifdef HAVE_LIBLASTFM
+  cover_searcher_->Init(cover_fetcher_);
+#endif
+}
+
+AlbumCoverChoiceController::~AlbumCoverChoiceController()
+{
+  if(cover_from_url_dialog_) {
+    delete cover_from_url_dialog_;
+  }
+}
+
+QString AlbumCoverChoiceController::LoadCoverFromFile(const Song& song) {
+#ifdef HAVE_LIBLASTFM
+  QString dir;
+
+  if (!song.art_automatic().isEmpty() && song.art_automatic() != AlbumCoverLoader::kEmbeddedCover) {
+    dir = song.art_automatic();
+  } else if (!song.filename().isEmpty() && song.filename().contains('/')) {
+    // we get rid of the filename because it's extension is screwing with the dialog's
+    // filters
+    dir = song.filename().section('/', 0, -2);
+  } else {
+    dir = "";
+  }
+
+  QString cover = QFileDialog::getOpenFileName(
+      this, tr("Choose manual cover"), dir,
+      tr(kImageFileFilter) + ";;" + tr(kAllFilesFilter));
+
+  if (cover.isNull())
+    return "";
+
+  // Can we load the image?
+  QImage image(cover);
+  if (image.isNull())
+    return "";
+
+  return cover;
+#else
+  return "";
+#endif
+}
+
+QImage AlbumCoverChoiceController::LoadCoverFromURL() {
+  if(!cover_from_url_dialog_) {
+    cover_from_url_dialog_ = new CoverFromURLDialog(this);
+  }
+
+  QImage image = cover_from_url_dialog_->Exec();
+  return image;
+}
+
+QImage AlbumCoverChoiceController::SearchForCover(const Song& song) const {
+#ifdef HAVE_LIBLASTFM
+  // Get something sensible to stick in the search box
+  QString query = song.artist();
+  if (!query.isEmpty())
+    query += " ";
+  query += song.album();
+
+  QImage image = cover_searcher_->Exec(query);
+  return image;
+#else
+  return QImage();
+#endif
+}
+
+QString AlbumCoverChoiceController::UnsetCover() const {
+  return AlbumCoverLoader::kManuallyUnsetCover;
+}
+
+void AlbumCoverChoiceController::ShowCover(const Song& song) {
+  QDialog* dialog = new QDialog(this);
+  dialog->setAttribute(Qt::WA_DeleteOnClose, true);
+  dialog->setWindowTitle(song.title());
+
+  QLabel* label = new QLabel(dialog);
+  label->setPixmap(AlbumCoverLoader::TryLoadPixmap(
+      song.art_automatic(), song.art_manual(), song.filename()));
+
+  dialog->resize(label->pixmap()->size());
+  dialog->show();
+}

src/ui/albumcoverchoicecontroller.h

@@ -0,0 +1,76 @@
+/* This file is part of Clementine.
+   Copyright 2010, David Sansome <me@davidsansome.com>
+
+   Clementine is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   Clementine is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with Clementine.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#ifndef ALBUMCOVERCHOICECONTROLLER_H
+#define ALBUMCOVERCHOICECONTROLLER_H
+
+#include <QWidget>
+
+class AlbumCoverFetcher;
+class AlbumCoverSearcher;
+class CoverFromURLDialog;
+class Song;
+
+// Controller for the common album cover related menu options. This includes:
+// - loading cover from file
+// - loading cover from URL
+// - searching for cover using last.fm
+// - unsetting the cover manually
+// - showing the cover in original size
+class AlbumCoverChoiceController : public QWidget {
+  Q_OBJECT
+
+ public:
+  AlbumCoverChoiceController(QWidget* parent = 0);
+  ~AlbumCoverChoiceController();
+
+  static const char* kImageFileFilter;
+  static const char* kAllFilesFilter;
+
+  // Some of the methods below require a currently selected song as an
+  // input parameter.
+
+  // Let's the user choose a cover from disk. If no cover will be chosen or the chosen
+  // cover will not be a proper image, this returns an empty string. Otherwise, the
+  // path to the chosen cover will be returned.
+  QString LoadCoverFromFile(const Song& song);
+
+  // Downloads the cover from an URL given by user. This returns the downloaded image
+  // or null image if something went wrong for example when user cancelled the
+  // dialog.
+  QImage LoadCoverFromURL();
+
+  // Lets the user choose a cover among all that have been found on last.fm.
+  // Returns the chosen cover or null cover if user didn't choose anything.
+  QImage SearchForCover(const Song& song) const;
+
+  // Returns a path which indicates that the cover has been unset manually.
+  QString UnsetCover() const;
+
+  // Shows the cover of given song in it's original size.
+  void ShowCover(const Song& song);
+
+private:
+#ifdef HAVE_LIBLASTFM
+  AlbumCoverSearcher* cover_searcher_;
+  AlbumCoverFetcher* cover_fetcher_;
+#endif
+
+  CoverFromURLDialog* cover_from_url_dialog_;
+};
+
+#endif // ALBUMCOVERCHOICECONTROLLER_H

src/ui/albumcovermanager.cpp

@@ -25,7 +25,7 @@
 #include "library/sqlrow.h"
 #include "playlist/songmimedata.h"
 #include "widgets/maclineedit.h"
-#include "ui/coverfromurldialog.h"
+#include "ui/albumcoverchoicecontroller.h"
 
 #include <QActionGroup>
 #include <QContextMenuEvent>
@@ -46,17 +46,13 @@
 #include <QTimer>
 
 const char* AlbumCoverManager::kSettingsGroup = "CoverManager";
-const char* AlbumCoverManager::kImageFileFilter =
-  QT_TR_NOOP("Images (*.png *.jpg *.jpeg *.bmp *.gif *.xpm *.pbm *.pgm *.ppm *.xbm *.tiff)");
-const char* AlbumCoverManager::kAllFilesFilter =
-  QT_TR_NOOP("All files (*)");
 
 AlbumCoverManager::AlbumCoverManager(LibraryBackend* backend, QWidget* parent,
                                      QNetworkAccessManager* network)
   : QMainWindow(parent),
     constructed_(false),
     ui_(new Ui_CoverManager),
-    cover_from_url_dialog_(NULL),
+    album_cover_choice_controller_(new AlbumCoverChoiceController(this)),
     backend_(backend),
     cover_loader_(new BackgroundThreadImplementation<AlbumCoverLoader, AlbumCoverLoader>(this)),
     cover_fetcher_(new AlbumCoverFetcher(this, network)),
@@ -74,13 +70,8 @@ AlbumCoverManager::AlbumCoverManager(LibraryBackend* backend, QWidget* parent,
 
   // Icons
   ui_->action_fetch->setIcon(IconLoader::Load("download"));
-  ui_->action_choose_manual->setIcon(IconLoader::Load("document-open"));
-  ui_->action_choose_url->setIcon(IconLoader::Load("download"));
-  ui_->action_show_fullsize->setIcon(IconLoader::Load("zoom-in"));
-  ui_->action_unset_cover->setIcon(IconLoader::Load("list-remove"));
   ui_->view->setIcon(IconLoader::Load("view-choose"));
   ui_->fetch->setIcon(IconLoader::Load("download"));
-  ui_->action_search_manual->setIcon(IconLoader::Load("find"));
   ui_->action_add_to_playlist->setIcon(IconLoader::Load("media-playback-start"));
   ui_->action_load->setIcon(IconLoader::Load("media-playback-start"));
 
@@ -112,9 +103,6 @@ AlbumCoverManager::~AlbumCoverManager() {
   CancelRequests();
 
   delete ui_;
-  if(cover_from_url_dialog_) {
-    delete cover_from_url_dialog_;
-  }
 }
 
 void AlbumCoverManager::Init() {
@@ -135,11 +123,22 @@ void AlbumCoverManager::Init() {
   ui_->view->setMenu(view_menu);
 
   // Context menu
-  context_menu_->addAction(ui_->action_choose_manual);
+  cover_from_file_ = context_menu_->addAction(
-  context_menu_->addAction(ui_->action_choose_url);
+        IconLoader::Load("document-open"), tr("Load cover from disk..."),
-  context_menu_->addAction(ui_->action_search_manual);
+        this, SLOT(LoadCoverFromFile()));
-  context_menu_->addAction(ui_->action_unset_cover);
+  cover_from_url_ = context_menu_->addAction(
-  context_menu_->addAction(ui_->action_show_fullsize);
+        IconLoader::Load("download"), tr("Load cover from URL..."),
+        this, SLOT(LoadCoverFromURL()));
+  search_for_cover_ = context_menu_->addAction(
+        IconLoader::Load("find"), tr("Search for album covers..."),
+        this, SLOT(SearchForCover()));
+  unset_cover_ = context_menu_->addAction(
+        IconLoader::Load("list-remove"), tr("Unset cover"),
+        this, SLOT(UnsetCover()));
+  show_cover_ = context_menu_->addAction(
+        IconLoader::Load("zoom-in"), tr("Show fullsize..."),
+        this, SLOT(ShowCover()));
+
   context_menu_->addSeparator();
   context_menu_->addAction(ui_->action_load);
   context_menu_->addAction(ui_->action_add_to_playlist);
@@ -154,15 +153,10 @@ void AlbumCoverManager::Init() {
   connect(ui_->fetch, SIGNAL(clicked()), SLOT(FetchAlbumCovers()));
   connect(cover_fetcher_, SIGNAL(AlbumCoverFetched(quint64,QImage)),
           SLOT(AlbumCoverFetched(quint64,QImage)));
-  connect(ui_->action_show_fullsize, SIGNAL(triggered()), SLOT(ShowFullsize()));
   connect(ui_->action_fetch, SIGNAL(triggered()), SLOT(FetchSingleCover()));
-  connect(ui_->action_choose_manual, SIGNAL(triggered()), SLOT(ChooseManualCover()));
-  connect(ui_->action_choose_url, SIGNAL(triggered()), SLOT(ChooseURLCover()));
-  connect(ui_->action_unset_cover, SIGNAL(triggered()), SLOT(UnsetCover()));
   connect(ui_->albums, SIGNAL(doubleClicked(QModelIndex)), SLOT(AlbumDoubleClicked(QModelIndex)));
   connect(ui_->action_add_to_playlist, SIGNAL(triggered()), SLOT(AddSelectedToPlaylist()));
   connect(ui_->action_load, SIGNAL(triggered()), SLOT(LoadSelectedToPlaylist()));
-  connect(ui_->action_search_manual, SIGNAL(triggered()), SLOT(SearchManual()));
 
   #ifdef Q_OS_DARWIN
     MacLineEdit* lineedit = new MacLineEdit(ui_->filter->parentWidget());
@@ -478,10 +472,10 @@ bool AlbumCoverManager::eventFilter(QObject *obj, QEvent *event) {
         some_with_covers = true;
     }
 
-    ui_->action_show_fullsize->setEnabled(some_with_covers && context_menu_items_.size() == 1);
+    cover_from_file_->setEnabled(context_menu_items_.size() == 1);
-    ui_->action_choose_manual->setEnabled(context_menu_items_.size() == 1);
+    cover_from_url_->setEnabled(context_menu_items_.size() == 1);
-    ui_->action_choose_url->setEnabled(context_menu_items_.size() == 1);
+    show_cover_->setEnabled(some_with_covers && context_menu_items_.size() == 1);
-    ui_->action_unset_cover->setEnabled(some_with_covers);
+    unset_cover_->setEnabled(some_with_covers);
 
     QContextMenuEvent* e = static_cast<QContextMenuEvent*>(event);
     context_menu_->popup(e->globalPos());
@@ -490,26 +484,45 @@ bool AlbumCoverManager::eventFilter(QObject *obj, QEvent *event) {
   return QMainWindow::eventFilter(obj, event);
 }
 
-void AlbumCoverManager::ShowFullsize() {
+Song AlbumCoverManager::GetSingleSelectionAsSong() {
-  if (context_menu_items_.size() != 1)
+  return context_menu_items_.size() != 1
-    return;
+             ? Song()
-  QListWidgetItem* item = context_menu_items_[0];
+             : ItemAsSong(context_menu_items_[0]);
+}
+
+Song AlbumCoverManager::GetFirstSelectedAsSong() {
+  return context_menu_items_.isEmpty()
+             ? Song()
+             : ItemAsSong(context_menu_items_[0]);
+}
+
+Song AlbumCoverManager::ItemAsSong(QListWidgetItem* item) {
+  Song result;
 
   QString title = item->data(Role_AlbumName).toString();
   if (!item->data(Role_ArtistName).toString().isNull())
-    title = item->data(Role_ArtistName).toString() + " - " + title;
+    result.set_title(item->data(Role_ArtistName).toString() + " - " + title);
+  else
+    result.set_title(title);
 
-  QDialog* dialog = new QDialog(this);
+  result.set_artist(item->data(Role_ArtistName).toString());
-  dialog->setAttribute(Qt::WA_DeleteOnClose, true);
+  result.set_album(item->data(Role_AlbumName).toString());
-  dialog->setWindowTitle(title);
 
-  QLabel* label = new QLabel(dialog);
+  result.set_filename(item->data(Role_FirstFilename).toString());
-  label->setPixmap(AlbumCoverLoader::TryLoadPixmap(
-      item->data(Role_PathAutomatic).toString(),
-      item->data(Role_PathManual).toString()));
 
-  dialog->resize(label->pixmap()->size());
+  result.set_art_automatic(item->data(Role_PathAutomatic).toString());
-  dialog->show();
+  result.set_art_manual(item->data(Role_PathManual).toString());
+
+  result.set_valid(true);
+  return result;
+}
+
+void AlbumCoverManager::ShowCover() {
+  Song song = GetSingleSelectionAsSong();
+  if(!song.is_valid())
+    return;
+
+  album_cover_choice_controller_->ShowCover(song);
 }
 
 void AlbumCoverManager::FetchSingleCover() {
@@ -525,19 +538,14 @@ void AlbumCoverManager::FetchSingleCover() {
   UpdateStatusText();
 }
 
-void AlbumCoverManager::ChooseManualCover() {
+void AlbumCoverManager::LoadCoverFromFile() {
-  if (context_menu_items_.size() != 1)
+  Song song = GetSingleSelectionAsSong();
+  if(!song.is_valid())
     return;
+
   QListWidgetItem* item = context_menu_items_[0];
 
-  QString dir = InitialPathForOpenCoverDialog(item->data(Role_PathAutomatic).toString(),
+  QString cover = album_cover_choice_controller_->LoadCoverFromFile(song);
-                                              item->data(Role_FirstFilename).toString());
-
-  QString cover = QFileDialog::getOpenFileName(
-      this, tr("Choose manual cover"), dir,
-      tr(kImageFileFilter) + ";;" + tr(kAllFilesFilter));
-  if (cover.isNull())
-    return;
 
   // Can we load the image?
   QImage image(cover);
@@ -545,8 +553,8 @@ void AlbumCoverManager::ChooseManualCover() {
     return;
 
   // Update database
-  backend_->UpdateManualAlbumArtAsync(item->data(Role_ArtistName).toString(),
+  backend_->UpdateManualAlbumArtAsync(song.artist(),
-                                      item->data(Role_AlbumName).toString(),
+                                      song.album(),
                                       cover);
 
   // Update the icon in our list
@@ -555,41 +563,27 @@ void AlbumCoverManager::ChooseManualCover() {
   cover_loading_tasks_[id] = item;
 }
 
-void AlbumCoverManager::ChooseURLCover() {
+void AlbumCoverManager::LoadCoverFromURL() {
-  if (context_menu_items_.size() != 1)
+  Song song = GetSingleSelectionAsSong();
+  if(!song.is_valid())
     return;
-  QListWidgetItem* item = context_menu_items_[0];
 
-  if(!cover_from_url_dialog_) {
+  QImage image = album_cover_choice_controller_->LoadCoverFromURL();
-    cover_from_url_dialog_ = new CoverFromURLDialog(this);
-  }
-
-  QImage image = cover_from_url_dialog_->Exec();
   if (image.isNull())
     return;
 
-  SaveAndSetCover(item, image);
+  SaveAndSetCover(context_menu_items_[0], image);
-}
-
-QString AlbumCoverManager::InitialPathForOpenCoverDialog(const QString& path_automatic, const QString& first_file_name) const {
-  if(!path_automatic.isEmpty() && path_automatic != AlbumCoverLoader::kEmbeddedCover) {
-    return path_automatic;
-  } else if (!first_file_name.isEmpty() && first_file_name.contains('/')) {
-    // we get rid of the filename because its extension is screwing with the dialog's
-    // filters
-    return first_file_name.section('/', 0, -2);
-  } else {
-    return QString();
-  }
 }
 
 void AlbumCoverManager::UnsetCover() {
+  QString unset = album_cover_choice_controller_->UnsetCover();
+
   foreach (QListWidgetItem* item, context_menu_items_) {
     item->setIcon(no_cover_icon_);
-    item->setData(Role_PathManual, AlbumCoverLoader::kManuallyUnsetCover);
+    item->setData(Role_PathManual, unset);
     backend_->UpdateManualAlbumArtAsync(item->data(Role_ArtistName).toString(),
                                         item->data(Role_AlbumName).toString(),
-                                        AlbumCoverLoader::kManuallyUnsetCover);
+                                        unset);
   }
 }
 
@@ -656,17 +650,12 @@ void AlbumCoverManager::LoadSelectedToPlaylist() {
   }
 }
 
-void AlbumCoverManager::SearchManual() {
+void AlbumCoverManager::SearchForCover() {
-  if (context_menu_items_.isEmpty())
+  Song song = GetFirstSelectedAsSong();
+  if(!song.is_valid())
     return;
 
-  // Get something sensible to stick in the search box
+  QImage image = album_cover_choice_controller_->SearchForCover(song);
-  QString query = context_menu_items_[0]->data(Role_ArtistName).toString();
-  if (!query.isEmpty())
-    query += " ";
-  query += context_menu_items_[0]->data(Role_AlbumName).toString();
-
-  QImage image = cover_searcher_->Exec(query);
   if (image.isNull())
     return;
 

src/ui/albumcovermanager.h

@@ -28,9 +28,9 @@
 #include "core/backgroundthread.h"
 #include "core/song.h"
 
+class AlbumCoverChoiceController;
 class AlbumCoverFetcher;
 class AlbumCoverSearcher;
-class CoverFromURLDialog;
 class LibraryBackend;
 class LineEditInterface;
 class SongMimeData;
@@ -49,8 +49,6 @@ class AlbumCoverManager : public QMainWindow {
   ~AlbumCoverManager();
 
   static const char* kSettingsGroup;
-  static const char* kImageFileFilter;
-  static const char* kAllFilesFilter;
 
   LibraryBackend* backend() const { return backend_; }
   QIcon no_cover_icon() const { return no_cover_icon_; }
@@ -85,12 +83,13 @@ class AlbumCoverManager : public QMainWindow {
   void AlbumCoverFetched(quint64 id, const QImage& image);
 
   // On the context menu
-  void ShowFullsize();
   void FetchSingleCover();
-  void SearchManual();
+
-  void ChooseManualCover();
+  void LoadCoverFromFile();
-  void ChooseURLCover();
+  void LoadCoverFromURL();
+  void SearchForCover();
   void UnsetCover();
+  void ShowCover();
 
   // For adding albums to the playlist
   void AlbumDoubleClicked(const QModelIndex& index);
@@ -120,6 +119,17 @@ class AlbumCoverManager : public QMainWindow {
 
   QString InitialPathForOpenCoverDialog(const QString& path_automatic, const QString& first_file_name) const;
 
+  // Returns the selected element in form of a Song ready to be used 
+  // by AlbumCoverChoiceController or invalid song if there's nothing
+  // or multiple elements selected.
+  Song GetSingleSelectionAsSong();
+  // Returns the first of the selected elements in form of a Song ready
+  // to be used by AlbumCoverChoiceController or invalid song if there's nothing
+  // selected.
+  Song GetFirstSelectedAsSong();
+
+  Song ItemAsSong(QListWidgetItem* item);
+
   void CancelRequests();
 
   void UpdateStatusText();
@@ -130,9 +140,16 @@ class AlbumCoverManager : public QMainWindow {
   bool constructed_;
 
   Ui_CoverManager* ui_;
-  CoverFromURLDialog* cover_from_url_dialog_;
+
+  AlbumCoverChoiceController* album_cover_choice_controller_;
   LibraryBackend* backend_;
 
+  QAction* cover_from_file_;
+  QAction* cover_from_url_;
+  QAction* search_for_cover_;
+  QAction* unset_cover_;
+  QAction* show_cover_;
+
   QAction* filter_all_;
   QAction* filter_with_covers_;
   QAction* filter_without_covers_;

src/ui/albumcovermanager.ui

@@ -151,26 +151,11 @@
    </layout>
   </widget>
   <widget class="QStatusBar" name="statusBar"/>
-  <action name="action_show_fullsize">
-   <property name="text">
-    <string>Show fullsize...</string>
-   </property>
-  </action>
   <action name="action_fetch">
    <property name="text">
     <string>Fetch automatically</string>
    </property>
   </action>
-  <action name="action_choose_manual">
-   <property name="text">
-    <string>Load cover from disk...</string>
-   </property>
-  </action>
-  <action name="action_unset_cover">
-   <property name="text">
-    <string>Unset cover</string>
-   </property>
-  </action>
   <action name="action_load">
    <property name="text">
     <string>Load</string>
@@ -181,16 +166,6 @@
     <string>Add to playlist</string>
    </property>
   </action>
-  <action name="action_search_manual">
-   <property name="text">
-    <string>Search for album covers...</string>
-   </property>
-  </action>
-  <action name="action_choose_url">
-   <property name="text">
-    <string>Load cover from URL...</string>
-   </property>
-  </action>
  </widget>
  <customwidgets>
   <customwidget>

src/ui/coverfromurldialog.cpp

@@ -39,6 +39,7 @@ CoverFromURLDialog::~CoverFromURLDialog() {
 }
 
 QImage CoverFromURLDialog::Exec() {
+  last_image_ = QImage();
   exec();
   return last_image_;
 }
@@ -55,8 +56,6 @@ void CoverFromURLDialog::accept() {
 void CoverFromURLDialog::LoadCoverFromURLFinished() {
   ui_->busy->hide();
 
-  last_image_ = QImage();
-
   QNetworkReply* reply = qobject_cast<QNetworkReply*>(sender());
   reply->deleteLater();
 

src/ui/edittagdialog.cpp

@@ -24,15 +24,11 @@
 #include "library/library.h"
 #include "library/librarybackend.h"
 #include "playlist/playlistdelegates.h"
+#include "ui/albumcoverchoicecontroller.h"
 #include "ui/coverfromurldialog.h"
 
-#ifdef HAVE_LIBLASTFM
-# include "albumcoversearcher.h"
-# include "core/albumcoverfetcher.h"
-#endif
-
 #include <QDateTime>
-#include <QFileDialog>
+#include <QDir>
 #include <QFuture>
 #include <QFutureWatcher>
 #include <QLabel>
@@ -50,16 +46,12 @@ const char* EditTagDialog::kTagFetchOnLoadText = QT_TR_NOOP("Generating audio fi
 EditTagDialog::EditTagDialog(QWidget* parent)
   : QDialog(parent),
     ui_(new Ui_EditTagDialog),
-    cover_from_url_dialog_(NULL),
+    album_cover_choice_controller_(new AlbumCoverChoiceController(this)),
     backend_(NULL),
     loading_(false),
     ignore_edits_(false),
 #ifdef HAVE_LIBTUNEPIMP
     tag_fetcher_(new TagFetcher()),
-#endif
-#ifdef HAVE_LIBLASTFM
-    cover_searcher_(new AlbumCoverSearcher(QIcon(":/nocover.png"), this)),
-    cover_fetcher_(new AlbumCoverFetcher(this)),
 #endif
     cover_loader_(new BackgroundThreadImplementation<AlbumCoverLoader, AlbumCoverLoader>(this)),
     cover_art_id_(0),
@@ -76,10 +68,6 @@ EditTagDialog::EditTagDialog(QWidget* parent)
   connect(resultsDialog_, SIGNAL(SongChoosen(QString, Song)), SLOT(FetchTagSongChoosen(QString, Song)));
 #endif
 
-#ifdef HAVE_LIBLASTFM
-  cover_searcher_->Init(cover_fetcher_);
-#endif
-
   ui_->setupUi(this);
   ui_->splitter->setSizes(QList<int>() << 200 << width() - 200);
   ui_->loading_container->hide();
@@ -144,21 +132,21 @@ EditTagDialog::EditTagDialog(QWidget* parent)
 
   // Set up the album cover menu
   cover_menu_ = new QMenu(this);
-  choose_cover_ = cover_menu_->addAction(
+  cover_from_file_ = cover_menu_->addAction(
         IconLoader::Load("document-open"), tr("Load cover from disk..."),
         this, SLOT(LoadCoverFromFile()));
-  download_cover_ = cover_menu_->addAction(
+  cover_from_url_ = cover_menu_->addAction(
         IconLoader::Load("download"), tr("Load cover from URL..."),
         this, SLOT(LoadCoverFromURL()));
   search_for_cover_ = cover_menu_->addAction(
         IconLoader::Load("find"), tr("Search for album covers..."),
-        this, SLOT(SearchCover()));
+        this, SLOT(SearchForCover()));
   unset_cover_ = cover_menu_->addAction(
         IconLoader::Load("list-remove"), tr("Unset cover"),
         this, SLOT(UnsetCover()));
   show_cover_ = cover_menu_->addAction(
         IconLoader::Load("zoom-in"), tr("Show fullsize..."),
-        this, SLOT(ZoomCover()));
+        this, SLOT(ShowCover()));
   ui_->summary_art_button->setMenu(cover_menu_);
 
   ui_->art->installEventFilter(this);
@@ -184,9 +172,6 @@ EditTagDialog::~EditTagDialog() {
   delete tag_fetcher_;
 # endif
   delete ui_;
-  if(cover_from_url_dialog_) {
-    delete cover_from_url_dialog_;
-  }
 }
 
 bool EditTagDialog::SetLoading(const QString& message) {
@@ -431,7 +416,7 @@ void EditTagDialog::UpdateSummaryTab(const Song& song) {
   ui_->summary->setText(summary);
 
 #ifndef HAVE_LIBLASTFM
-  choose_cover_->setEnabled(false);
+  cover_from_file_->setEnabled(false);
   search_for_cover_->setEnabled(false);
 #endif
 
@@ -506,97 +491,63 @@ void EditTagDialog::ResetField() {
   }
 }
 
-void EditTagDialog::LoadCoverFromFile() {
+Song EditTagDialog::GetFirstSelected() {
-#ifdef HAVE_LIBLASTFM
   const QModelIndexList sel = ui_->song_list->selectionModel()->selectedIndexes();
   if (sel.isEmpty())
-    return;
+    return Song();
-  const Song& song = data_[sel.first().row()].original_;
+  return data_[sel.first().row()].original_;
+}
 
-  // Figure out the initial path.  Logic copied from
+void EditTagDialog::LoadCoverFromFile() {
-  // AlbumCoverManager::InitialPathForOpenCoverDialog
+  Song song = GetFirstSelected();
-  QString dir;
+  if(!song.is_valid()) {
-  if (!song.art_automatic().isEmpty() && song.art_automatic() != AlbumCoverLoader::kEmbeddedCover) {
+    return;
-    dir = song.art_automatic();
-  } else {
-    dir = song.filename().section('/', 0, -1);
   }
 
-  QString cover = QFileDialog::getOpenFileName(
+  QString cover = album_cover_choice_controller_->LoadCoverFromFile(song);
-      this, tr("Choose manual cover"), dir,
-      tr(AlbumCoverManager::kImageFileFilter) + ";;" + tr(AlbumCoverManager::kAllFilesFilter));
-  if (cover.isNull())
-    return;
 
-  // Can we load the image?
+  if(!cover.isEmpty()) {
-  QImage image(cover);
+    SetAlbumArt(cover);
-  if (image.isNull())
+  }
-    return;
-
-  // Update database
-  SetAlbumArt(cover);
-#endif
 }
 
 void EditTagDialog::LoadCoverFromURL() {
-  // TODO: duplication
+  Song song = GetFirstSelected();
-  const QModelIndexList sel = ui_->song_list->selectionModel()->selectedIndexes();
+  if(!song.is_valid()) {
-  if (sel.isEmpty())
     return;
-  const Song& song = data_[sel.first().row()].original_;
-
-  if(!cover_from_url_dialog_) {
-    cover_from_url_dialog_ = new CoverFromURLDialog(this);
   }
 
-  QImage image = cover_from_url_dialog_->Exec();
+  QImage image = album_cover_choice_controller_->LoadCoverFromURL();
   if (image.isNull())
     return;
 
   SetAlbumArt(AlbumCoverManager::SaveCoverInCache(song.artist(), song.album(), image));
 }
 
-void EditTagDialog::SearchCover() {
+void EditTagDialog::SearchForCover() {
-#ifdef HAVE_LIBLASTFM
+  Song song = GetFirstSelected();
-  const QModelIndexList sel = ui_->song_list->selectionModel()->selectedIndexes();
+  if(!song.is_valid()) {
-  if (sel.isEmpty())
     return;
-  const Song& song = data_[sel.first().row()].original_;
+  }
 
-  // Get something sensible to stick in the search box
+  QImage image = album_cover_choice_controller_->SearchForCover(song);
-  QString query = song.artist();
-  if (!query.isEmpty())
-    query += " ";
-  query += song.album();
-
-  QImage image = cover_searcher_->Exec(query);
   if (image.isNull())
     return;
 
   SetAlbumArt(AlbumCoverManager::SaveCoverInCache(song.artist(), song.album(), image));
-#endif
 }
 
 void EditTagDialog::UnsetCover() {
-  SetAlbumArt(AlbumCoverLoader::kManuallyUnsetCover);
+  SetAlbumArt(album_cover_choice_controller_->UnsetCover());
 }
 
-void EditTagDialog::ZoomCover() {
+void EditTagDialog::ShowCover() {
-  const QModelIndexList sel = ui_->song_list->selectionModel()->selectedIndexes();
+  Song song = GetFirstSelected();
-  if (sel.isEmpty())
+  if(!song.is_valid()) {
     return;
-  const Song& song = data_[sel.first().row()].original_;
+  }
 
-  QDialog* dialog = new QDialog(this);
+  album_cover_choice_controller_->ShowCover(song);
-  dialog->setAttribute(Qt::WA_DeleteOnClose, true);
-  dialog->setWindowTitle(song.title());
-
-  QLabel* label = new QLabel(dialog);
-  label->setPixmap(AlbumCoverLoader::TryLoadPixmap(
-      song.art_automatic(), song.art_manual(), song.filename()));
-
-  dialog->resize(label->pixmap()->size());
-  dialog->show();
 }
 
 void EditTagDialog::SetAlbumArt(const QString& path) {

src/ui/edittagdialog.h

@@ -31,13 +31,8 @@
 #include "widgets/lineedit.h"
 #include "trackselectiondialog.h"
 
-#ifdef HAVE_LIBLASTFM
+class AlbumCoverChoiceController;
-  class AlbumCoverFetcher;
-  class AlbumCoverSearcher;
-#endif
-
 class AlbumCoverLoader;
-class CoverFromURLDialog;
 class LibraryBackend;
 class Ui_EditTagDialog;
 
@@ -89,9 +84,9 @@ private slots:
 
   void LoadCoverFromFile();
   void LoadCoverFromURL();
-  void SearchCover();
+  void SearchForCover();
   void UnsetCover();
-  void ZoomCover();
+  void ShowCover();
 
   void PreviousSong();
   void NextSong();
@@ -120,6 +115,8 @@ private:
     QString id_;
   };
 
+  Song GetFirstSelected();
+
   bool DoesValueVary(const QModelIndexList& sel, const QString& id) const;
   bool IsValueModified(const QModelIndexList& sel, const QString& id) const;
 
@@ -140,7 +137,8 @@ private:
 
 private:
   Ui_EditTagDialog* ui_;
-  CoverFromURLDialog* cover_from_url_dialog_;
+
+  AlbumCoverChoiceController* album_cover_choice_controller_;
 
   LibraryBackend* backend_;
 
@@ -156,18 +154,14 @@ private:
   TagFetcher* tag_fetcher_;
 #endif
 
-#ifdef HAVE_LIBLASTFM
-  AlbumCoverSearcher* cover_searcher_;
-  AlbumCoverFetcher* cover_fetcher_;
-#endif
-
   BackgroundThread<AlbumCoverLoader>* cover_loader_;
   quint64 cover_art_id_;
   bool cover_art_is_set_;
 
   QMenu* cover_menu_;
-  QAction* choose_cover_;
+
-  QAction* download_cover_;
+  QAction* cover_from_file_;
+  QAction* cover_from_url_;
   QAction* search_for_cover_;
   QAction* unset_cover_;
   QAction* show_cover_;

src/widgets/nowplayingwidget.cpp

@@ -19,7 +19,7 @@
 #include "core/albumcoverloader.h"
 #include "core/kittenloader.h"
 #include "library/librarybackend.h"
-#include "ui/coverfromurldialog.h"
+#include "ui/albumcoverchoicecontroller.h"
 #include "ui/iconloader.h"
 
 #ifdef HAVE_LIBLASTFM
@@ -28,8 +28,6 @@
 # include "ui/albumcoversearcher.h"
 #endif
 
-#include <QFileDialog>
-#include <QLabel>
 #include <QMenu>
 #include <QMovie>
 #include <QPainter>
@@ -63,13 +61,9 @@ const int NowPlayingWidget::kTopBorder = 4;
 
 NowPlayingWidget::NowPlayingWidget(QWidget *parent)
   : QWidget(parent),
-    cover_from_url_dialog_(NULL),
+    album_cover_choice_controller_(new AlbumCoverChoiceController(this)),
     cover_loader_(new BackgroundThreadImplementation<AlbumCoverLoader, AlbumCoverLoader>(this)),
     kitten_loader_(NULL),
-#ifdef HAVE_LIBLASTFM
-    cover_searcher_(new AlbumCoverSearcher(QIcon(":/nocover.png"), this)),
-    cover_fetcher_(new AlbumCoverFetcher(this)),
-#endif
     backend_(NULL),
     mode_(SmallSongDetails),
     menu_(new QMenu(this)),
@@ -99,21 +93,23 @@ NowPlayingWidget::NowPlayingWidget(QWidget *parent)
 
   menu_->addActions(mode_group->actions());
   menu_->addSeparator();
-  choose_cover_ = menu_->addAction(
+
+  cover_from_file_ = menu_->addAction(
         IconLoader::Load("document-open"), tr("Load cover from disk..."),
         this, SLOT(LoadCoverFromFile()));
-  download_cover_ = menu_->addAction(
+  cover_from_url_ = menu_->addAction(
         IconLoader::Load("download"), tr("Load cover from URL..."),
         this, SLOT(LoadCoverFromURL()));
   search_for_cover_ = menu_->addAction(
         IconLoader::Load("find"), tr("Search for album covers..."),
-        this, SLOT(SearchCover()));
+        this, SLOT(SearchForCover()));
   unset_cover_ = menu_->addAction(
         IconLoader::Load("list-remove"), tr("Unset cover"),
         this, SLOT(UnsetCover()));
   show_cover_ = menu_->addAction(
         IconLoader::Load("zoom-in"), tr("Show fullsize..."),
-        this, SLOT(ZoomCover()));
+        this, SLOT(ShowCover()));
+
   menu_->addSeparator();
   above_statusbar_action_ = menu_->addAction(tr("Show above status bar"));
   above_statusbar_action_->setCheckable(true);
@@ -130,16 +126,6 @@ NowPlayingWidget::NowPlayingWidget(QWidget *parent)
   // Start loading the cover loader thread
   cover_loader_->Start();
   connect(cover_loader_, SIGNAL(Initialised()), SLOT(CoverLoaderInitialised()));
-
-#ifdef HAVE_LIBLASTFM
-  cover_searcher_->Init(cover_fetcher_);
-#endif
-}
-
-NowPlayingWidget::~NowPlayingWidget() {
-  if(cover_from_url_dialog_) {
-    delete cover_from_url_dialog_;
-  }
 }
 
 void NowPlayingWidget::CreateModeAction(Mode mode, const QString &text, QActionGroup *group, QSignalMapper* mapper) {
@@ -155,7 +141,7 @@ void NowPlayingWidget::CreateModeAction(Mode mode, const QString &text, QActionG
 void NowPlayingWidget::set_ideal_height(int height) {
   small_ideal_height_ = height;
   UpdateHeight(aww_
-      ?  kitten_loader_->Worker().get()
+      ? kitten_loader_->Worker().get()
       : cover_loader_->Worker().get());
 }
 
@@ -217,7 +203,7 @@ void NowPlayingWidget::NowPlaying(const Song& metadata) {
 
   // Loads the cover too.
   UpdateHeight(aww_
-      ?  kitten_loader_->Worker().get()
+      ? kitten_loader_->Worker().get()
       : cover_loader_->Worker().get());
   UpdateDetailsText();
 
@@ -359,7 +345,7 @@ void NowPlayingWidget::FadePreviousTrack(qreal value) {
 void NowPlayingWidget::SetMode(int mode) {
   mode_ = Mode(mode);
   UpdateHeight(aww_
-      ?  kitten_loader_->Worker().get()
+      ? kitten_loader_->Worker().get()
       : cover_loader_->Worker().get());
   UpdateDetailsText();
   update();
@@ -372,7 +358,7 @@ void NowPlayingWidget::SetMode(int mode) {
 void NowPlayingWidget::resizeEvent(QResizeEvent* e) {
   if (visible_ && mode_ == LargeSongDetails && e->oldSize().width() != e->size().width()) {
     UpdateHeight(aww_
-        ?  kitten_loader_->Worker().get()
+        ? kitten_loader_->Worker().get()
         : cover_loader_->Worker().get());
     UpdateDetailsText();
   }
@@ -380,7 +366,7 @@ void NowPlayingWidget::resizeEvent(QResizeEvent* e) {
 
 void NowPlayingWidget::contextMenuEvent(QContextMenuEvent* e) {
 #ifndef HAVE_LIBLASTFM
-  choose_cover_->setEnabled(false);
+  cover_from_file_->setEnabled(false);
   search_for_cover_->setEnabled(false);
 #endif
 
@@ -432,38 +418,15 @@ void NowPlayingWidget::EnableKittens(bool aww) {
 }
 
 void NowPlayingWidget::LoadCoverFromFile() {
-#ifdef HAVE_LIBLASTFM
+  QString cover = album_cover_choice_controller_->LoadCoverFromFile(metadata_);
-  // Figure out the initial path.  Logic copied from
+
-  // AlbumCoverManager::InitialPathForOpenCoverDialog
+  if(!cover.isEmpty()) {
-  QString dir;
+    SetAlbumArt(cover);
-  if (!metadata_.art_automatic().isEmpty() && metadata_.art_automatic() != AlbumCoverLoader::kEmbeddedCover) {
-    dir = metadata_.art_automatic();
-  } else {
-    dir = metadata_.filename().section('/', 0, -1);
   }
-
-  QString cover = QFileDialog::getOpenFileName(
-      this, tr("Choose manual cover"), dir,
-      tr(AlbumCoverManager::kImageFileFilter) + ";;" + tr(AlbumCoverManager::kAllFilesFilter));
-  if (cover.isNull())
-    return;
-
-  // Can we load the image?
-  QImage image(cover);
-  if (image.isNull())
-    return;
-
-  // Update database
-  SetAlbumArt(cover);
-#endif
 }
 
 void NowPlayingWidget::LoadCoverFromURL() {
-  if(!cover_from_url_dialog_) {
+  QImage image = album_cover_choice_controller_->LoadCoverFromURL();
-    cover_from_url_dialog_ = new CoverFromURLDialog(this);
-  }
-
-  QImage image = cover_from_url_dialog_->Exec();
   if (image.isNull())
     return;
 
@@ -471,38 +434,21 @@ void NowPlayingWidget::LoadCoverFromURL() {
       metadata_.artist(), metadata_.album(), image));
 }
 
-void NowPlayingWidget::SearchCover() {
+void NowPlayingWidget::SearchForCover() {
-#ifdef HAVE_LIBLASTFM
+  QImage image = album_cover_choice_controller_->SearchForCover(metadata_);
-  // Get something sensible to stick in the search box
-  QString query = metadata_.artist();
-  if (!query.isEmpty())
-    query += " ";
-  query += metadata_.album();
-
-  QImage image = cover_searcher_->Exec(query);
   if (image.isNull())
     return;
 
   SetAlbumArt(AlbumCoverManager::SaveCoverInCache(
       metadata_.artist(), metadata_.album(), image));
-#endif
 }
 
 void NowPlayingWidget::UnsetCover() {
-  SetAlbumArt(AlbumCoverLoader::kManuallyUnsetCover);
+  SetAlbumArt(album_cover_choice_controller_->UnsetCover());
 }
 
-void NowPlayingWidget::ZoomCover() {
+void NowPlayingWidget::ShowCover() {
-  QDialog* dialog = new QDialog(this);
+  album_cover_choice_controller_->ShowCover(metadata_);
-  dialog->setAttribute(Qt::WA_DeleteOnClose, true);
-  dialog->setWindowTitle(metadata_.title());
-
-  QLabel* label = new QLabel(dialog);
-  label->setPixmap(AlbumCoverLoader::TryLoadPixmap(
-      metadata_.art_automatic(), metadata_.art_manual(), metadata_.filename()));
-
-  dialog->resize(label->pixmap()->size());
-  dialog->show();
 }
 
 void NowPlayingWidget::SetAlbumArt(const QString& path) {

src/widgets/nowplayingwidget.h

@@ -28,10 +28,11 @@
   class AlbumCoverSearcher;
 #endif
 
+class AlbumCoverChoiceController;
 class AlbumCoverLoader;
-class CoverFromURLDialog;
 class LibraryBackend;
 
+class QAction;
 class QActionGroup;
 class QMenu;
 class QMovie;
@@ -44,7 +45,6 @@ class NowPlayingWidget : public QWidget {
 
 public:
   NowPlayingWidget(QWidget* parent = 0);
-  ~NowPlayingWidget();
 
   static const char* kSettingsGroup;
   static const int kPadding;
@@ -95,9 +95,9 @@ private slots:
 
   void LoadCoverFromFile();
   void LoadCoverFromURL();
-  void SearchCover();
+  void SearchForCover();
   void UnsetCover();
-  void ZoomCover();
+  void ShowCover();
 
 private:
   void CreateModeAction(Mode mode, const QString& text, QActionGroup* group,
@@ -109,24 +109,21 @@ private:
   void SetAlbumArt(const QString& path);
 
 private:
-  CoverFromURLDialog* cover_from_url_dialog_;
+  AlbumCoverChoiceController* album_cover_choice_controller_;
 
   BackgroundThread<AlbumCoverLoader>* cover_loader_;
   BackgroundThread<AlbumCoverLoader>* kitten_loader_;
 
-#ifdef HAVE_LIBLASTFM
-  AlbumCoverSearcher* cover_searcher_;
-  AlbumCoverFetcher* cover_fetcher_;
-#endif
-
   LibraryBackend* backend_;
 
   Mode mode_;
 
   QMenu* menu_;
+
   QAction* above_statusbar_action_;
-  QAction* choose_cover_;
+
-  QAction* download_cover_;
+  QAction* cover_from_file_;
+  QAction* cover_from_url_;
   QAction* search_for_cover_;
   QAction* unset_cover_;
   QAction* show_cover_;