unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Selecting tooltip frames considered harmful
@ 2018-02-24  9:52 martin rudalics
  2018-02-24 10:41 ` Eli Zaretskii
  2018-02-25 13:33 ` Stefan Monnier
  0 siblings, 2 replies; 15+ messages in thread
From: martin rudalics @ 2018-02-24  9:52 UTC (permalink / raw)
  To: emacs-devel

Emacs tooltip frames are not "first-class" frames.  They don't have
minibuffer windows which means that calling 'message' when a tooltip
frame is selected will crash Emacs.  For example, with emacs -Q run
the following function and move the mouse cursor over the mode line
until a tooltip pops up:

(defun foo ()
   (interactive)
   (track-mouse
     (while (progn
              (setq event (read-key))
              (or (mouse-movement-p event)
                  (memq (car event) '(select-window switch-frame))))
       (dolist (frame (visible-frame-list))
	(when (frame-parameter frame 'tooltip)
	  (select-frame frame)
	  (message "..."))))))

Note also the following excerpt from choose_minibuf_frame

       /* I don't think that any frames may validly have a null minibuffer
	 window anymore.  */
       if (NILP (sf->minibuffer_window))
	emacs_abort ();

so the following function will crash us in a similar way:

(defun bar ()
   (interactive)
   (track-mouse
     (while (progn
              (setq event (read-key))
              (or (mouse-movement-p event)
                  (memq (car event) '(select-window switch-frame))))
       (dolist (frame (visible-frame-list))
	(when (frame-parameter frame 'tooltip)
	  (select-frame frame)
	  (read-from-minibuffer "..."))))))

You won't observe these crashes with GTK+ tooltips since these are not
Emacs frames.  Customize 'x-gtk-use-system-tooltips' to nil in order
to see them with GTK builds.

There seem to be various ways to fix this.  For the 'message' case we
could:

(1) Ignore the message in message3_nolog (the ideal solution but maybe
     too simplistic).

(2) Send the message to stderr instead (confusing for plain messages
     and hardly suited for 'read-from-minibuffer').

(3) Give tooltip frames a minibuffer window of some other frame (with
     the usual consequences when trying to delete that frame).

(4) Try to find a frame with a minibuffer and put the message there
     (it might be tricky to find the most suited minibuffer here).

(5) Avoid that a tooltip frame gets selected (hardly feasible).

A similar solution should be found when reading from the minibuffer.

Suggestions welcome, martin



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Selecting tooltip frames considered harmful
  2018-02-24  9:52 Selecting tooltip frames considered harmful martin rudalics
@ 2018-02-24 10:41 ` Eli Zaretskii
  2018-02-25 19:08   ` martin rudalics
  2018-02-25 13:33 ` Stefan Monnier
  1 sibling, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2018-02-24 10:41 UTC (permalink / raw)
  To: martin rudalics; +Cc: emacs-devel

> Date: Sat, 24 Feb 2018 10:52:14 +0100
> From: martin rudalics <rudalics@gmx.at>
> 
> (1) Ignore the message in message3_nolog (the ideal solution but maybe
>      too simplistic).
> 
> (2) Send the message to stderr instead (confusing for plain messages
>      and hardly suited for 'read-from-minibuffer').
> 
> (3) Give tooltip frames a minibuffer window of some other frame (with
>      the usual consequences when trying to delete that frame).
> 
> (4) Try to find a frame with a minibuffer and put the message there
>      (it might be tricky to find the most suited minibuffer here).
> 
> (5) Avoid that a tooltip frame gets selected (hardly feasible).
> 
> A similar solution should be found when reading from the minibuffer.
> 
> Suggestions welcome, martin

I think we should do (1) and document it.

Thanks.



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Selecting tooltip frames considered harmful
  2018-02-24  9:52 Selecting tooltip frames considered harmful martin rudalics
  2018-02-24 10:41 ` Eli Zaretskii
@ 2018-02-25 13:33 ` Stefan Monnier
  2018-02-25 19:08   ` martin rudalics
  1 sibling, 1 reply; 15+ messages in thread
From: Stefan Monnier @ 2018-02-25 13:33 UTC (permalink / raw)
  To: emacs-devel

> (5) Avoid that a tooltip frame gets selected (hardly feasible).

(6) Have tooltip frames be "transient for" some other frame (or maybe
    even for some other *window*), so we can redirect the messages (and
    the read-from-minibuffer, and ...) to that other frame.


-- Stefan




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Selecting tooltip frames considered harmful
  2018-02-24 10:41 ` Eli Zaretskii
@ 2018-02-25 19:08   ` martin rudalics
  2018-02-25 19:55     ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: martin rudalics @ 2018-02-25 19:08 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

 >> (1) Ignore the message in message3_nolog (the ideal solution but maybe
 >>       too simplistic).
{...]
 > I think we should do (1) and document it.

I'd tend to agree but I'm not sure whether the story will end here.

First of all I have to mention that the toy examples I presented will
hardly occur in practice.  Moreover, they could be easily fixed by
making 'visible-frame-list' not return a tooltip frame (and thus
always return a subset of the elements returned by 'frame-list') and
by having 'next-frame' and 'previous-frame' never return a tooltip
frame either.  This way the user should no longer be enabled to select
a tooltip frame from Elisp.

The problem did happen in practice here because I started implementing
a feature that allows mouse drags to cross frame boundaries.  This
means that when processing a MotionNotify event which is currently
handled as

	f = (x_mouse_grabbed (dpyinfo) ? dpyinfo->last_mouse_frame
	     : x_window_to_frame (dpyinfo, event->xmotion.window));

I have to set f to the frame specified by the event window instead of
the last mouse frame.  If I now show a tooltip frame during mouse
tracking (as 'mouse-drag-and-drop-region' does by default), that event
frame happens to be a tooltip frame surprisingly often, in particular
with the Lucid build.

I resolved this problem by never choosing a tooltip frame in the
scenario above (and a few related ones) but always go to the initially
grabbed frame instead.  Still I wonder whether the problem with a
selected tooltip frame might arise in practice and whether we should
do something about it.

Unfortunately, when we allow selecting a tooltip frame (1) is not
sufficient since Emacs will crash immediately afterwards - xdisp.c has
message_with_string, vmessage, setup_echo_area_for_printing,
redisplay_internal, see

	internal_condition_case_1 (redisplay_window_1,
				   FRAME_MINIBUF_WINDOW (sf), list_of_error,
				   redisplay_window_error);

and

       Lisp_Object mini_window = FRAME_MINIBUF_WINDOW (sf);
       struct frame *mini_frame = XFRAME (WINDOW_FRAME (XWINDOW (mini_window)));

all silently assuming that the selected frame has a minibuffer window.
So the necessary changes are quite substantial.  And obviously the
abort in minibuffer.c I cited earlier

       /* I don't think that any frames may validly have a null minibuffer
	 window anymore.  */
       if (NILP (sf->minibuffer_window))
	emacs_abort ();

is even more difficult to handle.

martin



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Selecting tooltip frames considered harmful
  2018-02-25 13:33 ` Stefan Monnier
@ 2018-02-25 19:08   ` martin rudalics
  2018-02-26  3:02     ` Stefan Monnier
  0 siblings, 1 reply; 15+ messages in thread
From: martin rudalics @ 2018-02-25 19:08 UTC (permalink / raw)
  To: Stefan Monnier, emacs-devel

 > (6) Have tooltip frames be "transient for" some other frame (or maybe
 >      even for some other *window*), so we can redirect the messages (and
 >      the read-from-minibuffer, and ...) to that other frame.

This is either

(3) Give tooltip frames a minibuffer window of some other frame (with
     the usual consequences when trying to delete that frame).

or

(4) Try to find a frame with a minibuffer and put the message there
     (it might be tricky to find the most suited minibuffer here).

The obvious candidate here is the minibuffer window of tip_last_frame
(making sure the tooltip frame gets deleted when tip_last_frame is
deleted).

The changes required to implement that are just as substantial as
those for not showing messages.

martin



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Selecting tooltip frames considered harmful
  2018-02-25 19:08   ` martin rudalics
@ 2018-02-25 19:55     ` Eli Zaretskii
  2018-02-26  9:05       ` martin rudalics
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2018-02-25 19:55 UTC (permalink / raw)
  To: martin rudalics; +Cc: emacs-devel

> Date: Sun, 25 Feb 2018 20:08:35 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: emacs-devel@gnu.org
> 
> Unfortunately, when we allow selecting a tooltip frame (1) is not
> sufficient since Emacs will crash immediately afterwards - xdisp.c has
> message_with_string, vmessage, setup_echo_area_for_printing,
> redisplay_internal, see

We should not allow selecting a tooltip frame, either.  It makes no
sense to do that.



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Selecting tooltip frames considered harmful
  2018-02-25 19:08   ` martin rudalics
@ 2018-02-26  3:02     ` Stefan Monnier
  2018-02-26  9:06       ` martin rudalics
  0 siblings, 1 reply; 15+ messages in thread
From: Stefan Monnier @ 2018-02-26  3:02 UTC (permalink / raw)
  To: martin rudalics; +Cc: emacs-devel

> The obvious candidate here is the minibuffer window of tip_last_frame
> (making sure the tooltip frame gets deleted when tip_last_frame is
> deleted).

I was rather thinking of the selected-window at the time the tooltip
frame is created (plus some way to re-set the "transient-for" field so
the same tooltip-frame can be reused?).

> The changes required to implement that are just as substantial as
> those for not showing messages.

First we need to decide what's The Right Thing, and I think that keeping
track of the "parent" frame/window is the right thing.

Once we agree on this, we can think about how best to
implement/approximate it.


        Stefan



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Selecting tooltip frames considered harmful
  2018-02-25 19:55     ` Eli Zaretskii
@ 2018-02-26  9:05       ` martin rudalics
  2018-02-26 15:50         ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: martin rudalics @ 2018-02-26  9:05 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

 > We should not allow selecting a tooltip frame, either.  It makes no
 > sense to do that.

But how would we go about that?  Refuse selecting a tooltip (or,
better, a frame without minibuffer window) right in select_window and
do_switch_frame?  Or have the callers of select_window and
do_switch_frame check whether the chosen frame can be selected?

And would we allow 'select-window' to return nil?

martin



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Selecting tooltip frames considered harmful
  2018-02-26  3:02     ` Stefan Monnier
@ 2018-02-26  9:06       ` martin rudalics
  2018-02-26 13:21         ` Stefan Monnier
  0 siblings, 1 reply; 15+ messages in thread
From: martin rudalics @ 2018-02-26  9:06 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

 >> The obvious candidate here is the minibuffer window of tip_last_frame
 >> (making sure the tooltip frame gets deleted when tip_last_frame is
 >> deleted).
 >
 > I was rather thinking of the selected-window at the time the tooltip
 > frame is created

A minibuffer or echo area window is specified frame-wise and is found
in the minibuffer_window slot of each frame.  So you probably mean the
mini window for the frame of the window selected at the time the
tooltip frame is created.  That window is in fact the mini window of
tip_last_frame.

 > (plus some way to re-set the "transient-for" field so
 > the same tooltip-frame can be reused?).

The same tooltip-frame (and its buffer and window) can be already
reused.  So what does the "transient-for" field stand for?

 > First we need to decide what's The Right Thing, and I think that keeping
 > track of the "parent" frame/window is the right thing.
 >
 > Once we agree on this, we can think about how best to
 > implement/approximate it.

We currently refuse to delete a frame when its minibuffer window
serves as the minibuffer window of another frame.  Since this sounds
like a bad idea in the context of tooltips, we probably would have to
delete a tooltip frame before deleting the frame whose minibuffer
serves as the surrogate for the tooltip.

So yes, we have to agree on what TRT is.  Either insist on all frames
having a minibuffer frame (or producing one on demand) or disallow
selecting frames which don't.

martin



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Selecting tooltip frames considered harmful
  2018-02-26  9:06       ` martin rudalics
@ 2018-02-26 13:21         ` Stefan Monnier
  2018-02-26 18:54           ` martin rudalics
  0 siblings, 1 reply; 15+ messages in thread
From: Stefan Monnier @ 2018-02-26 13:21 UTC (permalink / raw)
  To: martin rudalics; +Cc: emacs-devel

> So you probably mean the mini window for the frame of the window
> selected at the time the tooltip frame is created.

Indeed.

> That window is in fact the mini window of tip_last_frame.

Maybe the lifetime of a tooltip frame is sufficiently restricted that
this is the case  (I'm not sufficiently familiar with that code to
tell), but the *semantics* should be "the echo area of the window for
which the tooltip frame is transient", I think (and if tip_last_frame
gives us that, then that can be used in the implementation).

> We currently refuse to delete a frame when its minibuffer window
> serves as the minibuffer window of another frame.

I think this is an accident of implementation, not a feature.
As a user I've found it annoying on a bunch of occasions.
So we don't have to reproduce that misfeature here.

A more useful behavior would be to allow deleting that frame, and then
next time the other frame needs a miniwindow, you just go "oops, our
miniwindow has been deleted, let's look for another one".

We could still have a check when deleting a frame to make sure there
remains at least 1 miniwindow somewhere, of course.


        Stefan



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Selecting tooltip frames considered harmful
  2018-02-26  9:05       ` martin rudalics
@ 2018-02-26 15:50         ` Eli Zaretskii
  2018-02-26 18:54           ` martin rudalics
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2018-02-26 15:50 UTC (permalink / raw)
  To: martin rudalics; +Cc: emacs-devel

> Date: Mon, 26 Feb 2018 10:05:00 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: emacs-devel@gnu.org
> 
>  > We should not allow selecting a tooltip frame, either.  It makes no
>  > sense to do that.
> 
> But how would we go about that?

For calls from within the display engine (if there are those), make
sure we skip such frames.  For any other calls, signal an error.



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Selecting tooltip frames considered harmful
  2018-02-26 13:21         ` Stefan Monnier
@ 2018-02-26 18:54           ` martin rudalics
  0 siblings, 0 replies; 15+ messages in thread
From: martin rudalics @ 2018-02-26 18:54 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

 > the *semantics* should be "the echo area of the window for
 > which the tooltip frame is transient", I think (and if tip_last_frame
 > gives us that, then that can be used in the implementation).

tip_last_frame should give us that.

 >> We currently refuse to delete a frame when its minibuffer window
 >> serves as the minibuffer window of another frame.
 >
 > I think this is an accident of implementation, not a feature.
 > As a user I've found it annoying on a bunch of occasions.
 > So we don't have to reproduce that misfeature here.
 >
 > A more useful behavior would be to allow deleting that frame, and then
 > next time the other frame needs a miniwindow, you just go "oops, our
 > miniwindow has been deleted, let's look for another one".
 >
 > We could still have a check when deleting a frame to make sure there
 > remains at least 1 miniwindow somewhere, of course.

Using a mechanism similar to that for finding the new
'default-minibuffer-frame' I presume.

martin



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Selecting tooltip frames considered harmful
  2018-02-26 15:50         ` Eli Zaretskii
@ 2018-02-26 18:54           ` martin rudalics
  2018-02-26 19:31             ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: martin rudalics @ 2018-02-26 18:54 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

 > For calls from within the display engine (if there are those), make
 > sure we skip such frames.  For any other calls, signal an error.

This still sounds like I would have to do the check in the callers of
select_window and all flavors ending up in selecting a frame.  But the
display engine calls are all for frames with a mode line or a title,
so I think no check is needed in the first place.  Still I could put
one there in the few occurrences to make sure.

Then this means that I could put one check into do_switch_frame and
another one into select_window and have them always signal an error.

I'm still unsure how to reconcile your proposal with Stefan's.  Doing
both would be redundant but could help just in case one of the two
doesn't work out as expected.

martin



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Selecting tooltip frames considered harmful
  2018-02-26 18:54           ` martin rudalics
@ 2018-02-26 19:31             ` Eli Zaretskii
  2018-02-27  2:57               ` Stefan Monnier
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2018-02-26 19:31 UTC (permalink / raw)
  To: martin rudalics; +Cc: emacs-devel

> Date: Mon, 26 Feb 2018 19:54:57 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: emacs-devel@gnu.org
> 
>  > For calls from within the display engine (if there are those), make
>  > sure we skip such frames.  For any other calls, signal an error.
> 
> This still sounds like I would have to do the check in the callers of
> select_window and all flavors ending up in selecting a frame.

I see only 4 calls to Fselect_window in xdisp.c.

> Then this means that I could put one check into do_switch_frame and
> another one into select_window and have them always signal an error.

Yes.

> I'm still unsure how to reconcile your proposal with Stefan's.

You can't.  You have to choose one of them.

Thanks.



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Selecting tooltip frames considered harmful
  2018-02-26 19:31             ` Eli Zaretskii
@ 2018-02-27  2:57               ` Stefan Monnier
  0 siblings, 0 replies; 15+ messages in thread
From: Stefan Monnier @ 2018-02-27  2:57 UTC (permalink / raw)
  To: emacs-devel

>> I'm still unsure how to reconcile your proposal with Stefan's.
> You can't.  You have to choose one of them.

Actually, they're trivial to reconcile: if he follows your
recommendation, there won't be any way to tell whether he followed my
recommendation as well.


        Stefan




^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2018-02-27  2:57 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-24  9:52 Selecting tooltip frames considered harmful martin rudalics
2018-02-24 10:41 ` Eli Zaretskii
2018-02-25 19:08   ` martin rudalics
2018-02-25 19:55     ` Eli Zaretskii
2018-02-26  9:05       ` martin rudalics
2018-02-26 15:50         ` Eli Zaretskii
2018-02-26 18:54           ` martin rudalics
2018-02-26 19:31             ` Eli Zaretskii
2018-02-27  2:57               ` Stefan Monnier
2018-02-25 13:33 ` Stefan Monnier
2018-02-25 19:08   ` martin rudalics
2018-02-26  3:02     ` Stefan Monnier
2018-02-26  9:06       ` martin rudalics
2018-02-26 13:21         ` Stefan Monnier
2018-02-26 18:54           ` martin rudalics

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).