[BUG] [FIX] ClickState Recursion (sm) (but not minor)

Karl Ramberg karl.ramberg at chello.se
Sat Aug 7 06:11:43 UTC 2004


Attachment did not show up properly in BFAV
Karl

Peace Jerome wrote:
> 
> The problem:(See also recent bug report)
> 
> MouseClickState>>handleEvent:from:  is nonrecursive
> except at one point where it is processing the first
> mouseup. There it calls handleEvent: with a copy of
> the mouseup event after firing the click selector.
> 
> I've check with a diagnostic and this is definitely a
> recursive call. The same mouseup comes thru a second
> time (the click state has changed. And the recursive
> event glides thru without causing anymore changes.)
> The recursive call releases it back to handleEvent
> with the true flag. It is processed to completion.
> 
> When handleEvent exits the remaining unrecursed event
> is released back to handleEvent with the original
> false flag. Furthur processing is not done and
> handleEvent releases back to its original caller.
> 
> I do not believe this is what was actually intended.
> But I do not understand what the actual intention was
> either.
> 
> Changing the code so that instead of
> 
> self click.
> aHand handleEvent: copiedEvt .
> ^false
> 
> we have just:
> 
> self click.
> ^true
> 
> would seem to do the trick as far as the post
> processing results are concerned.
> 
> Some times the most useful thing you can think to do
> is poke something. Here is the method changed as
> indicated.
> 
> 'From Squeak3.8alpha of ''17 July 2004'' [latest
> update: #5976] on 1 August 2004 at 11:32:23 am'!
> "Change Set:            ClickStateRecurseFix-wiz
> Date:                   1 August 2004
> Author:                 Jerome Peace (wiz)
> 
> This fix removes the recursion from MouseClickState
> event handling.
> 
> It does the most obvious thing and just removes the
> call to handleEvent so that the mouse up event is not
> fired twice. If I am right this should clear up some
> problems and keep mousedown events equal in number to
> mouse up events (or at least eliminate some of the
> errors).
> 
> I have tried it and it does not seem to break
> anything. Caveat: I am mucking here without complete
> knowledge of why this was done this way in the first
> place. There might be something of importance that I
> am ignoring. I am tracking that knowledge down and in
> the mean time proposing this as a testable fix.
> 
> The fix is done on top of ned konz
> SecondClickTimeoutFix-nk so it includes his changes to
> handleEvent:from. It does not include any else so his
> cs is meant to be loaded first. This set only changes
> one method.
> 
> "!
> 
> !MouseClickState methodsFor: 'event handling' stamp:
> 'wiz 8/1/2004 11:19'!
> handleEvent: evt from: aHand
>         "Process the given mouse event to detect a click,
> double-click, or drag.
>         Return true if the event should be processed by the
> sender, false if it shouldn't.
>         NOTE: This method heavily relies on getting *all*
> mouse button events."
>         | localEvt timedOut isDrag |
>         timedOut _ (evt timeStamp - firstClickTime) >
> dblClickTime.
>         localEvt _ evt transformedBy: (clickClient
> transformedFrom: aHand owner).
>         isDrag _ (localEvt position - firstClickDown
> position) r > dragThreshold.
>         clickState == #firstClickDown ifTrue: [
>                 "Careful here - if we had a slow cycle we may have a
> timedOut mouseUp event"
>                 (timedOut and:[localEvt isMouseUp not]) ifTrue:[
>                         "timeout before #mouseUp -> keep waiting for drag
> if requested"
>                         clickState _ #firstClickTimedOut.
>                         dragSelector ifNil:[
>                                 aHand resetClickState.
>                                 self doubleClickTimeout; click "***"].
>                         ^true].
>                 localEvt isMouseUp ifTrue:[
> 
>                         (timedOut or:[dblClickSelector isNil]) ifTrue:[
>                                 self click.
>                                 aHand resetClickState.
>                                 ^true].
>                         "Otherwise transfer to #firstClickUp"
>                         firstClickUp _ evt copy.
>                         clickState _ #firstClickUp.
>                         "If timedOut or the client's not interested in dbl
> clicks get outta here"
>                         self click.
>                         "
>                         aHand handleEvent: firstClickUp.
>                         ^false
>                         "
>                         ^true].
>                 isDrag ifTrue:["drag start"
>                         self doubleClickTimeout. "***"
>                         aHand resetClickState.
>                         dragSelector "If no drag selector send #click
> instead"
>                                 ifNil: [self click]
>                                 ifNotNil: [self drag: firstClickDown].
>                         ^true].
>                 ^false].
> 
>         clickState == #firstClickTimedOut ifTrue:[
>                 localEvt isMouseUp ifTrue:["neither drag nor double
> click"
>                         aHand resetClickState.
>                         self doubleClickTimeout; click. "***"
>                         ^true].
>                 isDrag ifTrue:["drag start"
>                         aHand resetClickState.
>                         self doubleClickTimeout; drag: firstClickDown.
> "***"
>                         ^true].
>                 ^false].
> 
>         clickState = #firstClickUp ifTrue:[
>                 (timedOut) ifTrue:[
>                         "timed out after mouseUp - signal timeout and pass
> the event"
>                         aHand resetClickState.
>                         self doubleClickTimeout. "***"
>                         ^true].
>                 localEvt isMouseDown ifTrue:["double click"
>                         clickState _ #secondClickDown.
>                         ^false]].
> 
>         clickState == #secondClickDown ifTrue: [
>                 timedOut ifTrue:[
>                         "timed out after second mouseDown - pass event
> after signaling timeout"
>                         aHand resetClickState.
>                         self doubleClickTimeout. "***"
>                         ^true].
>                 isDrag ifTrue: ["drag start"
>                         self doubleClickTimeout. "***"
>                         aHand resetClickState.
>                         dragSelector "If no drag selector send #click
> instead"
>                                 ifNil: [self click]
>                                 ifNotNil: [self drag: firstClickDown].
>                         ^true].
>                 localEvt isMouseUp ifTrue: ["double click"
>                         aHand resetClickState.
>                         self doubleClick.
>                         ^false]
>         ].
> 
>         ^true
> ! !
> 
> Attached is the gzipped cs above.
> The second attachment is the gzipped diagnostic change
> set. (See also the full bug report).
> 
> Yours in service, Jerome Peace (wiz)
> 
> 
> 
> __________________________________
> Do you Yahoo!?
> New and Improved Yahoo! Mail - 100MB free storage!
> http://promotions.yahoo.com/new_mail
> 
>   ------------------------------------------------------------------------
>                                          Name: ClickStateRecurseFix-wiz.1.c.gz
>    ClickStateRecurseFix-wiz.1.c.gz       Type: application/gzip (application/gzip)
>                                      Encoding: base64
>                                   Description: ClickStateRecurseFix-wiz.1.c.gz
> 
>                                         Name: ClickStateDiagnostics-wiz.1.gz
>    ClickStateDiagnostics-wiz.1.gz       Type: application/gzip (application/gzip)
>                                     Encoding: base64
>                                  Description: ClickStateDiagnostics-wiz.1.gz
> 
>   ------------------------------------------------------------------------
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ClickStateDiagnostics-wiz.cs.gz
Type: application/octet-stream
Size: 2029 bytes
Desc: Unknown Document
Url : http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20040807/30220f6f/ClickStateDiagnostics-wiz.cs.obj
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ClickStateRecurseFix-wiz.cs.gz
Type: application/octet-stream
Size: 1421 bytes
Desc: Unknown Document
Url : http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20040807/30220f6f/ClickStateRecurseFix-wiz.cs.obj


More information about the Squeak-dev mailing list