Subject: Re: bug#64440: 30.0.50; [PATCH] Highlight on non toolkit menu bar items

From: Manuel Giraud <[email-masked]>
Date: Mon, 11 Sep 2023 22:51:31 +0200

Eli Zaretskii <[email-masked]> writes:

>> From: Manuel Giraud <[email-masked]>
>> Cc: [email-masked],  [email-masked],  [email-masked]
>> Date: Mon, 11 Sep 2023 20:56:37 +0200
>> 
>> Eli Zaretskii <[email-masked]> writes:
>> 
>> [...]
>> 
>> > Thanks.  That's what I imagined we have there.  So I guess considering
>> > that a string ends where is SCHARS end is reasonable.
>> >
>> > But note that the above means you could also detect where each item
>> > ends by looking for the glyph whose string position is -1.  So maybe
>> > add an assertion there that the glyph after the last character has its
>> > position as -1, in case we could have some complications there with
>> > double-width characters or something.
>> 
>> What do you think of this:
>> 
>> +  /* Convert to pixels bounds.  */
>> +  row = MATRIX_ROW (w->current_matrix, *vpos);
>> +  *x_start = 0;
>> +  for (i = 0; i < *h_start; ++i)
>> +    *x_start += row->glyphs[TEXT_AREA][i].pixel_width;
>> +
>> +  *x_end = *x_start;
>> +  for (i = *h_start; CHARPOS (row->glyphs[TEXT_AREA][i]) != -1; ++i)
>> +    *x_end += row->glyphs[TEXT_AREA][i].pixel_width;
>
> That's OK, but I think we should also make sure 'i' never exceeds the
> value row->used[TEXT_AREA] - 1.  This is only important for the last
> item, but still.
>
> And I would also add an assertion that row->reversed_p is false.  We
> don't currently support R2L menu bars, but if we ever do, the above
> loops should go backwards in such glyph rows.

Ok.  Here is the full version of the second patch:

>From 4420fde7757a9b02087c4330c6102cb40c9e4038 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <[email-masked]>
Date: Mon, 3 Jul 2023 17:35:06 +0200
Subject: [PATCH 2/2] Highlight on non toolkit menu bar items

* src/xdisp.c (get_menu_bar_item, note_menu_bar_highlight): New
functions to highlight item in the menu-bar.
(note_mouse_highlight): Use it.
---
 src/xdisp.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 135 insertions(+)

diff --git a/src/xdisp.c b/src/xdisp.c
index ca7e3b9743e..b194837e69c 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -13922,6 +13922,131 @@ update_menu_bar (struct frame *f, bool save_match_data, bool hooks_run)
   return hooks_run;
 }
 
