bug#75219: 31.0.50; mouse-2 mode-line binding overridden by mouse-1-click-follows-link

[Eli Zaretskii <eliz@gnu.org>]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: visuweshm@gmail.com,  75219@debbugs.gnu.org
> Date: Sun, 05 Jan 2025 16:51:03 -0500
> 
> > Also, for clicks on mode line there will be no buffer position in the
> > event, and then it sounds like your patch will again fall back on
> > returning point?  I thought that clicks on the mode line should
> > examine local keymaps only on the mode-line's string at the click,
> > don't you agree?
> 
> +1

Wait a minute... we already have in current-active-maps code to
support this, right below the call to click_position:

      if (CONSP (position))
	{
	  Lisp_Object string = POSN_STRING (position);

	  /* For a mouse click, get the local text-property keymap
	     of the place clicked on, rather than point.  */

	  if (POSN_INBUFFER_P (position))
	    {
	      Lisp_Object pos = POSN_BUFFER_POSN (position);
	      if (FIXNUMP (pos)
		  && XFIXNUM (pos) >= BEG && XFIXNUM (pos) <= Z)
		{
		  local_map = get_local_map (XFIXNUM (pos),
					     current_buffer, Qlocal_map);

		  keymap = get_local_map (XFIXNUM (pos),
					  current_buffer, Qkeymap);
		}
	    }

	  /* If on a mode line string with a local keymap,
	     or for a click on a string, i.e. overlay string or a
	     string displayed via the `display' property,
	     consider `local-map' and `keymap' properties of
	     that string.  */

	  if (CONSP (string) && STRINGP (XCAR (string)))
	    {
	      Lisp_Object pos = XCDR (string);
	      string = XCAR (string);
	      if (FIXNUMP (pos)
		  && XFIXNUM (pos) >= 0
		  && XFIXNUM (pos) < SCHARS (string))
		{
		  Lisp_Object map = Fget_text_property (pos, Qlocal_map, string);
		  if (!NILP (map))
		    local_map = map;

		  map = Fget_text_property (pos, Qkeymap, string);
		  if (!NILP (map))
		    keymap = map;
		}
	    }

	}

The "STRINGP (XCAR (string))" case is ours.  The problem there is that
it overrides the previous values of local_map and keymap only if the
corresponding properties on the string are non-nil.  I think we should
override them unconditionally, since the values calculated from point
are irrelevant when the click was on a mode line.  And indeed, with
the patch below (which basically does the same with properties on a
string as we already do with properties on buffer text), the bug is
solved.

My only hesitation is whether we should do this with _any_ string or
only with mode-line and header-line strings.  If the string is a
display string or an overlay string, and the keymap properties of that
string at the click are nil, should we fall back on the keymap
properties at point, or should we ignore the buffer properties and
behave as if there are no keymap properties at the click?

diff --git a/src/keymap.c b/src/keymap.c
index c0f49a7..4defe3a 100644
--- a/src/keymap.c
+++ b/src/keymap.c
@@ -1745,13 +1745,8 @@ DEFUN ("current-active-maps", Fcurrent_active_maps, Scurrent_active_maps,
 		  && XFIXNUM (pos) >= 0
 		  && XFIXNUM (pos) < SCHARS (string))
 		{
-		  Lisp_Object map = Fget_text_property (pos, Qlocal_map, string);
-		  if (!NILP (map))
-		    local_map = map;
-
-		  map = Fget_text_property (pos, Qkeymap, string);
-		  if (!NILP (map))
-		    keymap = map;
+		  local_map = Fget_text_property (pos, Qlocal_map, string);
+		  keymap = Fget_text_property (pos, Qkeymap, string);
 		}
 	    }