bug#52293: [External] : bug#52293: 29.0.50; [PATCH v3] Prevent further cases of duplicated separators in context menus

[Jim Porter <jporterbugs@gmail.com>]

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

On 12/15/2021 12:57 AM, Juri Linkov wrote:
> I'm not sure if de-duplication is needed for other menus.  Currently
> de-duplication is used only to simplify creation of context menus.
> But there is no indication that someone needed this for other menus.
> Otherwise, they would report duplicate separators in other menus as a bug.

Ok, we can worry about that another time.

I've attached an updated patch that lets context-menu-functions add 
items to the beginning of the keymap as they currently do, while still 
removing consecutive separators correctly. Since this logic is a bit 
tricky, it could probably use an automated test or two, but before I 
write some, I wanted to check that the strategy I'm using seems 
reasonable. It's probably easiest to explain the logic by just pointing 
to the patch; I added several comments describing the behavior so that 
reviewers (and future readers) should be able to make sense of it.

[-- Attachment #2: 0001-Prevent-further-cases-of-duplicated-separators-in-co.patch --]
[-- Type: text/plain, Size: 2676 bytes --]

From 0310f926b62fd74932e3766c03bd7a39974c457b Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Fri, 31 Dec 2021 23:04:57 -0800
Subject: [PATCH] Prevent further cases of duplicated separators in context
 menus

In some cases, context menu items are added before the overall prompt
string.  This could cause multiple consecutive separators to appear if
they "surround" the prompt string.

* lisp/mouse.el (context-menu-map): Improve the de-duplication logic
to ignore non-menu-items when checking for consecutive separators.
---
 lisp/mouse.el | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/lisp/mouse.el b/lisp/mouse.el
index 11fdd3f639..41cfff8d82 100644
--- a/lisp/mouse.el
+++ b/lisp/mouse.el
@@ -329,20 +329,27 @@ context-menu-map
 
     ;; Remove duplicate separators as well as ones at the beginning or
     ;; end of the menu.
-    (let ((l menu) saw-first-item)
+    (let ((l menu) (remove-next-separator t))
       (while (and (consp l)
                   (consp (cdr l)))
-        ;; If the next item is a separator, remove it if 1) we haven't
-        ;; seen any other items yet, or 2) it's followed by either
-        ;; another separator or the end of the list.
-        (if (and (equal (cdr-safe (cadr l)) menu-bar-separator)
-                 (or (not saw-first-item)
-                     (null (caddr l))
-                     (equal (cdr-safe (caddr l)) menu-bar-separator)))
-            (setcdr l (cddr l))
-          ;; The "first item" is any cons cell; this excludes the
-          ;; `keymap' symbol and the menu name.
-          (when (consp (cadr l)) (setq saw-first-item t))
+        (if (equal (cdr-safe (cadr l)) menu-bar-separator)
+            (progn
+              ;; The next item is a separator.  Remove it if requested
+              ;; by remove-next-separator or if it's the last item in
+              ;; the list.
+              (if (or remove-next-separator
+                      (null (caddr l)))
+                  (setcdr l (cddr l))
+                (setq l (cdr l)))
+              ;; We just saw a separator.  Remove any immediately
+              ;; following this.
+              (setq remove-next-separator t))
+          ;; If the next item is a cons cell, we found a non-separator
+          ;; item.  Don't remove the next separator we see.  We
+          ;; specifically check for cons cells to avoid treating the
+          ;; overall prompt string as a menu item.
+          (when (consp (cadr l))
+            (setq remove-next-separator nil))
           (setq l (cdr l)))))
 
     (when (functionp context-menu-filter-function)
-- 
2.25.1