Bug 121621 - Action List under Settings - Configure Shortcuts - List includes Add to CD 39 times.
Summary: Action List under Settings - Configure Shortcuts - List includes Add to CD 39...
Status: RESOLVED FIXED
Alias: None
Product: juk
Classification: Applications
Component: general (show other bugs)
Version: 2.3
Platform: openSUSE Linux
: NOR normal
Target Milestone: ---
Assignee: Scott Wheeler
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-02-08 23:30 UTC by Neill Conroy
Modified: 2006-02-17 02:28 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
Fix for duplicate KActions being created. (5.19 KB, patch)
2006-02-11 05:08 UTC, Michael Pyne
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Neill Conroy 2006-02-08 23:30:58 UTC
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.
Comment 1 Scott Wheeler 2006-02-08 23:38:18 UTC
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?
Comment 2 Michael Pyne 2006-02-09 01:56:33 UTC
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.
Comment 3 Michael Pyne 2006-02-11 05:08:11 UTC
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.
Comment 4 Michael Pyne 2006-02-17 02:28:57 UTC
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;