Bug in KActionCollection::connectNotify

David Faure faure at kde.org
Mon Mar 10 12:42:02 GMT 2008


On Friday 07 March 2008, Nicolas Ternisien wrote:
> I've just done a little patch on KActionCollection, which deprecate
> actionHighlighted() and rename it actionHovered(), to match the
> QAction API. This patch is linked to my previous and never answered
> email attached here.
> 
> That's my first KDEcore patch, so be cool with it ;-)

The idea is OK, but the implementation isn't: you can't add new virtual methods in a public class like KActionCollection, it breaks binary compatibility.
+  virtual void slotActionHovered();
It doesn't need to be virtual. All it does is emitting a signal, apps can decide what to do by connecting to the signal.

Otherwise, I have to ask: did you test this patch? I see an obvious bug in it:
   if (QMetaObject::normalizedSignature(SIGNAL(actionHighlighted(QAction*))) == signal) {
you need to also check for the new signal, otherwise it won't detect that somebody connected to it and it won't be emitted...
This should be something like
   if (QMetaObject::normalizedSignature(SIGNAL(actionHighlighted(QAction*))) == signal
    || QMetaObject::normalizedSignature(SIGNAL(actionHovered(QAction*))) == signal) {

-- 
David Faure, faure at kde.org, sponsored by Trolltech to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).




More information about the kde-core-devel mailing list