Bug 48888

Subject: Re:  New: click on a folder and then move the mouse may selects wrong folder

On Tuesday 08 October 2002 20:29, Roger Larsson wrote:
> ------- You are receiving this mail because: -------
> You are the assignee for the bug, or are watching the assignee.
>
> http://bugs.kde.org/show_bug.cgi?id=48888
>            Summary: click on a folder and then move the mouse may
> selects wrong folder
>            Product: kmail
>            Version: 1.4.7
>           Platform: Compiled Sources
>         OS/Version: Linux
>             Status: UNCONFIRMED
>           Severity: normal
>           Priority: NOR
>          Component: general
>         AssignedTo: kmail@kde.org
>         ReportedBy: roger.larsson@norran.net
>
>
> Version:           1.4.7 (using KDE 3.0.8 (KDE 3.1 beta2))
> Installed from:    compiled sources
> Compiler:          gcc version 2.95.3 20010315 (SuSE)
> OS:          Linux (i686) release 2.4.18-4GB
>
> Now and then when I move the mouse to click on a folder
> and then immediately move on to the messages area the wrong
> folder will be selected (often happens when clicking on a big
> folder that takes time to open)
>
> It looks like it first selects the correct folder - but then
> move on to a folder that has the same screen row as where
> the mouse finally stops.

This is a bug in QListView. It's reproducable with the listview example 
program which is included in the Qt sources. I already send a bug 
report to the Trolls (between 21.07. and 27.07. I exchanged five 
messages with Anders Bakken) but they said "Sorry. Nothing we can do 
about it I am afraid.". For the last message, which I sent after I had 
investigated the problem a little bit more, I never received a reply.

Regards,
Ingo

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.0 (GNU/Linux)

iD8DBQE9o2RqGnR+RTDgudgRAtK3AKCSmt/Oj/IQ0RxKMVDIVjnWU+eNpgCgnBAn
Gmo1k6pVwgUDvQDidZuacmU=
=Kr4s
-----END PGP SIGNATURE-----

Subject: Re:  click on a folder and then move the mouse may selects wrong folder

But it _does_ look like it does the right thing at the beginning.
And then suddenly decides that "I should use the current mouse position"

Does kmail get double events?

Subject: Re:  click on a folder and then move the mouse may selects wrong folder

On Wednesday 09 October 2002 08:34, Roger Larsson wrote:
> But it _does_ look like it does the right thing at the beginning.
> And then suddenly decides that "I should use the current mouse
> position"
>
> Does kmail get double events?

Exactly. The following is what happens when you quickly move the mouse 
after you selected a folder:

Well, I did some investigation (added some qDebugs to qlistview.cpp and
to listviews.cpp) and tracked down the problem a little bit. This is a
sample debug output (with comments added):
=====
QListView::contentsMousePressEventEx()
[this is called by QListView::contentsMousePressEvent() because the left
mouse button has been pressed. this is o.k.]

QListView::contentsMousePressEventEx().2 - calling setCurrentItem(i)
[the item I clicked on is set as current item. good.]

QListView::setCurrentItem(...) - emitting selectionChanged(...) signal
[the selectionChanged signal is emitted. good.]

ListViews::slotFolderChanged(Sub Sub Folder 2)
[this slot is connected to the selectionChanged signal. also o.k.]

QListView::contentsMouseMoveEvent() - calling doAutoScroll()
[this function is called because of the mouse movements. by why is
doAutoScroll() called? this is the corresponding code:
    // if we don't need to autoscroll
    if ( !needAutoScroll ) {
        // if there is a autoscroll timer, delete it
        if ( d->scrollTimer ) {
            disconnect( d->scrollTimer, SIGNAL(timeout()),
                        this, SLOT(doAutoScroll()) );
            d->scrollTimer->stop();
            delete d->scrollTimer;
            d->scrollTimer = 0;
        }
        // call this to select an item
 qDebug("QListView::contentsMouseMoveEvent() - calling doAutoScroll()");
        doAutoScroll();
    }

It's a little bit odd that doAutoScroll() is called just to "select an
item".
]

QListView::contentsMouseMoveEvent() - calling doAutoScroll()
[the mouse is still moving]

QListView::contentsMouseMoveEvent() - calling doAutoScroll()
[the mouse is still moving]

QListView::doAutoScroll() - calling setCurrentItem(c)
[this should probably not happen. this call to setCurrentItem(c) causes
the wrong selection of the item the mouse pointer points to after
moving it.]

