Subject: Re: bug#76620: 30.1.50; mouse-1 mode-line bindings are unusable when point is on a button

From: Spencer Baugh <[email-masked]>
Date: Mon, 08 Sep 2025 16:49:59 -0400

Stefan Monnier <[email-masked]> writes:
>>> I'm not sure I understand the logic of this change, so please
>>> elaborate, by showing the form(s) of POSITION for which the current
>>> code doesn't work, and please explain how this patch fixes that.
>> When you click on a part of the mode line which is after the end of
>> mode-line-format, (at least on GNU/Linux) you get an event like this:
>>
>> (down-mouse-1 (#<window 3 on *Bugs*> mode-line (2379 . 2071) 21536370
>>  nil nil (157 . 88) nil (8 . 19) (15 . 32)))
>>
>> posn-string is nil for this but it's still in the mode-line.  My change
>> checks for whether a position is in the mode-line before checking if
>> posn-string is nil, so it catches such events.
>
> I suspect this answer (in one way or another) should find its way either
> into a comment or into the commit message.
> [ I tend to have that impression for most questions that occur during
>   a code review, tho, so maybe it's just me.  ]

I agree, I added it to the commit message.

> I have a more demanding request: any chance we could concoct an ERT test
> for that?  I realize that it might be difficult, but we keep skipping
> such "UI" tests because they're difficult, so I think we should try to
> slowly get closer to having an actual test suite of the UI elements.
> If you can identify one obstacle that makes it currently impossible,
> maybe we can fix that obstacle, and thus make one step in that direction.

A very reasonable request, I think.  How about the test in the attached
patch?  It's not really an end-to-end test, but I think it's still
useful.

In fact, while writing this test, I noticed another bug in this code,
which I've now also fixed.  Good call :)

The test would be improved if we had a nice way to generate click events
from Lisp.  Then I could write some code which does "click on the mode
line" and test that that has the right behavior.  But I have no idea how
to do that, is there even a way?

>From 40881a88b06f3f6bbda2f48b7be908ef49e02d74 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <[email-masked]>
Date: Thu, 28 Aug 2025 14:13:24 -0400
Subject: [PATCH] Ignore keymaps at point for positions outside the buffer

Correct a few edge cases where we used the keymaps at point when
looking up keymaps for an event position which is outside the
current buffer.  Namely:

- Clicking on a part of the mode line which is after the end of
  mode-line-format produces an event with non-nil posn-area but
  nil posn-string.

- Even if posn-string doesn't have a local keymap, we should
  still ignore the keymaps at point if posn-string is non-nil.

* src/keymap.c (Fcurrent_active_maps): Ignore keymaps at point
for more positions outside the buffer.  (bug#76620)
---
 src/keymap.c             | 38 ++++++++++++++++----------------------
 test/src/keymap-tests.el | 27 +++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 22 deletions(-)

diff --git a/src/keymap.c b/src/keymap.c
index 2c250578b00..295b209f06b 100644
--- a/src/keymap.c
+++ b/src/keymap.c
@@ -1735,11 +1735,20 @@ DEFUN ("current-active-maps", Fcurrent_active_maps, Scurrent_active_maps,
 		}
 	    }
 
-	  /* 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.  */
+	  Lisp_Object pos_area = POSN_POSN (position);
+	  if (EQ (pos_area, Qmode_line) || EQ (pos_area, Qheader_line))
+	    {
+	      /* For clicks on mode line or header line, ignore the maps
+		 we found at POSITION, because properties at point are
+		 not relevant in that case.  */
+	      local_map = Qnil;
+	      keymap = Qnil;
+	    }
+
+	  /* 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 only the
+	     `local-map' and `keymap' properties of that string.  */
 
 	  if (CONSP (string) && STRINGP (XCAR (string)))
 	    {
@@ -1749,23 +1758,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);
-		  Lisp_Object pos_area = POSN_POSN (position);
-		  /* For clicks on mode line or header line, override
-		     the maps we found at POSITION unconditionally, even
-		     if the corresponding properties of the mode- or
-		     header-line string are nil, because propertries at
-		     point are not relevant in that case.  */
-		  if (!NILP (map)
-		      || EQ (pos_area, Qmode_line)
-		      || EQ (pos_area, Qheader_line))
-		    local_map = map;
-		  map = Fget_text_property (pos, Qkeymap, string);
-		  if (!NILP (map)
-		      || EQ (pos_area, Qmode_line)
-		      || EQ (pos_area, Qheader_line))
-		    keymap = map;
+		  local_map = Fget_text_property (pos, Qlocal_map, string);
+		  keymap = Fget_text_property (pos, Qkeymap, string);
 		}
 	    }
 
diff --git a/test/src/keymap-tests.el b/test/src/keymap-tests.el
index c605c3eb09d..950c741a6dd 100644
--- a/test/src/keymap-tests.el
+++ b/test/src/keymap-tests.el
@@ -509,6 +509,33 @@ keymap-unset-test-remove-and-inheritance
     ;; From the parent this time/
     (should (equal (keymap-lookup map "u") #'undo))))
 
+(defun keymap-test--maps-for-posn (area string)
+  (current-active-maps
+   nil
+   ;; FIXME: This test would be better if this was a real position
+   ;; created by a real click.
+   `(,(selected-window) ,area (1 . 1) 0 (,string . 0) nil (1 . 1) nil (1 . 1) (1 . 1))))
+
+(ert-deftest keymap-test-keymaps-for-non-buffer-positions ()
+  "`current-active-maps' with non-buffer positions.  (bug#76620)"
+  (with-temp-buffer
+    (pop-to-buffer (current-buffer))
+    (let ((keymap (make-sparse-keymap "keymap-at-point")))
+      (insert (propertize "string" 'keymap keymap))
+      (goto-char (point-min))
+      (should (memq keymap (current-active-maps)))
+      (should-not (memq keymap (keymap-test--maps-for-posn 'mode-line nil)))
+      (should-not (memq keymap (keymap-test--maps-for-posn 'mode-line "s")))
+      (should-not (memq keymap (keymap-test--maps-for-posn nil "s")))
+      (should (memq keymap (keymap-test--maps-for-posn nil nil)))
+      (let* ((mode-line-keymap (make-sparse-keymap "keymap-in-mode-line"))
+             (s (propertize "string" 'keymap mode-line-keymap)))
+        ;; Respect `keymap' in the string clicked on.
+        (should-not (memq keymap (keymap-test--maps-for-posn nil s)))
+        (should-not (memq keymap (keymap-test--maps-for-posn 'mode-line s)))
+        (should (memq mode-line-keymap (keymap-test--maps-for-posn nil s)))
+        (should (memq mode-line-keymap (keymap-test--maps-for-posn 'mode-line s)))))))
+
 (provide 'keymap-tests)
 
 ;;; keymap-tests.el ends here
-- 
2.43.7