Tool-bar buttons unresponsive with tooltips on

Tool-bar buttons unresponsive with tooltips on

[Chong Yidong]

With `emacs -q' and tool-bar-mode on:

  1. Hover the mouse cursor over (e.g.) the "find file" icon
  2. Wait for the tool-tip to pops up.
  3. Click [mouse-1].

Result: the tool-tip is dismissed, but the button isn't clicked.  You
have to click [mouse-1] a second time to launch find-file.

Expected behavior: the [mouse-1] should not only dismisses the
tool-tip, but also click the button.


In GNU Emacs 22.0.50.1 (i686-pc-linux-gnu, X toolkit)
 of 2005-10-13 on furball
X server distributor `The X.Org Foundation', version 11.0.60802000
Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8
  default-enable-multibyte-characters: t

Re: Tool-bar buttons unresponsive with tooltips on

[Chong Yidong]

Chong Yidong <cyd@stupidchicken.com> writes:

> With `emacs -q' and tool-bar-mode on:
>
>   1. Hover the mouse cursor over (e.g.) the "find file" icon
>   2. Wait for the tool-tip to pops up.
>   3. Click [mouse-1].
>
> Result: the tool-tip is dismissed, but the button isn't clicked.  You
> have to click [mouse-1] a second time to launch find-file.
>
> Expected behavior: the [mouse-1] should not only dismisses the
> tool-tip, but also click the button.

This seems to be related to the remember_mouse_glyph changes made a
few days ago.  Reverting this change removes the problem:

2005-10-12  Kim F. Storm  <storm@cua.dk>

	* xdisp.c (remember_mouse_glyph): New generic version based on
	glyph_rect and remember_mouse_glyph from xterm.c enhanced to
	properly handle all different window areas.

	* dispextern.h (remember_mouse_glyph): Add prototype.

	* xterm.c (glyph_rect, remember_mouse_glyph): Remove X versions.
	(note_mouse_movement, XTmouse_position): Use generic
	remember_mouse_glyph, add last_mouse_glyph arg.

	* w32term.c (note_mouse_movement): Fix last_mouse_glyph check.
	(glyph_rect, remember_mouse_glyph): Remove w32 specific versions.
	(note_mouse_movement, w32_mouse_position): Use generic
	remember_mouse_glyph, add last_mouse_glyph arg.

	* macterm.c (note_mouse_movement): Add call to remember_mouse_glyph.
	(glyph_rect, remember_mouse_glyph): Remove mac specific versions.
	(XTmouse_position): Adapt to use generic remember_mouse_glyph
	instead of pixel_to_glyph_coords.

	* window.c (coordinates_in_window): Fix x position for ON_RIGHT_MARGIN.
	Fix x position for ON_TEXT when left margin width > 0.

Re: Tool-bar buttons unresponsive with tooltips on

[Chong Yidong]

This patch fixes the bug.  However, it's pure hackery since I'm not
familiar with the code at all.  Can someone tell me if it makes sense
to do this?



*** emacs/src/xterm.c.~1.884.~	2005-10-17 08:54:26.000000000 -0400
--- emacs/src/xterm.c	2005-10-19 18:45:30.000000000 -0400
***************
*** 3609,3615 ****
        last_mouse_scroll_bar = Qnil;
        note_mouse_highlight (frame, event->x, event->y);
        /* Remember which glyph we're now on.  */
!       remember_mouse_glyph (frame, event->x, event->y, &last_mouse_glyph);
        return 1;
      }
  
--- 3609,3616 ----
        last_mouse_scroll_bar = Qnil;
        note_mouse_highlight (frame, event->x, event->y);
        /* Remember which glyph we're now on.  */
!       if (!NILP (tip_frame) && frame != XFRAME (tip_frame))
! 	remember_mouse_glyph (frame, event->x, event->y, &last_mouse_glyph);
        return 1;
      }

Re: Tool-bar buttons unresponsive with tooltips on

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Wed, 19 Oct 2005 18:49:03 -0400
> 
> This patch fixes the bug.  However, it's pure hackery since I'm not
> familiar with the code at all.  Can someone tell me if it makes sense
> to do this?

I don't have the answer to your question, but the snippet you changed
absolutely needs a comment that would explain its subtleties (as
discussed in this thread).

Re: Tool-bar buttons unresponsive with tooltips on

[Kim F. Storm]

Chong Yidong <cyd@stupidchicken.com> writes:

> This patch fixes the bug.  However, it's pure hackery since I'm not
> familiar with the code at all.  Can someone tell me if it makes sense
> to do this?

It seems like you got the test wrong -- this looks more correct
to me:

      if (NILP (tip_frame) !! frame != XFRAME (tip_frame))
 	remember_mouse_glyph (frame, event->x, event->y, &last_mouse_glyph);

Does it still fix the bug with this change?  If so, feel free to
install the change with a suitable comment explaining why it is
needed.


>
> *** emacs/src/xterm.c.~1.884.~	2005-10-17 08:54:26.000000000 -0400
> --- emacs/src/xterm.c	2005-10-19 18:45:30.000000000 -0400
> ***************
> *** 3609,3615 ****
>         last_mouse_scroll_bar = Qnil;
>         note_mouse_highlight (frame, event->x, event->y);
>         /* Remember which glyph we're now on.  */
> !       remember_mouse_glyph (frame, event->x, event->y, &last_mouse_glyph);
>         return 1;
>       }
>   
> --- 3609,3616 ----
>         last_mouse_scroll_bar = Qnil;
>         note_mouse_highlight (frame, event->x, event->y);
>         /* Remember which glyph we're now on.  */
> !       if (!NILP (tip_frame) && frame != XFRAME (tip_frame))
> ! 	remember_mouse_glyph (frame, event->x, event->y, &last_mouse_glyph);
>         return 1;
>       }

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Tool-bar buttons unresponsive with tooltips on

[Chong Yidong]

> It seems like you got the test wrong

Whoops.

> Does it still fix the bug with this change?

Unfortunately, no.  I'll keep on looking into it.

Re: Tool-bar buttons unresponsive with tooltips on

[Jan D.]

Chong Yidong wrote:
>>It seems like you got the test wrong
> 
> 
> Whoops.
> 
> 
>>Does it still fix the bug with this change?
> 
> 
> Unfortunately, no.  I'll keep on looking into it.
> 

I've checked in this change.  I hope it doesn't ruin the intent of calling 
remember_mouse_glyph in the first place.

	Jan D.


--- xterm.c	20 Oct 2005 15:04:13 -0000	1.885
+++ xterm.c	23 Oct 2005 21:10:30 -0000
@@ -3599,15 +3599,16 @@
        return 1;
      }

+  note_mouse_highlight (frame, event->x, event->y);
+
    /* Has the mouse moved off the glyph it was on at the last sighting?  */
    if (event->x < last_mouse_glyph.x
-	   || event->x >= last_mouse_glyph.x + last_mouse_glyph.width
-	   || event->y < last_mouse_glyph.y
-	   || event->y >= last_mouse_glyph.y + last_mouse_glyph.height)
+      || event->x >= last_mouse_glyph.x + last_mouse_glyph.width
+      || event->y < last_mouse_glyph.y
+      || event->y >= last_mouse_glyph.y + last_mouse_glyph.height)
      {
        frame->mouse_moved = 1;
        last_mouse_scroll_bar = Qnil;
-      note_mouse_highlight (frame, event->x, event->y);
        /* Remember which glyph we're now on.  */
        remember_mouse_glyph (frame, event->x, event->y, &last_mouse_glyph);
        return 1;

Re: Tool-bar buttons unresponsive with tooltips on

[Jason Rumney]

"Jan D." <jan.h.d@swipnet.se> writes:

> I've checked in this change.  I hope it doesn't ruin the intent of
> calling remember_mouse_glyph in the first place.

The if statement is clearly trying to avoid calling note_mouse_highlight
for every pixel of mouse movement, since that was the only substantial
code inside the if statement originally. Moving note_mouse_highlight
outside the if block is certainly ruining the intent of that code.

I don't understand what note_mouse_highlight has to do with being able
to click the mouse on a toolbar button. I think if you can find the
reason why note_mouse_highlight affects that, you will be led to the
real bug.

Re: Tool-bar buttons unresponsive with tooltips on

[Jan D.]

23 okt 2005 kl. 23.54 skrev Jason Rumney:

> "Jan D." <jan.h.d@swipnet.se> writes:
>
>
>> I've checked in this change.  I hope it doesn't ruin the intent of
>> calling remember_mouse_glyph in the first place.
>>
>
> The if statement is clearly trying to avoid calling  
> note_mouse_highlight
> for every pixel of mouse movement, since that was the only substantial
> code inside the if statement originally. Moving note_mouse_highlight
> outside the if block is certainly ruining the intent of that code.
>
> I don't understand what note_mouse_highlight has to do with being able
> to click the mouse on a toolbar button. I think if you can find the
> reason why note_mouse_highlight affects that, you will be led to the
> real bug.
>

I think the intent was to avoid doing frame->mouse_moved = 1, and  
note_mouse_highlight just happened to be there also.

The reason is that showing the tip window generates an EnterNotify  
event that clears dpyinfo->mouse_face_window (sets to Qnil).   
note_mouse_highlight calls note_tool_bar_highlight and that function  
sets mouse_face_window again to the tool bar window.  Later when  
checking if the click is in the tool bar, it is only recognised as a  
tool bar click if mouse_face_window is eq to the tool bar window.

Now, window managers are free to generate EnterNotify whenever they  
want and it seems that KDE generates one more than it used to, and  
also more than other window managers.  This is not a bug however.

     Jan D.

Re: Tool-bar buttons unresponsive with tooltips on

[Jason Rumney]

"Jan D." <jan.h.d@swipnet.se> writes:
>
> The reason is that showing the tip window generates an EnterNotify
> event that clears dpyinfo->mouse_face_window (sets to Qnil).

Why does it do that? tooltip windows should not affect any window
states.

> note_mouse_highlight calls note_tool_bar_highlight and that function
> sets mouse_face_window again to the tool bar window.  Later when
> checking if the click is in the tool bar, it is only recognised as a
> tool bar click if mouse_face_window is eq to the tool bar window.
>
> Now, window managers are free to generate EnterNotify whenever they
> want and it seems that KDE generates one more than it used to, and
> also more than other window managers.  This is not a bug however.

The EnterNotify may not be a bug, but the way we handle it is.
We should test for tip_window and avoid doing certain things, the same
way we do for other events.

Re: Tool-bar buttons unresponsive with tooltips on

[Kim F. Storm]

"Jan D." <jan.h.d@swipnet.se> writes:

> Chong Yidong wrote:
>>>It seems like you got the test wrong
>> Whoops.
>> 
>>>Does it still fix the bug with this change?
>> Unfortunately, no.  I'll keep on looking into it.
>> 
>
> I've checked in this change.  I hope it doesn't ruin the intent of
> calling remember_mouse_glyph in the first place.
>
> 	Jan D.

It certainly ruins the intentions as Jason already pointed out.

Does this patch (vs. current CVS) fix the original problem?


*** xterm.c	24 Oct 2005 00:20:45 +0200	1.886
--- xterm.c	24 Oct 2005 11:08:59 +0200	
***************
*** 249,254 ****
--- 249,255 ----
  /* Where the mouse was last time we reported a mouse event.  */
  
  static XRectangle last_mouse_glyph;
+ static FRAME_PTR last_mouse_glyph_frame;
  static Lisp_Object last_mouse_press_frame;
  
  /* The scroll bar in which the last X motion event occurred.
***************
*** 3596,3616 ****
        frame->mouse_moved = 1;
        last_mouse_scroll_bar = Qnil;
        note_mouse_highlight (frame, -1, -1);
        return 1;
      }
  
-   note_mouse_highlight (frame, event->x, event->y);
  
    /* Has the mouse moved off the glyph it was on at the last sighting?  */
!   if (event->x < last_mouse_glyph.x
        || event->x >= last_mouse_glyph.x + last_mouse_glyph.width
        || event->y < last_mouse_glyph.y
        || event->y >= last_mouse_glyph.y + last_mouse_glyph.height)
      {
        frame->mouse_moved = 1;
        last_mouse_scroll_bar = Qnil;
        /* Remember which glyph we're now on.  */
        remember_mouse_glyph (frame, event->x, event->y, &last_mouse_glyph);
        return 1;
      }
  
--- 3597,3620 ----
        frame->mouse_moved = 1;
        last_mouse_scroll_bar = Qnil;
        note_mouse_highlight (frame, -1, -1);
+       last_mouse_glyph_frame = 0;
        return 1;
      }
  
  
    /* Has the mouse moved off the glyph it was on at the last sighting?  */
!   if (frame != last_mouse_glyph_frame
!       || event->x < last_mouse_glyph.x
        || event->x >= last_mouse_glyph.x + last_mouse_glyph.width
        || event->y < last_mouse_glyph.y
        || event->y >= last_mouse_glyph.y + last_mouse_glyph.height)
      {
        frame->mouse_moved = 1;
        last_mouse_scroll_bar = Qnil;
+       note_mouse_highlight (frame, event->x, event->y);
        /* Remember which glyph we're now on.  */
        remember_mouse_glyph (frame, event->x, event->y, &last_mouse_glyph);
+       last_mouse_glyph_frame = frame;
        return 1;
      }
  
***************
*** 3821,3826 ****
--- 3825,3831 ----
  	       the frame are divided into.  */
  
  	    remember_mouse_glyph (f1, win_x, win_y, &last_mouse_glyph);
+ 	    last_mouse_glyph_frame = f1;
  
  	    *bar_window = Qnil;
  	    *part = 0;
***************
*** 6637,6643 ****
          int tool_bar_p = 0;
  
          bzero (&compose_status, sizeof (compose_status));
! 	bzero (&last_mouse_glyph, sizeof (last_mouse_glyph));
  
          if (dpyinfo->grabbed
              && last_mouse_frame
--- 6642,6648 ----
          int tool_bar_p = 0;
  
          bzero (&compose_status, sizeof (compose_status));
! 	last_mouse_glyph_frame = 0;
  
          if (dpyinfo->grabbed
              && last_mouse_frame

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Tool-bar buttons unresponsive with tooltips on

[Chong Yidong]

> Does this patch (vs. current CVS) fix the original problem?

Yes it does.  Thanks.

Re: Tool-bar buttons unresponsive with tooltips on

[Kim F. Storm]

Chong Yidong <cyd@stupidchicken.com> writes:

>> Does this patch (vs. current CVS) fix the original problem?
>
> Yes it does.  Thanks.

Good.  I have installed the patch for X.

Maybe a similar patch is needed on w32 and/or mac?

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Tool-bar buttons unresponsive with tooltips on

[Jan D.]

> "Jan D." <jan.h.d@swipnet.se> writes:
>
>>
>> The reason is that showing the tip window generates an EnterNotify
>> event that clears dpyinfo->mouse_face_window (sets to Qnil).
>>
>
> Why does it do that? tooltip windows should not affect any window
> states.

Probably an old leftover.  There should be a C variable to identify  
tip frames, like the hourglass_p variable.

>
>
>> note_mouse_highlight calls note_tool_bar_highlight and that function
>> sets mouse_face_window again to the tool bar window.  Later when
>> checking if the click is in the tool bar, it is only recognised as a
>> tool bar click if mouse_face_window is eq to the tool bar window.
>>
>> Now, window managers are free to generate EnterNotify whenever they
>> want and it seems that KDE generates one more than it used to, and
>> also more than other window managers.  This is not a bug however.
>>
>
> The EnterNotify may not be a bug, but the way we handle it is.
> We should test for tip_window and avoid doing certain things, the same
> way we do for other events.

Kim checked in another fix that works.

     Jan D.