[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: accel problems: might be qt-copt, might be kdelibs
From:       Ellis Whitehead <ellis () kde ! org>
Date:       2002-10-19 22:16:31
[Download RAW message or body]

On Saturday 19 October 2002 23:12, Simon Hausmann wrote:
> On Sat, Oct 19, 2002 at 11:01:50PM +0200, Ellis Whitehead wrote:
> > On Saturday 19 October 2002 17:32, Simon Hausmann wrote:
> > > On Sat, Oct 19, 2002 at 04:55:43PM +0200, Simon Hausmann wrote:
> > > > On Wed, Oct 16, 2002 at 01:17:12AM +0200, Ingo Klöcker wrote:
> > > > > On Tuesday 15 October 2002 22:23, Ellis Whitehead wrote:
> > > > > > On Tuesday 15 October 2002 10:28, Matthias Ettrich wrote:
> > > > > > > On Monday 14 October 2002 22:45, Ellis Whitehead wrote:
> > > > > > > > > The problem in konqy still exists.
> > > > > > > > > Pressing alt+left goes back and activates the menubar,
> > > > > > > > > which then can't be deactivated using Escape.
> > > > > > >
> > > > > > > There were some problems in Qt, they will hopefully go away
> > > > > > > with the next beta.
> > > > > > >
> > > > > > > Matthias
> > > > > >
> > > > > > So Marc, Alexander, Ingo, David: shall I commit the patch?  It
> > > > > > fixes the KMail problem Marc initially mentioned.
> > > > >
> > > > > Well, if your patch doesn't break other things than please commit.
> > > >
> > > > Unfortunately the patch breaks ksirc, quite bad. KSirc's lineedit
> > > > listens for AccelOverride events and based on those processes keys.
> > > > With the patch the lineedit receivers two AccelOverride events for
> > > > one keypress. You can imagine the effect ;-)

Btw, using AccelOverride to process shortcut keys isn't a good idea for 
widgets, since it doesn't stop the KeyPress event from occuring.  

> > The double-evoking of AccelOverride is an unfortunate side-effect of the
> > event filter.  I've written a small patch for the problems you've
> > mentioned, but I can't test it till kdecore links for me again ('make'
> > problems in current cvs, apparently).  I'll post the patch once it's
> > working again.
>
> Great! (that you have a patch, not that kdecore doesn't build :)

I'm attaching the patch, in case you want to take a look.  At least it 
compiles...

> > > This event filter make me wonder: What is it that is so broken in Qt
> > > that KDE has to re-invent the whole wheel and handle X11 key events?
> >
> > It's not that Qt is broken, but rather that it wasn't designed to support
> > the KPart paradigm.  An accel is global within a Qt application, and we
> > can't give certain accels more priority over others (such as giving the
> > KPart with focus priority over a second visible KPart that doesn't have
> > the focus).
>
> I wonder why an accelerator is used then. If the activation depends
> on whether the part has the focus or not (and I agree with that
> behaviour) then I think the right thing is to listen for regular key
> events. Isn't that what the idea behind key events and focus
> handling?

A couple of the biggest reason for having actions is for the configurability 
and sparing the programmer the processing of KeyEvents.  The most significant 
conflicts come up when multiple unrelated KParts are embedded at one time.  
Each KPart want's to let its actions have configurable shortcuts, so they use 
KAction (and thus KAccel).

> If this is about actions then I think the actioncollection should
> simply listen for key events on the part's widget and catch action
> shortcuts appropriately.
>
> Or does that miss any case?

Yes, the problem is that KeyPress event is set to 'accept()' be default, so if 
the focus is in a subwidget (say the edit widget of a dialog), then the 
dialog doesn't ever get the KeyPress event to filter.

There are other benefits to having a KAccelEventHandler.  I've been working on  
a significant simplification of the currently complicated kaccel* code, which 
will reduce the line count to about half for KDE 3.2.  This is possible in 
part because KAccelEventHandler allows for unifying more code between KAccel 
& KGlobalAccel and for not having to work around the Qt accelerator id 
paradigm.  That means I can remove most of kaccelbase.* and kaccelaction.*.
Furthermore, it makes it possible to define accelerators which will _only_ be 
activated when the watch widget has focus.  This lets programmers make keys 
which they currently process in keyPressEvent be defined as configurable 
actions.
In KDE 3.2, KAction will handle accelerators of all three scopes (global, 
application, and widget) rather than the programmer having to use different 
classes and APIs for each.  Rather nice, if you ask me. ;)  Anyhow, this 
requires KAccelEventHandler, I think.

Cheers,
Ellis

["kdecore.patch" (text/x-diff)]

Index: kapplication.cpp
===================================================================
RCS file: /home/kde/kdelibs/kdecore/kapplication.cpp,v
retrieving revision 1.537
diff -u -3 -d -p -b -B -r1.537 kapplication.cpp
--- kapplication.cpp	2002/09/11 08:12:36	1.537
+++ kapplication.cpp	2002/10/19 21:46:26
@@ -355,9 +355,28 @@ void KApplication::x11FilterDestroyed()
     }
 }
 
