[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: Re: PATCH: Re: kdelibs/kdeui
From: Ellis Whitehead <kde () ellisw ! net>
Date: 2002-01-30 16:53:55
[Download RAW message or body]
Simon,
If after you read this, you still feel like having unplugAll() in ~KAction()
might be risky, I'll go ahead and revert the commit.
> On Wednesday 30 January 2002 03:48, Simon Hausmann wrote:
> > On Tue, Jan 29, 2002 at 09:38:27PM -0500, Ellis Whitehead wrote:
> > > This patch has KAction::~KAction() call unplugAll() to remove itself
> > > from any menu or toolbar it's in.
> >
> > I remember we had trouble with that, and Waldo also remembered that
> > we had this in some 2.x version and had a lot of bug reports about
> > this one.
According to the records on kde-core-devel and bugs.kde.org, the problem was
in krootwm.cc and not in KAction::~KAction() or KAction::unplugAll(). The
bookmark list was getting deleted twice, and unplugAll() is merely where the
error showed up.
> > In addtion it doesn't work quite the way one would expect
> > as calling that virtual method will only do half of the job by
> > calling only KAction's own unplugAll().
Good that you pointed out the issue with calling virtual unplug() from the
KAction destructor. However, I don't think that it's a "real" problem.
Judging by a quick look at all the virtual unplug() methods in kaction.cpp,
they are basically just cut-and-paste without:
if ( m_parentCollection )
m_parentCollection->disconnectHighlight( {bar,menu}, this );
Seems like they can be removed anyhow. I'll have to look at it more closely,
but if we add a check for QMenuBar in KAction::unplug(), then maybe all of
them could be removed. If there are cases where that's not true, then
unplugAll() will need to be called from the destructor of that class.
> > (see previous threads on
> > kde-core-devel about this problem)
There was a discussion in early November (while I was at the ALS conference
and not keeping up on the list), but in the end the only reason stated
against calling unplugAll() from ~KAction() was about not needing to make
those calls during shut-down.
http://lists.kde.org/?l=kde-core-devel&m=95846925315076&w=2
The safety and convenience is worth it, though, so that actions from a dynamic
widget can be used by a different persistent widget. Without the unplugAll()
call in ~KAction(), attempts to handle this are very hacky.
Regards,
Ellis
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic