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

List:       kde-core-devel
Subject:    Re: Konqueror internals question (hunting an annoying KDE3 usability
From:       Hans Meine <hans_meine () gmx ! net>
Date:       2007-08-13 13:53:09
Message-ID: 200708131553.13974.hans_meine () gmx ! net
[Download RAW message or body]

[Attachment #2 (multipart/mixed)]


On Montag 13 August 2007, Hans Meine wrote:
> Hmm.  I manually applied the diff, but it does not seem to fix the problem.
> Strange.
>
> [...]
>
> My analysis from the last mail still holds - the timer still fires and
> clears the location bar after slotAddTab() has finished. :-(

I propose two alternative fixes which both work for me:

1) in KonqViewManager::showTab (which is called by slotAddTab), stop the timer 
immediately after the call to showPage() (which starts the timer), before 
calling emitActivePartChanged:

--- konq_viewmgr.cc     (Revision 699556)
+++ konq_viewmgr.cc     (Arbeitskopie)
@@ -717,6 +717,9 @@
   if (tabContainer->currentPage() != view->frame())
   {
     tabContainer->showPage( view->frame() );
+    // showPage() indirectly calls setActivePart(..., false);
+    // prevent a second later call to emitActivePartChanged()
+    m_activePartChangedTimer->stop();
     emitActivePartChanged();
   }
 }

2) Alternatively, if I understand correctly, the timer is only used to give 
better user feedback for drag&drop by not calling the possibly-slow 
emitActivePartChanged immediately.  Then, I don't see a reason why it should 
*ever* be called twice, so I added the timer stop() in 
emitActivePartChanged() directly (which makes more sense from an 
encapsulation point of view) and removed it from setActivePart() (where David 
added it in r698562).

--- konq_viewmgr.cc     (Revision 699556)
+++ konq_viewmgr.cc     (Arbeitskopie)
@@ -1369,7 +1369,6 @@
         // This is not done with right-clicking so that the part is activated 
before the
         // popup appears (#75201)
     } else {
-        m_activePartChangedTimer->stop();
         emitActivePartChanged();
     }
 }
@@ -1396,6 +1395,8 @@

 void KonqViewManager::emitActivePartChanged()
 {
+    // prevent unnecessary multiple calls to slotPartActivated:
+    m_activePartChangedTimer->stop();
     m_pMainWindow->slotPartActivated( activePart() );
 }

Both patches are attached, too.

-- 
Ciao, /  /                                                    .o.
     /--/                                                     ..o
    /  / ANS                                                  ooo

["konqueror_dont_clear_locationbar_twice.diff" (text/x-diff)]

Index: konq_viewmgr.cc
===================================================================
--- konq_viewmgr.cc	(Revision 699556)
+++ konq_viewmgr.cc	(Arbeitskopie)
@@ -717,6 +717,9 @@
   if (tabContainer->currentPage() != view->frame())
   {
     tabContainer->showPage( view->frame() );
+    // showPage() indirectly calls setActivePart(..., false);
+    // prevent a second later call to emitActivePartChanged()
+    m_activePartChangedTimer->stop();
     emitActivePartChanged();
   }
 }

["konqueror_dont_clear_locationbar_twice_never.diff" (text/x-diff)]

Index: konq_viewmgr.cc
===================================================================
--- konq_viewmgr.cc	(Revision 699556)
+++ konq_viewmgr.cc	(Arbeitskopie)
@@ -1369,7 +1369,6 @@
         // This is not done with right-clicking so that the part is activated before the
         // popup appears (#75201)
     } else {
-        m_activePartChangedTimer->stop();
         emitActivePartChanged();
     }
 }
@@ -1396,6 +1395,8 @@
 
 void KonqViewManager::emitActivePartChanged()
 {
+    // prevent unnecessary multiple calls to slotPartActivated:
+    m_activePartChangedTimer->stop();
     m_pMainWindow->slotPartActivated( activePart() );
 }
 

["signature.asc" (application/pgp-signature)]

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