Bug 124501 - Some keyboard shortcuts bound twice
Summary: Some keyboard shortcuts bound twice
Status: RESOLVED FIXED
Alias: None
Product: kmail
Classification: Applications
Component: general (show other bugs)
Version: 1.9.1
Platform: Debian testing Linux
: NOR normal
Target Milestone: ---
Assignee: kdepim bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-03-29 16:58 UTC by Magnus Holmgren
Modified: 2007-09-14 12:17 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Removes the superflous bindings (2.68 KB, patch)
2006-03-29 17:03 UTC, Magnus Holmgren
Details
Updated version of the above patch (3.00 KB, patch)
2006-06-20 20:00 UTC, Boris Dušek
Details
Fix a default shortcut to SHIFT+Key_P (589 bytes, patch)
2006-06-20 23:22 UTC, Boris Dušek
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Magnus Holmgren 2006-03-29 16:58:06 UTC
Version:           1.9.1 (using KDE KDE 3.5.1)
Installed from:    Debian testing/unstable Packages
OS:                Linux

In void KMMainWidget::createWidgets(void), there are a number (10) of double key bindings like this:

  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()));

The KAction should be sufficient; the second statement should be removed in each case.
Comment 1 Magnus Holmgren 2006-03-29 17:03:17 UTC
Created attachment 15352 [details]
Removes the superflous bindings

And please add KActions for the other key bindings as well ... but that's a
different bug (or 7).
Comment 2 Boris Dušek 2006-06-20 20:00:32 UTC
Created attachment 16722 [details]
Updated version of the above patch

This patch is mostly the same as the previous, only with updates on the "j"
shortcut, which is now made configurable and assigned by default to "SHIFT+P".
I tested on my machine and works fine.

Can please someone commit this?
Comment 3 Magnus Holmgren 2006-06-20 20:37:41 UTC
Except my patch isn't compatible with trunk anymore, since the code has been changed due to all constructors taking a shortcut argument being deprecated.

Still, they just have to delete a bunch of lines, which they should be able to do quickly even manually.
Comment 4 Ingo Klöcker 2006-06-20 21:55:44 UTC
*** Bug 128984 has been marked as a duplicate of this bug. ***
Comment 5 Ingo Klöcker 2006-06-20 23:03:00 UTC
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
Comment 6 Boris Dušek 2006-06-20 23:22:49 UTC
Created attachment 16729 [details]
Fix a default shortcut to SHIFT+Key_P

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. Attached is patch that does just this.