setDefaultShortcutContext in KActionCollection

[Hamish Rodda]

On Thu, 26 Jul 2007 08:30:35 pm Andreas Hartmetz wrote:
> On Wednesday 25 July 2007 21:48:52 Robin Stocker wrote:
> > > Can you give a usage example?
> >
> > Ah, sure. I wanted to fix a bug in Konsole for KDE 4, where the
> > shortcuts (Ctrl+Shift+N, etc) didn't work when the menu bar was hidden.
> > This is because the shortcuts are for actions which are added to the
> > menu and when the menu is hidden it doesn't get events and so the
> > shortcuts don't work anymore (this behaviour changed from Qt 3 to Qt 4,
> > I have tested it with minimal examples).
> >
> > The solution is to add the actions not only to the menu, but also to the
> > main window. Then I thought, well, why not use the setAssociatedWidget
> > method of KActionCollection? But this doesn't work because of the second
> > line in addAction in kactioncollection.cpp (line 238):
> >
> >     if (d->associatedWidgets.count()) {
> >         action->setShortcutContext(Qt::WidgetShortcut);
> >
> >         foreach (QWidget* w, d->associatedWidgets)
> >             w->addAction(action);
> >
> >     }
> >
> > The shortcut context is set from the default Qt::WindowShortcut to
> > Qt::WidgetShortcut, which means that the main window would have to have
> > direct focus, but that's never the case.
> >
> > Now, I don't understand why the shortcut context is set to
> > Qt::WidgetShortcut in the first place. Do you know why this could be
> > necessary? If we could remove this line, there would be no need for
> > setDefaultShortcutContext anymore, because the default of QAction is
> > Qt::WindowShortcut, which is fine. Also, it would be less surprising
> > that the shortcut context is changed in a addAction method.
>
> Yes!
>
> > The other solution is to add setDefaultShortcutContext again, but it
> > would be more code in both kactioncollection.cpp and the client code.
>
> Would be nice to avoid this.
>
> > I hope this makes the problem clear. Here's my original mail with the
> > patch which describes the problem and the solutions in greater detail:
> >
> > http://lists.kde.org/?l=konsole-devel&m=118529209602516&w=2
> >
> > Thank you for looking into this,
> >   Robin
>
> Phew, and I thought that we'd have to reintroduce the old functions...
> KActionCollection::setAssociatedWidget() is new in KDE 4, so nobody should
> be depending on the "bug"... I hope.

As I said in my other email just now, this was all so that KAccel could be 
ported easily, it was no bug.  Almost everywhere that this function is used 
currently actually relies on this because it is a direct port from KAccel.

> Removing the line *now* might break something, but IMO it's much better to
> break applications now when everything is somewhat broken. Suprising
> behavior can bite application developers even long after release.
> So, without further ado, I'm going to commit this change.

It will break almost every use of this function.  However it's probably best 
to find every call and set the widget shortcut context where relevant, to 
avoid hidden smarts in KActionCollection (that were less hidden before the 
default shortcut context functions were removed).

Cheers,
Hamish.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20070730/3d655bc9/attachment.sig>