Re: [cp-patches] FYI: Stopping the DefaultCaret timer when the component looses the focus.

[Mark Wielaard]

Hi Audrius,

> This should fix the resource leak, stopping the caret blinking timer 
> when the caret looses the focus.
> 2005-11-22  Audrius Meskauskas  <address@hidden>
> 
>         * javax/swing/text/DefaultCaret.java (focusGained):
>         Update timer status. (focusLost): Stop the timer
>         (unless the event is temporary).
>         (updateTimerStatus): New method.
>         (setVisible): Delegate timer management to the
>         updateTimerStatus.
> 

I have some trouble following the logic here. The property visible seems
to be used for multiple things. The blink timer sets and unsets visible
on each tick. But at the same time visible is used in
upstateTimerStatus() to see whether or not to start/stop the Timer.

> +  /**
> +   * Install (if not present) and start the timer, if the caret must blink. 
> The
> +   * caret does not blink if it is invisible, or the component is disabled or
> +   * not editable.
> +   */
> +  private void updateTimerStatus()
> +  {
> +    if (visible && textComponent.isEnabled() && textComponent.isEditable())
> +      {
> +        if (blinkTimer == null)
> +          initBlinkTimer();
> +        if (!blinkTimer.isRunning())
> +          blinkTimer.start();
> +      }
> +    else
> +      {
> +        if (blinkTimer != null)
> +          blinkTimer.stop();
> +      }

Is it correct to check for visible here?
Isn't just isEnabled() && isEditable() the correct check?

I get the feeling that the visible property isn't the correct thing to
change and test for the blinking case. I would expect visible to be
always true and blinkRate > 0 when we have a blinking Caret and another
extra private variable (lets call it showing) to alternate between true
and false when blinking. I might be completely confused of course. A bit
more design documentation would be nice.

Cheers,

Mark

signature.asc
Description: This is a digitally signed message part