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

[Eli Zaretskii <eliz@gnu.org>]

> From: Alex <agrambot@gmail.com>
> Cc: rudalics@gmx.at,  23568@debbugs.gnu.org
> Date: Sat, 03 Jun 2017 13:19:50 -0600
> 
> > If this is supposed to be used as part of mode-line display, then no.
> > Signaling errors in the middle of redisplay is generally a bad idea,
> > because they cause another redisplay cycle, which again signals an
> > error, and Emacs just freezes.
> 
> I see. In the general case, is it recommended to silently fallback to a
> different method, or should there be a log message?
> 
> I changed the procedure so that it only uses the coordinates if they're
> numbers.

It depends.  For Lisp code that we write, it is indeed better to avoid
the problem altogether.  For user-defined Lisp, about which we can
make no assumptions, there should be a log message in case of errors.

> >> +(defun display-monitor-attribute (attribute &optional display x y)
> >> +  "Return the value of the ATTRIBUTE of the current monitor.
> >
> > The doc string should say something about what "the current monitor"
> > means, or have a link to where that is explained.
> 
> I expanded that section, copying a chunk of it from
> `frame-monitor-attributes'.
> 
> >> +DISPLAY can be a display name, a terminal name, or a frame.
> >
> > "Terminal name" or "terminal object"?
> 
> `display-monitor-attributes' can take both, which is where I copied that
> line from.
> 
> I don't think I should have done that, actually. I don't believe this
> function makes sense for a terminal/display input, as a
> terminal/display, as I understand it, can have multiple physical
> monitors.
> 
> I changed all instances of `display' to `frame'. I'm still not sure on
> the name, though. Perhaps a better prefix would be `physical-monitor-*',
> or just `monitor-*'?
> 
> What do you think?

LGTM, thanks.

> > Why is this in xmenu.c?  Is the problem unique to X window system?
> 
> The xmenu.c part contains the actual bugfix, while the frame.el part is
> just a partial abstraction for the fix so that other procedures can use
> it (such as compute_tip_xy in xfns.c).
> 
> The procedure I'm editing (menu_position_func) is inside an #ifdef
> USE_GTK, and uses x_display_pixel_{height,width} currently. I'm not sure
> if it's useful outside of X currently.
> 
> I haven't tested the issue on other systems, but I don't think it's an
> issue in Windows as half of the code came from Windows code according to
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=22549#61.

Ah, okay.  Then maybe just add a comment there saying that the w32
case is handled elsewhere, perhaps with a pointer to the function
which does that.

> Here's an updated diff:

Thanks, it would be nice if you could also provide a ChangeLog-style
commit log message.  That'd make the job of pushing the changes much
easier.