[PATCH] Re: KDE/kdelibs/kdeui/xmlgui

[Friedrich W. H. Kossebau]

Hi Mike,

Am Samstag, 3. Januar 2009, um 01:32 Uhr, schrieb Michael Jansen:
> On Saturday 03 January 2009 00:10:03 Michael Jansen wrote:
> > > Please try the attached patch.
> > > It tries to solve the problem by just passing 0 as parent to
> > > KStandardAction::create(). As with all action classes which are created
> > > by KStandardAction::create() the parent seems just to be used only for
> > > Qt's garbage control. So this should be theoretically no problem. With
> > > 0 as parent KStandardAction::create() now does not add the new action
> > > to the collection itself. And practically I found no problem, too, e.g.
> > > with Okular, Konqueror and Konsole. And Okteta :P
> > >
> > > So if this patch works for you all, it should be committed and your
> > > ui_standards.rc commit, Urs, can be reverted :)
> >
> > I have no objections to the patch if it solves the problem. My problem is
> > WHY does it solve the problem?
> >
> > if you look at kactioncollection.cpp:264/287/283 you will find code
> > trying to prevent the problem you described. In this case line 283 is
> > relevant.
> >
> >     // Check if we have this action under a different name.
> >     // Not using takeAction because we don't want to remove it from
> > categories,
> >     // and because it has the new name already.
> >     const int oldIndex = d->actions.indexOf(action);
> >     if (oldIndex != -1) {
> >         d->actionByName.remove(objectName);
> >         d->actions.removeAt(oldIndex);
> >     }
> >
> > So the problem you guys seem to have with having the SAME action with two
> > names in the action collection should not happen. Are you guys sure it's
> > the SAME action?

After you pointed at that code, I really have wondered, too ;)

> OK. I got it. See http://websvn.kde.org/?view=rev&revision=904755 . I've
> added a unit test and changed your comments a bit.
>
> The problem was introduced by me. It should be absolutely impossible to get
> the same action twice into a collection. I introduced the setObjectName()
> call in this KActionCollection::addAction(KStandardAction..) method which
> exposed a flaw in our logic trying to prevent double additions.

Fine. Now I feel better, with you as someone familiar with this code having 
given the final polish to the fix. And on any more errors I can now point at 
you :P

Thanks
Friedrich
-- 
Okteta - KDE 4 Hex Editor - http://utils.kde.org/projects/okteta