Version: 2.3 (using KDE KDE 3.5.0) Installed from: SuSE RPMs Compiler: One supplied with SuSE 10.0 OSS OS: Linux While attempting to add a shortcut for "Add to Playlist" under Settings - Configure Shortcuts, the Action List presented included "Add to Audio or Data CD" 39 times in a row. I shut down JuK and restarted with the same result. I rebooted and restarted JuK, again with the same result. I am running SuSE 10.0 OSS, up to date as of this date. I have a DEll Dimension 4200, with 1 1Gb Pentium III, 384 MB of RAM. SuSE is running in a 15 GB partition, with /home/ running in its own 2.6 GB partition.
Can you confirm that you don't have this with other application as well? If you do not, can you attach (if present) ~/.kde/share/apps/juk/jukui.rc and ~/.kde/share/config/jukrc to the bug report?
I can confirm this bug. It is because every single Playlist duplicates the action defined by K3bExporter, so every playlist you right-click in will add the K3b action. I tried simply making the action static in K3bExporter::action() but that makes the feature only work for Collection List. I will try to put in more thought as to how to fix things. Note: You will have a minimum of two "Add to Audio or Data CD" actions, because one of those is for the PlaylistBox (which adds the whole playlist, not just the selected items). The text should probably be more clear about the difference.
Created attachment 14635 [details] Fix for duplicate KActions being created. This patch is ugly IMHO (adding a static instance method to PlaylistCollection, and introducing a special KAction subclass in k3bexporter.cpp). What it does is makes the KAction returned by K3bExporter::action() always the same action. Each instance of K3bExporter will add itself to a mapping maintained by the new PlaylistAction. When the action is activated, it finds the visible playlist, finds out what K3bExporter belongs to it, and calls the requested slot. I tried to make PlaylistAction more generic than necessary in case we decide to actually use KAction more in the playlist context menus. If this patch is too ugly, I could always just add a manual slot to the Playlist context menu and point it right to K3bExporter but we already don't use enough KAction IMO (i.e. you can't make a shortcut for "Create Playlist From Selected Items"). It would be more consistent though.
SVN commit 510357 by mpyne: Fix bug 121621 (Action List in Configure Shortcuts includes up to as many Add to CD items as you have playlists). I'm still not in love with the patch so the underlying code may change a bit between here and there however. Mailing Carsten for changelog update please. :) BUG:121621 CCMAIL:cniehaus@gmx.de M +74 -8 k3bexporter.cpp M +5 -0 k3bexporter.h M +8 -0 playlistcollection.cpp M +4 -0 playlistcollection.h --- branches/KDE/3.5/kdemultimedia/juk/k3bexporter.cpp #510356:510357 @@ -24,6 +24,7 @@ #include <kapplication.h> #include <qcstring.h> +#include <qmap.h> #include <dcopref.h> #include <dcopclient.h> @@ -36,25 +37,90 @@ using ActionCollection::actions; +// static member variable definition + +PlaylistAction *K3bExporter::m_action = 0; + +// Special KAction subclass used to automatically call a slot when activated, +// depending on the visible playlist at the time. In other words, use *one* +// instance of this action for many playlists. +// +// This is used to handle some actions in the Playlist context menu. +class PlaylistAction : public KAction +{ + public: + PlaylistAction(const char *name, + const QString &userText, + const QIconSet &pix, + const char *slot, + const KShortcut &cut = 0) : + KAction(userText, pix, cut, 0 /* receiver */, 0 /* slot */, actions(), name), + m_slot(slot) + { + } + + typedef QMap<const Playlist *, QObject *> PlaylistRecipientMap; + + /** + * Defines a QObject to call (using the m_slot SLOT) when an action is + * emitted from a Playlist. + */ + void addCallMapping(const Playlist *p, QObject *obj) + { + m_playlistRecipient[p] = obj; + } + + protected slots: + void slotActivated() + { + kdDebug(65432) << k_funcinfo << endl; + + // Determine current playlist, and call its slot. + Playlist *p = PlaylistCollection::instance()->visiblePlaylist(); + if(!p) + return; + + // Make sure we're supposed to notify someone about this playlist. + QObject *recipient = m_playlistRecipient[p]; + if(!recipient) + return; + + // Invoke the slot using some trickery. + // XXX: Use the QMetaObject to do this in Qt 4. + connect(this, SIGNAL(activated()), recipient, m_slot); + emit(activated()); + disconnect(this, SIGNAL(activated()), recipient, m_slot); + } + + private: + QCString m_slot; + PlaylistRecipientMap m_playlistRecipient; +}; + K3bExporter::K3bExporter(Playlist *parent) : PlaylistExporter(parent), m_parent(parent) { } KAction *K3bExporter::action() { - if(!KStandardDirs::findExe("k3b").isNull()) { - return new KAction( + if(!m_action && !KStandardDirs::findExe("k3b").isNull()) { + m_action = new PlaylistAction( + "export_to_k3b", i18n("Add Selected Items to Audio or Data CD"), SmallIconSet("k3b"), - 0, - this, - SLOT(slotExport()), - actions(), - "export_to_k3b" + SLOT(slotExport()) ); + + m_action->setShortcutConfigurable(false); } - return 0; + // Tell the action to let us know when it is activated when + // m_parent is the visible playlist. This allows us to reuse the + // action to avoid duplicate entries in KActionCollection. + if(m_action) + m_action->addCallMapping(m_parent, this); + + return m_action; } void K3bExporter::exportPlaylistItems(const PlaylistItemList &items) --- branches/KDE/3.5/kdemultimedia/juk/k3bexporter.h #510356:510357 @@ -22,6 +22,7 @@ class QWidget; class DCOPRef; class PlaylistBox; +class PlaylistAction; /** * Class that will export the selected items of a playlist to K3b. @@ -61,6 +62,10 @@ // Private member variable declarations Playlist *m_parent; + + // Static member used to avoid adding more than one action to KDE's + // action list. + static PlaylistAction *m_action; }; /** --- branches/KDE/3.5/kdemultimedia/juk/playlistcollection.cpp #510356:510357 @@ -49,6 +49,12 @@ using namespace ActionCollection; //////////////////////////////////////////////////////////////////////////////// +// static methods +//////////////////////////////////////////////////////////////////////////////// + +PlaylistCollection *PlaylistCollection::m_instance = 0; + +//////////////////////////////////////////////////////////////////////////////// // public methods //////////////////////////////////////////////////////////////////////////////// @@ -65,6 +71,8 @@ m_belowDistraction(0), m_distraction(0) { + m_instance = this; + m_actionHandler = new ActionHandler(this); PlayerManager::instance()->setPlaylistInterface(this); --- branches/KDE/3.5/kdemultimedia/juk/playlistcollection.h #510356:510357 @@ -43,10 +43,14 @@ friend class CollectionList; friend class DynamicPlaylist; + static PlaylistCollection *m_instance; + public: PlaylistCollection(QWidgetStack *playlistStack); virtual ~PlaylistCollection(); + static PlaylistCollection *instance() { return m_instance; } + virtual QString name() const; virtual FileHandle currentFile() const; virtual int count() const;