[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: [PATCH] Re: KDE/kdelibs/kdeui/xmlgui
From:       "Friedrich W. H. Kossebau" <kossebau () kde ! org>
Date:       2009-01-11 18:10:26
Message-ID: 200901111910.27295.kossebau () kde ! org
[Download RAW message or body]

Hi,

not yet done, I fear :)

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?
>
> 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.

Perhaps there is another flaw? Take Kate. It tries to reset the container menu 
of a standard action (should that be possible at all?), see 
kdesdk/kate/data/kateui.rc:
   <Menu name="documents"><text>&amp;Document</text>
    <Action name="go_back"/>
    <Action name="go_forward"/>
     <Separator />
   </Menu>
while kdelibs/kdeui/xmlgui/ui_standards.rc places these actions in the "Go" 
menu.

They are added (kdesdk/kate/app/katefilelist.cpp) with 
  m_windowNext = actionCollection->addAction(KStandardAction::Back, this, 
SLOT(slotPrevDocument()));
  m_windowPrev = actionCollection->addAction(KStandardAction::Forward, this, 
SLOT(slotNextDocument()));

Result: The action appears currently twice, in the menu "Go", as defined in 
ui_standards.rc, and in the menu "Document", as defined by kateui.rc.

I have no time to investigate currently, perhaps someone else, e.g you, 
Michael, can?

Cheers
Friedrich
-- 
Okteta - KDE 4 Hex Editor - http://utils.kde.org/projects/okteta
[prev in list] [next in list] [prev in thread] [next in thread]