QListView::setCurrentItem(...) - emitting selectionChanged(...) signal
[this shouldn't happen]

ListViews::slotFolderChanged(Sub Folder 2)
[the wrong item is selected]

QListView::contentsMouseReleaseEvent()
[the mouse button has already been released long ago. this slot is
called way too late.]
=====

Regards,
Ingo

connecting with "currentChanged" proved to be to hot.  
It is issued before the mouse release event is processed.  
And since the process took some time. The mouse movements  
got processed before the release - kind of strange...  
But if it is connected with "clicked" instead it works much  
better - I do have to test it some more... but it looks promising.  
   
Index: kmfoldertree.cpp   
===================================================================   
RCS file: /cvs/kdenetwork/kmail/kmfoldertree.cpp,v   
retrieving revision 1.203   
diff -u -3 -p -r1.203 kmfoldertree.cpp   
--- kmfoldertree.cpp    2002/09/12 19:34:55     1.203   
+++ kmfoldertree.cpp    2002/10/09 22:55:53   
@@ -129,7 +129,7 @@ void KMFolderTree::connectSignals()   
   connect(&mUpdateTimer, SIGNAL(timeout()),   
           this, SLOT(delayedUpdate()));   
   
-  connect(this, SIGNAL(currentChanged(QListViewItem*)),   
+  connect(this, SIGNAL(clicked(QListViewItem*)),   
          this, SLOT(doFolderSelected(QListViewItem*)));   
   
   connect(kernel->folderMgr(), SIGNAL(changed()),   
   

I think that the earlier patch can solve some other problems too...   
* "currentChanged" is often disconnected from "doFolderSelected"   
* then something is done   
* and the connection is reestablished.   
It does not look necessary any more.   
   
This bug might exist in kmmainwin.cpp and kmheaders.cpp too...   
But since the necessary operation in much faster, but the window   
to hit the bug is much smaller.   
  
Other places in KDE?  

The workaround is removed from kmail sources.     
  Gave other problems according to Ingo Kl

Got a message from Trolltech, on the 25 Oct.   
"It is properly fixed for the next release now."   
  
But qt-copy is still from 21 Oct. 
   

qt-copy updated Oct 30 fixes the Qt part of this problem.   
I can no longer reproduce it with their listviews example.  
(sleep added in selection to simulate some workload) 
  
But KMail behaves strange   
* Standing on a directory 
* klicking on a sibling above 
* moving up to a sub directory (not clicking on it) 
=> selection moves down! 
  
- does KMail have a workaround that now should be removed?    
(kmail from the same date, i.e. later than rc1)  

Subject: Re: [Issue N9282]  [QTBUG] in QListView (Was: Re: [PATCH for Bug 48888] Re: two kmail problems)

On Friday 25 October 2002 15.53, you wrote:
> On Friday, 25 Oct 2002 15:17 Roger Larsson wrote:
> > On Friday 25 October 2002 10.53, you wrote:
> > > On Friday, 25 Oct 2002 0:53 Roger Larsson wrote:
> > > > Played a little more with it...
> > > >
> > > > You ARE on the right track...
> > > >
> > > > The case when it moves to "Folder 1" seems to be an exception.
> > > > Usually it works as intended.
> >
> > But why does it sometimes get lost and end up at "Folder 1"?
> > Another problem that I found with the patched code:
> > * Click on "Folder 19" (selection moves)
> > * Click and hold on "Folder 19"
> > * Hold while moving to "Folder 14", no update...
> > * Release (on "Folder 14")
> > * Suprise "Folder 11" gets selected!
> 
> Both problems were a bug in the logic of the fix. It is fixed properly
> now for 3.0.7 and 3.1 final.

I am sorry but I am not that sure anymore...

KMail does not work perfectly for me - not much better infact.
[When I had a patch that did not use currentChanged but instead used clicked,
it worked much better]

Anyway I put a printout of the folder name in KMFolderTree::doFolderSelected
and noticed that I can get the moving selections.
(I click on one but move on with the mouse => currentChanged on the clicked
 AND currentChanged on the position where I stop the cursor)

Put a breakpoint there too. And repeated (klicked on one folder and moved the 
mouse)

Breakpoint got hit. Looked normal. (first stack)
Continued.
Hit again?!
Got hit from a MouseMovement?!
Moved up to see what was up. Some notices.
d->buttonDown was yes (as with last report)
e->button() was NoButton, but
e->state() was LeftButton (How come?)
In QListView::contentsMouseMoveEvent needAutoScroll was FALSE
but why does it do one anyway?

    // if we don't need to autoscroll
    if ( !needAutoScroll ) {
	// if there is a autoscroll timer, delete it
	if ( d->scrollTimer ) {
	    disconnect( d->scrollTimer, SIGNAL(timeout()),
			this, SLOT(doAutoScroll()) );
	    d->scrollTimer->stop();
	    delete d->scrollTimer;
	    d->scrollTimer = 0;
	}
	// call this to select an item
=>	doAutoScroll();
    }

Why is an autoscroll done if it is not needed???

Then the nightly update started and crashed my gdb - since I have too little 
swap...

This was mostly a random dump since it is time for some sleep...
(To make sure that it works as described I updated qt-copy,
recompiled, and retested)

/RogerL



Created an attachment (id=398)
listview.bug

Created attachment 400 [details]
Visualization on how to provoke the bug

I have found a way to reliably reproduce this bug with 3.1rc2+

Click on the red dot, hold and drag (on the same item), release and move the
mouse
over another folder.

Result: the final folder opens (in this case) "embedded"

 This is very annoying.  It looks to be a Qt bug for sure.  David Faure has a 
patch for Qt.  Did it get accepted?  What's the status here? 

Created attachment 430 [details]
Alternative qt patch

This is an alternative patch [Davids patch did only removed the extra
autoScroll(); ]
With this you can select another folder by holding the left mouse button down.
But KMail will need to disable multi select on the items...

*** Bug 50733 has been marked as a duplicate of this bug. ***

Subject: Solved? (Re: Fwd: Re: [Issue N10289]  QListView selects wrong item between press and release)

OK
I think I have found a way to solve this...

First I noticed that the SelectionMode had to be Extended or Multi
to enter the really bad parts in qlistview::doAutoScroll

But even after modifying kfoldertree and klistview I could provoke the bug.
(In this case the code is virtually identical to my patch)

But with the qt-copy patch, that verifies that buttons are still down before
continuing with doAutoScroll, I have not been able to provoke the bug.

I have not tried to provoke the bug on qt listviews example yet.

What has to be done is to add some stuff that takes time
probably on selectionChanged ...
I will try to find a way.

Anyway:
  I think klistview.patch fixes a potential problem.
  I do not understand why kfoldertree needs to be Extended, do you know?
  (maybe to get consistent behavor when klistview is not corrected...)

  And yes, I do think the qt-copy patch corrects a bug too. And most likely
  other users of QCursor::pos should check if the button assumed to be down
  is still down... [but since Qt-3.1.0 is out already, the two other might be
  OK as they improve the situation]

/RogerL

Created an attachment (id=477)
kfoldertree.patch

Created an attachment (id=478)
klistview.patch

Created an attachment (id=479)
qt-copy.patch

I have been running with the second (2002-11-18) qt-copy patch for some time now. 
And it works - even if it is kind of ugly... 

*** Bug 52392 has been marked as a duplicate of this bug. ***

Subject: Re:  click on a folder and then move the mouse may selects wrong folder

Fixed in HEAD.

Created attachment 709 [details]
Patch from Trolltech

The patch was described like this. I have not tested it yet. But
it looks OK.
One thing to take a look at is who uses the old
doAutoScroll. It might be there for binary compatibility only,
then those users should be corrected...


"Ok this should now be fixed (attached diff for current 3.1).

The problem was as you said that if a slot is slow at executing, the
curser pos when doAutoScroll is called is not the correct pos to use.

Now contentsMouseMoveEvent calls a private doAutoScroll( const QPoint& )
(which is also used by the protected slot) with the correct pos."

*** Bug 52989 has been marked as a duplicate of this bug. ***

*** Bug 53524 has been marked as a duplicate of this bug. ***

I am sorry to say, that the problem is NOT gone! I still have the problem with KMail 1.5 from 
the KDE 3.1 final release :-( Or did the fix not make it to that release yet? 

TrollTech patch was added to qt-copy 2003-02-02, expect it in later _Qt_ releases. 

*** Bug 54197 has been marked as a duplicate of this bug. ***

*** Bug 54778 has been marked as a duplicate of this bug. ***

*** Bug 54778 has been marked as a duplicate of this bug. ***

Subject: Re:  click on a folder and then move the mouse may selects wrong folder

Hey I understood. Not necesary to send me this email each three days, then the 
bug seems to be closed with the modify of qt 3.1.1 on CVS.
Gianni
Alle 01:17, venerd

*** Bug 58234 has been marked as a duplicate of this bug. ***