Mantis - Squeak
Viewing Issue Advanced Details
6825 Morphic minor always 12-22-07 00:53 12-23-07 04:12
0006825: Squeak Oxymorons: ScrollBar>>waitForDelay1:delay2 --doesn't wait. (It always returns true).
This is another viral bug. Now copied into OmniBrowser as well as morphic.

See the code below which is dgd's most recent version.

It does a lot of work to keep track of the scroll delay but then returns true.

This explains why the scrollbars don't work when you hold the mouse down in the space above or below them.

'dgd 2/21/2003 23:05 ScrollBar waitForDelay1:delay2:'
'di 4/22/2001 18:29 ScrollBar waitForDelay1:delay2:'
'di 8/17/1998 09:38 ScrollBar waitForDelay1:delay2:'

waitForDelay1: delay1 delay2: delay2
    "Return true if an appropriate delay has passed since the last scroll operation.
    The delay decreases exponentially from delay1 to delay2."

    | now scrollDelay |
    timeOfLastScroll isNil ifTrue: [self resetTimer]. "Only needed for old instances"
    now := Time millisecondClockValue.
    (scrollDelay := currentScrollDelay) isNil
        ifTrue: [scrollDelay := delay1 "initial delay"].
    currentScrollDelay := scrollDelay * 9 // 10 max: delay2. "decrease the delay"
    timeOfLastScroll := now.
    ^true [^] (1,375 bytes) 12-23-07 04:06

12-23-07 03:08   
upon further inspection

its not as bad as I thought.
setting currentScrollDelay will change the steptime so the timing calculation has an effect on when the steps are taken.

Still the routine should be structured differently s.t. it does not return a testable value.

Of course the routines that call it would all have to be changed inorder to not test the returned value.

The code calls for a refactoring that changes the name of the routine to something like setDelay etc. and changes the way it is called.
And a rethink of where the responsibilities actually belong.

In an enviorment where the code needs to document itself for a constant stream of new maintainers and code copiers, it really needs to be clear and uncluttered.