bug#43756: Fix for TTY menus mouse interaction

[Jared Finder via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

[-- Attachment #1: Type: text/plain, Size: 3801 bytes --]

On 2020-10-03 12:23 am, Eli Zaretskii wrote:
>> Date: Fri, 02 Oct 2020 12:54:54 -0700
>> From: Jared Finder <jared@finder.org>
>> Cc: larsi@gnus.org, 43756@debbugs.gnu.org
>> 
>> > Ah, xterm-mouse-mode.  AFAIR, no one has made TTY menus work with
>> > xterm-mouse-mode.  The first thing to do is to disable tmm-menubar,
>> > and then you need to cause a mouse click call menu-bar-open with the
>> > 2nd argument set to the X coordinate of the click.
>> 
>> I think this may be getting mixed up with my feature proposal on
>> emacs-devel? I did find this bug when working on making the TTY menus
>> work with xterm-mouse-mode and they both affect menus. The patches I
>> attached in that thread follow the pattern you described.
>> 
>> This bug report is separable from the rest of that feature. It's also
>> much smaller, just two lines. :) This fixes the current behavior of 
>> the
>> command tmm-menubar-mouse, which is bound to <menu-bar> <mouse-1>.
> 
> So you are saying that when tmm-menubar-mouse is invoked by mouse
> clicks, it shows incorrect menus after "M-x ielm", but only if you
> click on the menu items specific to IELM?  It sounds like tmm-menubar
> has a bug in its translation of the X coordinate of the click to the
> menu-bar item, perhaps because it considers only the global menu
> keymap.

I think the issue is that the ordering of elements in 
menu-bar-final-items controls the order of the display menu bar, but 
tmm-menubar-keymap does not currently take that into account. It was 
pure luck that my first patch fixed anything. :)

Attached is a new patch and more detailed repro steps.

First, the ordering bug (the changes inside tmm-menubar-keymap):

Environment:

* Emacs repo from git://git.sv.gnu.org/emacs.git, commit 
f6277911eb2c520aec8f0efd80c91999226e3322
* Run on Debian under Windows Subsystem for Linux 2 (the VM version). 
I've also seen this on MacOS and native Win32 builds of Emacs.

Steps to reproduce:

emacs -Q
M-x load-library <RET> tmm <RET>

Defining the following stripped down version of tmm-menubar makes it 
easier to visually compare the displayed menu bar with what tmm-menubar 
would show:

(defun menubar-items ()
   "Simplified version of tmm-menubar."
   (let ((column 0)
         (menu-bar (tmm-menubar-keymap))
         list)
     (catch 'done
       (map-keymap
        (lambda (key binding)
          (pcase binding
            ((or `(,(and (pred stringp) name) . ,_) ;Simple menu item.
                 `(menu-item ,name ,_cmd            ;Extended menu item.
                             . ,(and props
                                     (guard (let ((visible
                                                   (plist-get props 
:visible)))
                                              (or (null visible)
                                                  (eval visible)))))))
             (push (cons name column) list)
             (setq column (+ column (length name) 1)))))
        menu-bar)
     (nreverse list))))

M-x ielm

Eval (menubar-items)
--> observe that the ordering of menu items returned is not the same as 
the displayed menu bar.
--> With my patch this is correct.

Eval (setf menu-bar-final-items '(help-menu completion signals inout))
Force redisplay via C-l
--> observe that the ordering of menu items in the displayed menu bar 
has changed.

Eval (menubar-items)
--> observe that the ordering of menu items returned has not changed.
--> With my patch this is correct.

Repeat above steps for other values of menu-bar-final-items. You can add 
'file to the list, set it to nil, etc.


The click bug is a simple fix in tmm-menubar since the lambda passed in 
to map-keymap always checks against the previous menu item (e.g. 
prev-key), therefore missing the last item. Repro steps are same as 
before.

   -- MJF

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fixing-small-bugs-in-tmm.el.patch --]
[-- Type: text/x-diff; name=0001-Fixing-small-bugs-in-tmm.el.patch, Size: 2812 bytes --]

From 12d3bfc01d6632fb8a903dd757ee9574da96e420 Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Thu, 17 Sep 2020 22:37:00 -0700
Subject: [PATCH 1/3] Fixing small bugs in tmm.el.

* `tmm-menubar-keymap' now properly sorts final items.
* `tmm-menubar' allows clicks on the final menu item.
---
 lisp/tmm.el    | 23 +++++++++++++++--------
 src/keyboard.c |  3 ++-
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/lisp/tmm.el b/lisp/tmm.el
index e9f3f5b038..074ee7593f 100644
--- a/lisp/tmm.el
+++ b/lisp/tmm.el
@@ -50,14 +50,18 @@ tmm-menubar-keymap
         (menu-end '()))
     (map-keymap
      (lambda (key binding)
-       (push (cons key binding)
-             ;; If KEY is the name of an item that we want to put last,
-             ;; move it to the end.
-             (if (memq key menu-bar-final-items)
-                 menu-end
-               menu-bar)))
+       (let ((pos (seq-position menu-bar-final-items key))
+             (menu-item (cons key binding)))
+         (if pos
+             ;; If KEY is the name of an item that we want to put
+             ;; last, store it separately with explicit ordering for
+             ;; sorting.
+             (push (cons pos menu-item) menu-end)
+           (push menu-item menu-bar))))
      (tmm-get-keybind [menu-bar]))
-    `(keymap ,@(nreverse menu-bar) ,@(nreverse menu-end))))
+    `(keymap ,@(nreverse menu-bar)
+             ,@(mapcar 'cdr (sort menu-end
+                                  (lambda (a b) (< (car a) (car b))))))))
 
 ;;;###autoload (define-key global-map "\M-`" 'tmm-menubar)
 ;;;###autoload (define-key global-map [menu-bar mouse-1] 'tmm-menubar-mouse)
@@ -96,7 +100,10 @@ tmm-menubar
                                          (or (null visible)
                                              (eval visible)))))))
                   (setq column (+ column (length name) 1)))))
-             menu-bar))))
+             menu-bar)
+            ;; Check the last menu item.
+            (when (> column x-position)
+              (setq menu-bar-item prev-key)))))
     (tmm-prompt menu-bar nil menu-bar-item)))
 
 ;;;###autoload
diff --git a/src/keyboard.c b/src/keyboard.c
index af075a42c7..ea5f961304 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -12106,7 +12106,8 @@ syms_of_keyboard (void)
 
   DEFVAR_LISP ("menu-bar-final-items", Vmenu_bar_final_items,
 	       doc: /* List of menu bar items to move to the end of the menu bar.
-The elements of the list are event types that may have menu bar bindings.  */);
+The order of this list controls the order of the items. The elements of the list
+are event types that may have menu bar bindings.  */);
   Vmenu_bar_final_items = Qnil;
 
   DEFVAR_LISP ("tab-bar-separator-image-expression", Vtab_bar_separator_image_expression,
-- 
2.20.1