Bug in KAction::initPrivate

[Simon Hausmann]

On Wed, Sep 25, 2002 at 02:49:31PM +0200, Michael Brade wrote:
> > Well, of course I only mean reverting the object-{42}-name magic. As
> > I said in my previous mail I completely agree with you that the
> > double connection is a bug.
> Hmmm... this still leaves us without a fix for KNotes. I tried a lazy way:=
> =20
> just disconnect the activated signals in KSelectAction's CTOR but somehow=20
> this doesn't work - I can't disconnect anything in KSelectAction's CTOR, no=
> t=20
> even this->disconnect() works, it returns false :-( Why?

Ok, we are facing two different problems here.

In KDE 3.0 (and in previous versions) the receiver/slot pair passed
to action constructors was usually connected to the activated()
signal. An exception to this was KListAction, which assumed a foo(
int ) signature of the slot. This inconsistency was not documented,
although I think the behaviour makes sense. Using a KListAction one
is generally interested in the activated( int ) signal, less in the
activated() one. 

Two of the KListAction constructors had the bug that they passed the
receiver/slot pair to the KAction constructor, which should have
caused run-time warnings because of the signal/slot incompatibility.
I can't remember any such warnings, but I'm pretty sure they
happened.

<at this point KDE 3.0 was released>

Then Ellis introduced the feature of attaching the integer value
enclosed by {} in the object name as argument to (the newly added)
activated(int) signal, to ease the work of the kdegames people.

After that one day Martijn told me on IRC that he had a strange
problem with doubled activation when using KListAction, his slot got
called twice. I think this was in kopete. I looked at the
KListAction constructor and I saw the explicit connect() call as
well as that the constructor passed the receiver/slot argument to
the base class. I saw no sense in this and thought it was the reason
for the double connect. See revision 1.278 and 1.279 of kaction.cpp.

What I apparently overlooked was that the KListAction constructor
was connecting a different signal, the one with the integer
argument, the one that KListAction declares.

I'm not exactly sure how Martijn got the double connect though. Did
the object name of the action happen to be like foo{42} ? (it's the
only explanation I can come up with)

So we actually face two different problems here:

1) The foo{42} feature, which I think is to be better implemented
   using QSignalMapper.

2) My change in the KListAction constructor, which attempted to fix
   Martijn's problem. Unfortunately my fix was wrong, as it causes
   the bug Michael is experiencing, by breaking the KDE 3.0
   behaviour of unconditionally connecting the activated( int )
   signal.

I believe (1) should be reverted for the sake of a cleaner API (see
my previous mail) and (2) should be reverted as well, as it's
obviously the wrong fix and breaks KNotes. In addition to that I
believe the KListAction constructors should be changed to not pass
any receiver/slot pair to the base class (for the above mentioned
reason to avoid run-time warnings) and it should be documented that
KListAction expects a foo( int ) type of slot for those constructors
that take a receiver/slot pair.

I'm happy that this comes up before 3.1, so we should be able to fix
this mess without breaking any KDE 3.0 apps.  It looks like Ellis
agrees to reverting (1) . I'd be good to hear a comment from the
kdegames people though.

Any other thoughts/opinions?

Simon