+// FIXME: remove this when we've get a better method of 
+// customizing accelerator handling -- hopefully in Qt.
+// For now, this is set whenever an accelerator is overriden
+// in KAccelEventHandler so that the AccelOverride isn't sent twice. -- ellis, \
19/10/02 +extern bool g_bKillAccelOverride;
+
 bool KApplication::notify(QObject *receiver, QEvent *event)
 {
     QEvent::Type t = event->type();
+    if (g_bKillAccelOverride)
+    {
+       g_bKillAccelOverride = false;
+       // Indicate that the accelerator has been overridden.
+       if (t == QEvent::AccelOverride)
+       {
+          static_cast<QKeyEvent *>(event)->accept();
+          return true;
+       }
+       else
+          kdWarning(125) << "g_bKillAccelOverride set, but received an event other \
than AccelOverride." << endl; +    }
+    
     if ((t == QEvent::AccelOverride) || (t == QEvent::KeyPress))
     {
        static const KShortcut& _selectAll = KStdAccel::selectAll();
Index: kaccel.cpp
===================================================================
RCS file: /home/kde/kdelibs/kdecore/kaccel.cpp,v
retrieving revision 1.136
diff -u -3 -d -p -b -B -r1.136 kaccel.cpp
--- kaccel.cpp	2002/10/16 18:31:18	1.136
+++ kaccel.cpp	2002/10/19 21:46:28
@@ -17,22 +17,13 @@
     Boston, MA 02111-1307, USA.
 */
 
+#include "kaccel.h"
+
 #include <qaccel.h>
 #include <qpopupmenu.h>
 #include <qstring.h>
 #include <qtimer.h>
 
-#ifdef Q_WS_X11
-#	include <X11/Xlib.h>
-#	include <X11/keysymdef.h>
-
-	// defined by X11 headers
-	const int XKeyPress = KeyPress;
-#	undef KeyPress
-#endif
-
-#include "kaccel.h"
-
 #include <kaccelbase.h>
 #include <kapplication.h>
 #include <kdebug.h>
@@ -41,11 +32,22 @@
 
 #include "kaccelprivate.h"
 
+#ifdef Q_WS_X11
+#	include <X11/Xlib.h>
+#	ifdef KeyPress // needed for --enable-final
+		// defined by X11 headers
+		const int XKeyPress = KeyPress;
+#		undef KeyPress
+#	endif
+#endif
+
 // TODO: Put in kaccelbase.cpp
 //---------------------------------------------------------------------
 // KAccelEventHandler
 //---------------------------------------------------------------------
 
+bool g_bKillAccelOverride = false;
+
 class KAccelEventHandler : public QWidget
 {
  public:
@@ -100,12 +102,19 @@ bool KAccelEventHandler::x11Event( XEven
 		
 		QKeyEvent ke( QEvent::AccelOverride, keyCodeQt, 0,  state );
 		ke.ignore();
+		
 		g_bActive = true;
+		g_bAccelActivated = false;
 		kapp->sendEvent( kapp->focusWidget(), &ke );
 		g_bActive = false;
-		bool bHandled = g_bAccelActivated;
-		g_bAccelActivated = false;
-		return bHandled;
+		
+		// If the Override event was accepted from a non-KAccel widget,
+		//  then kill the next AccelOverride in KApplication::notify.
+		if( ke.isAccepted() && !g_bAccelActivated )
+			g_bKillAccelOverride = true;
+		
+		// Stop event processing if a KDE accelerator was activated.
+		return g_bAccelActivated;
 	}
 	
 	return false;
@@ -125,6 +134,7 @@ KAccelPrivate::KAccelPrivate( KAccel* pP
 	m_bAutoUpdate = true;
 	connect( (QAccel*)m_pAccel, SIGNAL(activated(int)), this, SLOT(slotKeyPressed(int)) \
);  
+	if( m_pWatch )
 	m_pWatch->installEventFilter( this );
 	KAccelEventHandler::self();
 }
@@ -289,7 +299,7 @@ void KAccelPrivate::slotMenuActivated( i
 
 bool KAccelPrivate::eventFilter( QObject* /*pWatched*/, QEvent* pEvent )
 {
-	if( KAccelEventHandler::active() && pEvent->type() == QEvent::AccelOverride ) {
+	if( KAccelEventHandler::active() && pEvent->type() == QEvent::AccelOverride && \
m_bEnabled ) {  QKeyEvent* pKeyEvent = (QKeyEvent*) pEvent;
 		KKey key( pKeyEvent );
 		kdDebug(125) << "KAccelPrivate::eventFilter( AccelOverride ): this = " << this << \
", key = " << key.toStringInternal() << endl; @@ -302,6 +312,8 @@ bool \
KAccelPrivate::eventFilter( QObject  if( m_mapIDToAction.contains( nID ) ) {
 					// TODO: reduce duplication between here and slotMenuActivated
 					KAccelAction* pAction = m_mapIDToAction[nID];
+					if( !pAction->isEnabled() ) 
+						continue;
 					connect( this, SIGNAL(menuItemActivated()), pAction->objSlotPtr(), \
pAction->methodSlotPtr() );  emit menuItemActivated();
 					disconnect( this, SIGNAL(menuItemActivated()), pAction->objSlotPtr(), \
pAction->methodSlotPtr() );



[prev in list] [next in list] [prev in thread] [next in thread]