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

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: Po Lu <luangruo@yahoo.com>,  stefankangas@gmail.com,  
> 64440@debbugs.gnu.org
> Date: Tue, 05 Sep 2023 11:53:38 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I'd appreciate a walkthrough of the patches with explanations for the
> > significant hunks.  It's a non-trivial change, so I think the
> > rationale and the main ideas of the implementation should be described
> > and discussed.
> 
> Hi,
> 
> For this walkthrough, I have inserted the 4 patches, made some elisions
> and comment inline.

Thanks.

> +  /* 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; i < *h_end; ++i)
> +    *x_end += row->glyphs[TEXT_AREA][i].pixel_width;
> 
> Here, I convert those limits from chars to pixels.

What does this glyph_row look like?  Does it include several strings
one after the other or something?

In general, we always test the index of a glyph in a glyph row against
glyphs[TEXT_AREA].used, and I'm worried that these tests are absent
from the code above.  Which is why I'm asking for more details about
the arrangement of the menu text in these glyph rows.

> +/* 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)
> +{

Is this function under a suitable #if condition?  AFAIU, it is only
appropriate in a build with X but without any toolkit, so it shouldn't
be compiled in other configurations.

> --- a/src/xdisp.c
> +++ b/src/xdisp.c
> @@ -33878,6 +33878,9 @@ show_mouse_face (Mouse_HLInfo *hlinfo, enum 
> draw_glyphs_face draw)
>    if (FRAME_WINDOW_P (f) && NILP (track_mouse))
>      {
>        if (draw == DRAW_NORMAL_TEXT
> +#ifndef HAVE_EXT_MENU_BAR
> +       && !EQ (hlinfo->mouse_face_window, f->menu_bar_window)
> +#endif

Won't this cpp conditional be true in a build --without-x?  Should it?

Thanks for working on this.