Subject: bug#23568: 25.0.94; Mode line menus appear incorrectly in some monitor configurations

From: Alex <[email-masked]>
Date: Fri, 02 Jun 2017 18:54:03 -0600

martin rudalics <[email-masked]> writes:

>> In the end, we would still leave the function interface so that users
>> can still call the discussed procedures from lisp code, right?
>
> I think so, yes.
>
>> (defun display-monitor-attribute (attribute &optional frame x y)
>>   "Return the value of the attribute of the 'current' monitor.
>> By default, use the frame info to determine the current monitor,
>> but if x and y are non-nil then use the given coordinates to
>> determine it."
>>   (if (and x y)
>
> Probably, FRAME should be replaced by DISPLAY as in other ‘display-...’
> functions.  X and Y must be checked properly to avoid throwing an error
> when they are used in the arithmetics.  And the doc-string should
> explicitly state that the first argument is ignored when X and Y are
> both non-nil.

I fixed up the docstrings to match display-monitor-attributes-list.

Don't you want to throw an error when x and y are used, but are not
integers?

Also, should yet another procedure be created that abstracts out adding
the min_{x,y} and width/height components of the geometry/workarea
attributes, or should it be expected for each caller to do that
manually?

The current diff is attached below:

diff --git a/lisp/frame.el b/lisp/frame.el
index 02871e0551..005f1f3e83 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -1908,6 +1908,61 @@ display-monitor-attributes-list
 		       ,(display-mm-height display)))
 	   (frames . ,(frames-on-display-list display)))))))))
 
+(defun display-monitor-attribute (attribute &optional display x y)
+  "Return the value of the ATTRIBUTE of the current monitor.
+DISPLAY can be a display name, a terminal name, or a frame.
+If DISPLAY is omitted or nil, it defaults to the selected frame’s display.
+
+By default, use DISPLAY to determine the current monitor.  If X
+and Y are both non-nil, then ignore the value of DISPLAY and use
+X and Y as pixel coordinates to determine the current monitor.
+
+See `display-monitor-attributes-list' for information on possible
+attributes."
+  (if (and x y)
+      (cl-loop for monitor in (display-monitor-attributes-list)
+               for geometry = (alist-get 'geometry monitor)
+               for min-x = (pop geometry)
+               for min-y = (pop geometry)
+               for max-x = (+ min-x (pop geometry))
+               for max-y = (+ min-y (car geometry))
+               when (and (<= min-x x)
+                         (< x max-x)
+                         (<= min-y y)
+                         (< y max-y))
+               return (alist-get attribute monitor))
+    (alist-get attribute (frame-monitor-attributes display))))
+
+(defun display-monitor-geometry (&optional display x y)
+    "Return the geometry of the current monitor.
+DISPLAY can be a display name, a terminal name, or a frame.
+If DISPLAY is omitted or nil, it defaults to the selected frame’s display.
+
+By default, use DISPLAY to determine the current monitor.  If X
+and Y are both non-nil, then ignore the value of DISPLAY and use
+X and Y as pixel coordinates to determine the current monitor.
+
+See `display-monitor-attributes-list' for information on the
+geometry attribute."
+  (display-monitor-attribute 'geometry display x y))
+
+(defun display-monitor-workarea (&optional display x y)
+  "Return the workarea of the current monitor.
+DISPLAY can be a display name, a terminal name, or a frame.
+If DISPLAY is omitted or nil, it defaults to the selected frame’s display.
+
+By default, use DISPLAY to determine the current monitor.  If X
+and Y are both non-nil, then ignore the value of DISPLAY and use
+X and Y as pixel coordinates to determine the current monitor.
+
+The result is a list of the form (x-min y-min x-max y-max),
+representing the minimum and maximum pixel coordinates of the
+workarea.
+
+See `display-monitor-attributes-list' for information on the
+workarea attribute."
+  (display-monitor-attribute 'workarea display x y))
+
 
 ;;;; Frame geometry values
 
diff --git a/src/xmenu.c b/src/xmenu.c
index 2805249164..04d5bde2ba 100644
--- a/src/xmenu.c
+++ b/src/xmenu.c
@@ -1160,9 +1160,33 @@ menu_position_func (GtkMenu *menu, gint *x, gint *y, gboolean *push_in, gpointer
 {
   struct next_popup_x_y *data = user_data;
   GtkRequisition req;
-  struct x_display_info *dpyinfo = FRAME_DISPLAY_INFO (data->f);
-  int disp_width = x_display_pixel_width (dpyinfo);
-  int disp_height = x_display_pixel_height (dpyinfo);
+  int disp_width = -1;
+  int disp_height = -1;
+
+  Lisp_Object frame, geometry;
+
+  XSETFRAME (frame, data->f);
+  geometry = call3 (Qdisplay_monitor_geometry,
+                    Qnil,
+                    make_number (data->x),
+                    make_number (data->y));
+
+  if (CONSP (geometry))
+    {
+      int min_x, min_y;
+
+      min_x = XINT (XCAR (geometry));
+      disp_width = min_x + XINT (Fnth (make_number (2), geometry));
+      min_y = XINT (Fnth (make_number (1), geometry));
+      disp_height = min_y + XINT (Fnth (make_number (3), geometry));
+    }
+  if ( disp_width < 0 || disp_height < 0 )
+    {
+      struct x_display_info *dpyinfo = FRAME_DISPLAY_INFO (data->f);
+
+      disp_width = x_display_pixel_width (dpyinfo);
+      disp_height = x_display_pixel_height (dpyinfo);
+    }
 
   *x = data->x;
   *y = data->y;
@@ -2361,6 +2385,10 @@ syms_of_xmenu (void)
   DEFSYM (Qdebug_on_next_call, "debug-on-next-call");
   defsubr (&Smenu_or_popup_active_p);
 
+#ifdef USE_GTK
+  DEFSYM (Qdisplay_monitor_geometry, "display-monitor-geometry");
+#endif
+
 #if defined (USE_GTK) || defined (USE_X_TOOLKIT)
   defsubr (&Sx_menu_bar_open_internal);
   Ffset (intern_c_string ("accelerate-menu"),