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

List:       kde-core-devel
Subject:    Re: [PATCH] KAcceleratorManager is broken wrt QDockWidget
From:       "Friedrich W. H. Kossebau" <kossebau () kde ! org>
Date:       2009-01-03 13:58:02
Message-ID: 200901031458.03383.kossebau () kde ! org
[Download RAW message or body]

Hi,

noone feels to act on this, many still on holiday? Lucky them :)

BTW, this effects almost all applications using multiple QDockWidgets, e.g. 
Dolphin, Lokalize, Parley, and Okteta. When QDockWidgets get tabbed by the 
user. Just try yourself, stack one QDockWidgets onto the other and see the 
cpu usage rising high as long as they are staying stacked (given that 
AutoCheckAccelerators=true, as default, see below).
KOffice programs are save, because their docked tools use their own 
QDockWidget titel, which is not handled by KAcceleratorManager.

Patch discussed below \/

Am Mittwoch, 31. Dezember 2008, um 02:32 Uhr, schrieb Friedrich W. H. 
Kossebau:
> Hi,
>
> who is experienced with KAcceleratorManager & friends? I think I have found
> a problem with regard to QDockWidgets:
>
> The title of a QDockWidget appears in up to three places:
> 1) title bar
> 2) tab bar title
> 3) toggle action menu entry
>
> A) endless loop due to automatically reset texts
> Currently KAcceleratorManager gives all three different accelerators - but
> as on a title change the QDockWidget updates it's toggle action and also
> the tab title, this leads to an endless loop, if AutoCheckAccelerators=true
> (see below).
> Can be even seen by executing "QT_FLUSH_PAINT=1 okteta" and stacking two or
> more tool views onto each other: the QDockWidget tabbar will now flash all
> the time. Does not block the program, but steals cpu time also without
> QT_FLUSH_PAINT set, up to 50 % if tools are visible.
>
> B) Tab vs titlebar
> If a tabbed QDockWidget is visible, both it's title bar and the tab are
> shown, including their shortcut.
> Should it be the same? Should it be different?
> Should one shortcut be omitted? Which one?
> How to have consistency with untabbed QDockWidget, which always show their
> title bar including the focus shortcut?
>
> C) Accelerator also with title of floating widgets
> The title bar also receives a accelerator if a the dock widget is undocked,
> i.e. floating. I do not think this is expected.
>
> How to solve all of this? I have no real idea (at least at 2.30 in the
> night), so no patch.
>
> But at least I learned about Dr. Klash:
> Edit your kdeglobals (`kde-config --localprefix`share/config/kdeglobals)
> and check your own program with adding a section
> 	[Development]
> 	CheckAccelerators=F12
> 	# default true
> 	AutoCheckAccelerators=false
> 	#default false
> 	AlwaysShowCheckAccelerators=false
> See
> http://techbase.kde.org/Development/Tools#Internationalization_.28i18n.29_T
>ools

The attached patch tries to solve the problem by dropping those QTabBars from 
accelerator setting which stack the QDockWidgets. Not perfect, but works 
better than before, i.e. no endless loop for accelerator setting. The tab 
texts get the accelerators from the QDockWidgets title, also the hidden ones. 

Problem is: How to precisely detect if the QTabBar is the one handling the 
QDockWidgets? The test, as done in the patch, for the parent being of 
QMainWindow type might perhaps effect other QTabBars as well? What chance is 
there to meet such examples in real life?

Your comments, please. I will commit sunday evening, otherwise :)

Cheers
Friedrich
-- 
Okteta - KDE 4 Hex Editor - http://utils.kde.org/projects/okteta

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

Index: kdelibs/kdeui/shortcuts/kacceleratormanager.cpp
===================================================================
--- kdelibs/kdeui/shortcuts/kacceleratormanager.cpp	(Revision 903572)
+++ kdelibs/kdeui/shortcuts/kacceleratormanager.cpp	(Arbeitskopie)
@@ -20,6 +20,7 @@
 #include "kacceleratormanager.h"
 
 #include <QtGui/QApplication>
+#include <QtGui/QMainWindow>
 #include <QtGui/QCheckBox>
 #include <QtGui/QComboBox>
 #include <QtGui/QGroupBox>
@@ -393,6 +394,17 @@
 
 void KAcceleratorManagerPrivate::manageTabBar(QTabBar *bar, Item *item)
 {
+  // ignore QTabBar for QDockWidgets, because QDockWidget on its title change
+  // also updates its tabbar entry, so on the next run of KCheckAccelerators
+  // this looks like a conflict and triggers a new reset of the shortcuts -> endless \
loop +  QWidget* parentWidget = bar->parentWidget();
+  if( parentWidget )
+  {
+    QMainWindow* mainWindow = qobject_cast<QMainWindow*>(parentWidget);
+    if( mainWindow ) // && (mainWindow->layout()->indexOf(bar) != -1)) \
QMainWindowLayout lacks proper support  +      return;
+  }
+
   for (int i=0; i<bar->count(); i++)
   {
     QString content = bar->tabText(i);
@@ -416,7 +428,7 @@
     Item *it = new Item;
     item->addChild(it);
     it->m_widget = dock;
-    it->m_content = KAccelString(content);
+    it->m_content = KAccelString(content, KAccelManagerAlgorithm::STANDARD_ACCEL);
 }
 
 



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