+#if defined HAVE_WINDOW_SYSTEM && !defined HAVE_EXT_MENU_BAR
+
+/* Get information about the menu-bar item at position X/Y on frame F.
+   Return menu-bar's item char position in H_START/H_END and pixel
+   position in X_START/X_END.  Value is
+
+   -1	if X/Y is not on a menu-bar item
+   0	if X/Y is on the same item that was highlighted before.
+   1	otherwise.  */
+
+static int
+get_menu_bar_item (struct frame *f, int x, int y, int *h_start, int *h_end,
+		   int *x_start, int *x_end, int *vpos)
+{
+  Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f);
+  struct window *w = XWINDOW (f->menu_bar_window);
+  struct glyph_row *row;
+  int dummy;
+  Lisp_Object items;
+  int i;
+
+  /* Find glyph's hpos and vpos under X/Y.  */
+  if (x_y_to_hpos_vpos (w, x, y, h_start, vpos, NULL, NULL, &dummy) == NULL)
+    return -1;
+
+  /* We do not support R2L menu bar.  */
+  row = MATRIX_ROW (w->current_matrix, *vpos);
+  if (row->reversed_p)
+    return -1;
+
+  /* Compute h_start and h_end for this menu bar item.  */
+  items = FRAME_MENU_BAR_ITEMS (f);
+  for (i = 0; i < ASIZE (items); i += 4)
+    {
+      Lisp_Object pos, string;
+      string = AREF (items, i + 1);
+      pos = AREF (items, i + 3);
+      if (NILP (string))
+	return -1;
+      if (*h_start >= XFIXNUM (pos)
+	  && *h_start < XFIXNUM (pos) + SCHARS (string))
+	{
+	  *h_start = XFIXNUM (pos);
+	  *h_end = *h_start + SCHARS (string);
+	  break;
+	}
+    }
+
+  /* Convert to pixels bounds.  */
+  *x_start = 0;
+  for (i = 0; i < *h_start; ++i)
+    *x_start += row->glyphs[TEXT_AREA][i].pixel_width;
+
+  *x_end = *x_start;
+  for (i = *h_start;
+       CHARPOS (row->glyphs[TEXT_AREA][i]) != -1
+	 && i < row->used[TEXT_AREA];
+       ++i)
+    *x_end += row->glyphs[TEXT_AREA][i].pixel_width;
+
+  /* Is mouse on the highlighted item?  */
+  if (EQ (f->menu_bar_window, hlinfo->mouse_face_window)
+      && *vpos >= hlinfo->mouse_face_beg_row
+      && *vpos <= hlinfo->mouse_face_end_row
+      && (*vpos > hlinfo->mouse_face_beg_row
+	  || *h_start >= hlinfo->mouse_face_beg_col)
+      && (*vpos < hlinfo->mouse_face_end_row
+	  || *h_end < hlinfo->mouse_face_end_col
+	  || hlinfo->mouse_face_past_end))
+    return 0;
+
+  return 1;
+}
+
+/* Possibly highlight a menu-bar item on frame F when mouse moves to
+   menu-bar window-relative coordinates X/Y.  Called from
+   note_mouse_highlight.  */
+
+static void
+note_menu_bar_highlight (struct frame *f, int x, int y)
+{
+  Lisp_Object window = f->menu_bar_window;
+  Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f);
+  int h_start, h_end, vpos, x_start, x_end;
+  int rc;
+
+  /* Function note_mouse_highlight is called with negative X/Y
+     values when mouse moves outside of the frame.  */
+  if (x <= 0 || y <= 0)
+    {
+      clear_mouse_face (hlinfo);
+      return;
+    }
+
+  h_start = h_end = 0;
+  rc = get_menu_bar_item (f, x, y, &h_start, &h_end, &x_start, &x_end, &vpos);
+  if (rc < 0)
+    {
+      /* Not on menu-bar item.  */
+      clear_mouse_face (hlinfo);
+      return;
+    }
+  else if (rc == 0)
+    /* On same menu-bar item as before.  */
+    return;
+
+  if (!NILP (Vmouse_highlight))
+    {
+      /* Record this as the current active region.  */
+      hlinfo->mouse_face_beg_col = h_start;
+      hlinfo->mouse_face_beg_row = vpos;
+      hlinfo->mouse_face_beg_x = x_start;
+      hlinfo->mouse_face_past_end = false;
+
+      hlinfo->mouse_face_end_col = h_end;
+      hlinfo->mouse_face_end_row = vpos;
+      hlinfo->mouse_face_end_x = x_end;
+      hlinfo->mouse_face_window = window;
+      hlinfo->mouse_face_face_id = MENU_FACE_ID;
+
+      /* Display it as active.  */
+      show_mouse_face (hlinfo, DRAW_MOUSE_FACE);
+    }
+}
+#endif
 
 
 /***********************************************************************
@@ -35538,6 +35663,16 @@ note_mouse_highlight (struct frame *f, int x, int y)
   w = XWINDOW (window);
   frame_to_window_pixel_xy (w, &x, &y);
 
+#if defined HAVE_WINDOW_SYSTEM && !defined HAVE_EXT_MENU_BAR
+  /* Handle menu-bar window differently since it doesn't display a
+     buffer.  */
+  if (EQ (window, f->menu_bar_window))
+    {
+      note_menu_bar_highlight (f, x, y);
+      return;
+    }
+#endif
+
 #if defined (HAVE_WINDOW_SYSTEM)
   /* Handle tab-bar window differently since it doesn't display a
      buffer.  */
-- 
2.40.0

-- 
Manuel Giraud