KActionCollection setAssociatedWidget problems

[Anders Lund]

On Sunday 21 October 2007, Andreas Hartmetz wrote:
> Am Sonntag, 21. Oktober 2007 17:31:43 schrieb Hamish Rodda:
> > On Mon, 22 Oct 2007 12:39:00 am Andreas Hartmetz wrote:
> > > Am Sonntag, 21. Oktober 2007 16:01:02 schrieb Hamish Rodda:
> > > > Hi,
> > > >
> > > > A while back there was a cleanup of KActionCollection which subtly
> > > > changed the behaviour of setAssociatedWidget(QWidget*).  This
> > > > function is intended to make the actions specific to that widget, and
> > > > accomplishes this by adding all actions to the widget and setting
> > > > their shortcut context to Qt::WidgetShortcut. (It replaces KAccel's
> > > > widget binding functionality)
> > > >
> > > > The problem is that in the cleanup, the mechanism to ensure any
> > > > further actions were added to the widget was kept, but the mechanism
> > > > to ensure the shortcut context got changed to Qt::WidgetShortcut was
> > > > deleted. Now, the only shortcuts which have their context changed are
> > > > those which are already a part of the action collection when
> > > > setAssociatedWidget is called.
> > >
> > > See below...
> > >
> > > > This has created a set of serious bugs: when parts are merged
> > > > together, actions' shortcuts step on each other's toes if the
> > > > setAssociatedWidget call was prior to their addition to the action
> > > > collection.  For instance, KDirOperator's delete shortcut (a
> > > > WindowShortcut) competes with katepart's delete shortcut, thus delete
> > > > doesn't work when you're typing in kate.
> > > >
> > > > While I agree that the magic of changing the shortcut context is not
> > > > ideal, it is the intention (or at least I hope it is) of every use of
> > > > setAssociatedWidget to conveniently have this effect.
> > >
> > > There was actually a complaint from an application programmer about
> > > the "magic" behavior. So that's why I removed it, but apparently not in
> > > all places.
> > >
> > > > I believe the best course of action is to reintroduce this behaviour
> > > > (one line patch attached).  The alternative really is to remove
> > > > setAssociatedShortcut and let the apps do it themselves, but I think
> > > > this would lead to more errors.
> > >
> > > IMHO collections that change their contents are a dangerous concept.
> > > There may be cases where it works, but generally it'll just give you
> > > problems. Imagine you are debugging an application and objects change
> > > but you have no idea why - because it's implicit and you can't know
> > > without reading the documentation of all classes involved. Even a
> > > breakpoint at the setter won't necessarily work because not everything
> > > goes through the setter - example: destroying and recreating an object
> > > changes its properties without calling any setters.
> >
> > Ok. Should we remove *AssociatedWidget from the api and do:
> >
> > for (KAction* action, yourActionCollection) {
> >    action->setShortcutContext(Qt::WidgetShortcut);
> >    widget->addAction(action);
> > }
> >
> > for every use of it?  Perhaps replace it with
> > KActionCollection::associateActionsWithWidget(QWidget*), which basically
> > does the above on a once-off only...
>
> I'd agree with both but I also don't feel like deciding about something I
> have never used as an application programmer...
> Any further opinions? Sorry, I know we've had this topic several times
> already :/

I don't get the point with having 
KActionCollection::setAssociatedWidget(QWidget*) if it does not do that, and 
maintain it.

-- 
Anders

www: http://www.alweb.dk
jabber: anderslund at jabber.dk