Shortcut Scheme support

[Michael Jansen]

Ok. I did some research and the reason is what we all knew or should have 
known from reading KActions documentation.

When calling "(QAction*)qaction->setShortcut()" kde's code doesn't get the 
chance to remember the default shortcut. Which means the code worked before 
apakus change. But only by chance.

In okular for example the "browse tool" setup using a qaction* had the 
shortcut 'ctrl+1' but as a custom shortcut. No default shortcut there. Working 
but wrong behaviour.

The "zoom tool" using kaction* had the "ctrl+2" shortcut as a default 
shortcut. The desired behaviour

So the code using QAction::setShortcut worked before the shortcut scheme but 
not the way people expected. And i would consider it a bug if a program sets 
custom shortcuts instead of default shortcut. But noone noticed.

Mike



On Sunday 28 September 2008 00:27:17 Michael Jansen wrote:
> Hi
>
> It looks like that patch introduced some pretty heavy behaviour change. In
> the current form i would strongly advise to revert the patch.
>
> It was never a good idea to use a QAction* when handling KActions. But it
> seem to have somehow worked. Since that patch many apps doing that fail to
> work. We had lokalize, there is an akregator patch and okular doesn't work
> correctly anymore too.
>
> The problem shows itself as missing shortcuts. I have no idea if anythings
> else break. Open okular and have a look at the tools menu. For me there is
> only one action with an shortcut. The code set shortcuts for all of them.
> See the difference
>
> 	// d->aMouseNormal is a QAction*
>     d->aMouseNormal  = new KAction( ... );
>     ac->addAction("mouse_drag", d->aMouseNormal );
>     connect( d->aMouseNormal, SIGNAL( triggered() ), this, SLOT(
> slotSetMouseNormal() ) );
>     d->aMouseNormal->setIconText( i18nc( "Browse Tool", "Browse" ) );
>     d->aMouseNormal->setCheckable( true );
>     d->aMouseNormal->setShortcut( Qt::CTRL + Qt::Key_1 );
>
> against
>
>     KAction * mz  = new KAction(KIcon( "zoom-original" ), i18n("&Zoom
> Tool"), this);
>     ac->addAction("mouse_zoom", mz );
>     connect( mz, SIGNAL( triggered() ), this, SLOT( slotSetMouseZoom() ) );
>     mz->setIconText( i18nc( "Zoom Tool", "Zoom" ) );
>     mz->setCheckable( true );
>     mz->setShortcut( Qt::CTRL + Qt::Key_2 );
>     mz->setActionGroup( actGroup );
>
> Only actions setup using KAction* work. I was told that code worked before.
> That will break many existing applications. Lokalize and Akregator changed
> their code to use KAction* but i think that's not acceptable for a kde 4.2
> release.
>
>
> Mike


-- 
Michael Jansen

http://www.michael-jansen.biz