Bug 180022 - [PATCH] Add accelerator to "save image as..." action popup
Summary: [PATCH] Add accelerator to "save image as..." action popup
Alias: None
Product: konqueror
Classification: Applications
Component: khtml (show other bugs)
Version: SVN
Platform: Unlisted Binaries Linux
: NOR normal (vote)
Target Milestone: ---
Assignee: Konqueror Developers
Depends on:
Reported: 2009-01-08 13:59 UTC by FiNeX
Modified: 2009-01-08 22:06 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:

accelerator patch (641 bytes, patch)
2009-01-08 13:59 UTC, FiNeX

Note You need to log in before you can comment on or make changes to this bug.
Description FiNeX 2009-01-08 13:59:05 UTC
Created attachment 30026 [details]
accelerator patch

Can I apply this simple patch to add an accelerator for "save image as..." action on right click on images in webpages?
Comment 1 David Faure 2009-01-08 14:10:17 UTC
I think we should rather use KAcceleratorManager to ensure all actions have an accel. I'll give it a try.
Comment 2 David Faure 2009-01-08 14:13:06 UTC
SVN commit 907616 by dfaure:

Assign accelerators to all menu entries automatically.
BUG: 180022
Please test for possible side effects before I backport :)

 M  +3 -0      konq_popupmenu.cpp  

WebSVN link: http://websvn.kde.org/?view=rev&revision=907616
Comment 3 FiNeX 2009-01-08 14:45:14 UTC
Your fix seems works right: it adds the accelerator on all entries and there are no conflicts. I've tried most of the actions using accelerators and them all works. Which other tests could I do for being sure it is does not introduce side effects?
Comment 4 David Faure 2009-01-08 14:55:25 UTC
I was thinking of whether it changed a previously hardcoded accelerator, so people who got into the habit of using it would be disoriented by this change.
But AFAIK this shouldn't happen, kacceleratormanager is supposed to honour hardcoded accels before assigning automatic ones.
Comment 5 FiNeX 2009-01-08 22:00:06 UTC
Before your patch, there were some conficts, for example "c" was used for both "copy text" and "copy link address". Now, the accelerator for "copy link address" has become "l" (select a link and right click on it) because of the new code. This behaviour looks good to me: I personaly prefer to learn a new accelerator rather than having the same for more actions.

Comment 6 David Faure 2009-01-08 22:06:15 UTC
That's right. OK, thanks for the testing - patch backported.