Version: (using KDE KDE 3.5.3) Installed from: Gentoo Packages Hi, running KMail shipped with 3.5.3 (in Kontact), whenever I press the key "j", a "Jump to folder" dialog pops up. This annoys me since I heavily use the vim-like behavior that uses "k", "j" to move down and up. I found the "Jump to folder" action in "Configure shortcuts" dialog and unset the keyboard shortcut for that action (set it to "None"), but kmail did not respect the change. Note that scrolling up with "k" key works.
I agree, I use j, k scrolling heavily too and it bugs me that I can't turn this off (using Kubuntu packages). Cheers.
Ok, so I looked into it and added three lines for 3 shortcuts, one of which was the "j" shortcut. Now it works as expected. Patch follows. Please test it/review it and report back any problems. Thanks.
Created attachment 16719 [details] Patch adding configurability of the shortcuts
I don't think the setShortcutConfigurable() calls are needed - KAction shortcuts are by default configurable. You just need to delete the mAccel->connectItem() calls. This bug overlaps bug 124501.
Yes, Magnus, the setShortcutConfigurable() calls are not needed. I updated your patch from bug 124501 with the "j" shortcut (which was probably added after 3.5.2) and sent it there. Thanks for the info. Please someone mark this as a duplicate of 124501 (I don't know how). Thanks.
*** This bug has been marked as a duplicate of 124501 ***
SVN commit 553370 by kloecker: Fix bug 124501 (Some keyboard shortcuts bound twice) and 128984 ("Jump to folder" kbd shortcut hardcoded). Don't use a QAccel. For one we already have an appropriate KAccel (actioncollection()->kaccel()). Moreover, using QAccel breaks the configurability of kbd shortcuts. KAction->plugAccel is deprecated but I don't see what else to use. I guess the additional plugAccel shouldn't be necessary, but at least for Return the accel didn't work out of the box (probably because the corresponding KAction doesn't appear in any menu or toolbar). BUGS: 124501 CCBUGS: 128984 M +37 -43 kmmainwidget.cpp M +0 -3 kmmainwidget.h --- branches/KDE/3.5/kdepim/kmail/kmmainwidget.cpp #553369:553370 @@ -38,6 +38,7 @@ #include <kstandarddirs.h> #include <dcopclient.h> #include <kaddrbook.h> +#include <kaccel.h> #include "globalsettings.h" #include "kcursorsaver.h" @@ -126,8 +127,7 @@ QWidget(parent, name), mQuickSearchLine( 0 ), mShowBusySplashTimer( 0 ), - mShowingOfflineScreen( false ), - mAccel( 0 ) + mShowingOfflineScreen( false ) { // must be the first line of the constructor: mStartupDone = FALSE; @@ -473,8 +473,6 @@ //----------------------------------------------------------------------------- void KMMainWidget::createWidgets(void) { - mAccel = new QAccel(this, "createWidgets()"); - // Create the splitters according to the layout settings QWidget *headerParent = 0, *folderParent = 0, *mimeParent = 0, *messageParent = 0; @@ -544,9 +542,10 @@ this, SLOT(slotMsgActivated(KMMessage*))); connect( mHeaders, SIGNAL( selectionChanged() ), SLOT( startUpdateMessageActionsTimer() ) ); - mAccel->connectItem(mAccel->insertItem(SHIFT+Key_Left), + QAccel *accel = actionCollection()->kaccel(); + accel->connectItem(accel->insertItem(SHIFT+Key_Left), mHeaders, SLOT(selectPrevMessage())); - mAccel->connectItem(mAccel->insertItem(SHIFT+Key_Right), + accel->connectItem(accel->insertItem(SHIFT+Key_Right), mHeaders, SLOT(selectNextMessage())); if (mReaderWindowActive) { @@ -562,33 +561,34 @@ mMsgView, SLOT(clearCache())); connect(mMsgView, SIGNAL(noDrag()), mHeaders, SLOT(slotNoDrag())); - mAccel->connectItem(mAccel->insertItem(Key_Up), + accel->connectItem(accel->insertItem(Key_Up), mMsgView, SLOT(slotScrollUp())); - mAccel->connectItem(mAccel->insertItem(Key_Down), + accel->connectItem(accel->insertItem(Key_Down), mMsgView, SLOT(slotScrollDown())); - mAccel->connectItem(mAccel->insertItem(Key_Prior), + accel->connectItem(accel->insertItem(Key_Prior), mMsgView, SLOT(slotScrollPrior())); - mAccel->connectItem(mAccel->insertItem(Key_Next), + accel->connectItem(accel->insertItem(Key_Next), mMsgView, SLOT(slotScrollNext())); } else { mMsgView = NULL; } - new KAction( i18n("Move Message to Folder"), Key_M, this, + KAction *action; + + action = new KAction( i18n("Move Message to Folder"), Key_M, this, SLOT(slotMoveMsg()), actionCollection(), "move_message_to_folder" ); - new KAction( i18n("Copy Message to Folder"), Key_C, this, + action->plugAccel( actionCollection()->kaccel() ); + + action = new KAction( i18n("Copy Message to Folder"), Key_C, this, SLOT(slotCopyMsg()), actionCollection(), "copy_message_to_folder" ); - new KAction( i18n("Jump to Folder"), Key_J, this, + action->plugAccel( actionCollection()->kaccel() ); + + action = new KAction( i18n("Jump to Folder"), Key_J, this, SLOT(slotJumpToFolder()), actionCollection(), "jump_to_folder" ); - mAccel->connectItem(mAccel->insertItem(Key_M), - this, SLOT(slotMoveMsg()) ); - mAccel->connectItem(mAccel->insertItem(Key_C), - this, SLOT(slotCopyMsg()) ); - mAccel->connectItem(mAccel->insertItem(Key_J), - this, SLOT(slotJumpToFolder()) ); + action->plugAccel( actionCollection()->kaccel() ); // create list of folders mFolderTree = new KMFolderTree(this, folderParent, "folderTree"); @@ -607,50 +607,45 @@ this, SLOT(slotFolderTreeColumnsChanged())); //Commands not worthy of menu items, but that deserve configurable keybindings - new KAction( + action = new KAction( i18n("Remove Duplicate Messages"), CTRL+Key_Asterisk, this, SLOT(removeDuplicates()), actionCollection(), "remove_duplicate_messages"); + action->plugAccel( actionCollection()->kaccel() ); - new KAction( + action = new KAction( i18n("Abort Current Operation"), Key_Escape, ProgressManager::instance(), SLOT(slotAbortAll()), actionCollection(), "cancel" ); - mAccel->connectItem(mAccel->insertItem(Key_Escape), - ProgressManager::instance(), SLOT(slotAbortAll())); + action->plugAccel( actionCollection()->kaccel() ); - new KAction( + action = new KAction( i18n("Focus on Next Folder"), CTRL+Key_Right, mFolderTree, SLOT(incCurrentFolder()), actionCollection(), "inc_current_folder"); - mAccel->connectItem(mAccel->insertItem(CTRL+Key_Right), - mFolderTree, SLOT(incCurrentFolder())); + action->plugAccel( actionCollection()->kaccel() ); - new KAction( + action = new KAction( i18n("Focus on Previous Folder"), CTRL+Key_Left, mFolderTree, SLOT(decCurrentFolder()), actionCollection(), "dec_current_folder"); - mAccel->connectItem(mAccel->insertItem(CTRL+Key_Left), - mFolderTree, SLOT(decCurrentFolder())); + action->plugAccel( actionCollection()->kaccel() ); - new KAction( + action = new KAction( i18n("Select Folder with Focus"), CTRL+Key_Space, mFolderTree, SLOT(selectCurrentFolder()), actionCollection(), "select_current_folder"); - mAccel->connectItem(mAccel->insertItem(CTRL+Key_Space), - mFolderTree, SLOT(selectCurrentFolder())); - new KAction( + action->plugAccel( actionCollection()->kaccel() ); + + action = new KAction( i18n("Focus on Next Message"), ALT+Key_Right, mHeaders, SLOT(incCurrentMessage()), actionCollection(), "inc_current_message"); - mAccel->connectItem( mAccel->insertItem( ALT+Key_Right ), - mHeaders, SLOT( incCurrentMessage() ) ); + action->plugAccel( actionCollection()->kaccel() ); - new KAction( + action = new KAction( i18n("Focus on Previous Message"), ALT+Key_Left, mHeaders, SLOT(decCurrentMessage()), actionCollection(), "dec_current_message"); - mAccel->connectItem( mAccel->insertItem( ALT+Key_Left ), - mHeaders, SLOT( decCurrentMessage() ) ); + action->plugAccel( actionCollection()->kaccel() ); - new KAction( + action = new KAction( i18n("Select Message with Focus"), ALT+Key_Space, mHeaders, SLOT( selectCurrentMessage() ), actionCollection(), "select_current_message"); - mAccel->connectItem( mAccel->insertItem( ALT+Key_Space ), - mHeaders, SLOT( selectCurrentMessage() ) ); + action->plugAccel( actionCollection()->kaccel() ); connect( kmkernel->outboxFolder(), SIGNAL( msgRemoved(int, QString) ), SLOT( startUpdateMessageActionsTimer() ) ); @@ -3516,8 +3511,7 @@ //----------------------------------------------------------------------------- void KMMainWidget::setAccelsEnabled( bool enabled ) { - if ( mAccel ) - mAccel->setEnabled( enabled ); + actionCollection()->kaccel()->setEnabled( enabled ); } --- branches/KDE/3.5/kdepim/kmail/kmmainwidget.h #553369:553370 @@ -30,7 +30,6 @@ #include "kmkernel.h" // for access to config #include <kaction.h> -class QAccel; class QVBoxLayout; class QSplitter; @@ -501,8 +500,6 @@ KXMLGUIClient *mGUIClient; static QValueList<KMMainWidget*>* s_mainWidgetList; - - QAccel *mAccel; }; #endif
Ingo, thanks for taking care of this. I suggest that the default shortcut for "Jump to folder" actions be changed to "SHIFT + Key_P", since "Key_J" collides with scrolling down. Attaching a patch that does just this.
Created attachment 16730 [details] Fix a default shortcut to SHIFT+Key_P
On Tuesday 20 June 2006 23:02, Ingo Klöcker wrote: > SVN commit 553370 by kloecker: > I guess the additional plugAccel > shouldn't be necessary, but at least for Return the accel didn't work > out of the box (probably because the corresponding KAction doesn't > appear in any menu or toolbar). Yes. Till
I'm sorry, but we won't change the default shortcut from j to something else. We love simple and easy to remember shortcuts (like c for Copy, m for Move and j for Jump).
I agree. I have no problem with it as long as it's configurable. I have changed *many* shortcuts. :-) Will you make the keys for scrolling configurable next? (As suggested in the infamous(?) bug 96301.) :-) BTW, this bug was marked duplicate before. Well, it doesn't really matter.
While I agree that shortcuts should be simple, I do not think that shipping kmail with conflicting default shortcuts is a good thing. Especially when the shortcut that introduces the conflict is a new one and takes precendence over another shortcut that people might have been using for years and *got used* to it. Even more used to it since scrolling with "j" and "k" is available in konqueror too. But if after reading the previous paragraph you still think that the shortcuts should conflict than I can't do much with it :).
I expect normal users to be far more confused by a Konqueror that scrolls with "j" and "k" than with a KMail which invokes the _J_ump to Folder functionality with "j". Scrolling with "j"/"k" is a relict of the stone age when computer keyboards didn't have arrow keys. If a usability study shows that choosing "j" as shortcut for Jump to Folder was a bad idea, then we'll change it. Until then we'll keep it. In any case, the shortcut is now configurable and no longer hardcoded. So this bug is resolved. BTW, thanks for your patches. It's always great to see users who don't just file bug reports and wait for us to fix them, but who also dive into the source code to resolve the problem themselves.