Deiconifying GTK frames on GNOME shell

Deiconifying GTK frames on GNOME shell

[martin rudalics]

When running a GTK3 build of Emacs on GNOME shell, the following
sequence of actions

(setq frame (make-frame))
(iconify-frame frame)
(make-frame-visible frame)

does not produce a visible frame here.  Rather, the frame stays
iconified although (frame-visible-p frame) for it returns t.
(raise-frame frame) and (select-frame-set-input-focus frame) do not work
either.  A similar problem happens when running Emacs under the
Enlightenment WM.

I invite users running Emacs under GNOME shell to tell us whether they
see the same behavior or whether the above sequence of operations works
as intended.

This affects all frame management routines calling candidate_frame and
people who customized `frame-auto-hide-function' to `iconify-frame'.  In
particular, if you show the minibuffer in a separate frame, you cannot
raise that frame from Emacs once you have iconified it.

If we decide that this is a bug _we_ want to fix then I can offer the
following changes which seem to make things work here:

(1) In xterm.c swap the calls to

       gtk_widget_show_all (FRAME_GTK_OUTER_WIDGET (f));
       gtk_window_deiconify (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)));

(2) When trying to deiconify an iconified frame make it invisible first
     and only then make it visible.

I have no idea whether and how these changes might affect other builds
so making these customizable might be a good idea.  But first I'd like
to know whether we really care about this issue.  Since I use GNOME
shell for testing purposes only, I do not care personally.

Thanks in advance, martin

Re: Deiconifying GTK frames on GNOME shell

[Colin Baxter]

>>>>> martin rudalics <rudalics@gmx.at> writes:

    > When running a GTK3 build of Emacs on GNOME shell, the following
    > sequence of actions

    > (setq frame (make-frame)) (iconify-frame frame)
    > (make-frame-visible frame)

    > does not produce a visible frame here.  Rather, the frame stays
    > iconified although (frame-visible-p frame) for it returns t.
    > (raise-frame frame) and (select-frame-set-input-focus frame) do
    > not work either.  A similar problem happens when running Emacs
    > under the Enlightenment WM.

    > I invite users running Emacs under GNOME shell to tell us whether
    > they see the same behavior or whether the above sequence of
    > operations works as intended.

For me, the sequence works as intended in openbox but not in either
gnome-session or gnome-session-classic.

Best wishes,

Re: Deiconifying GTK frames on GNOME shell

[Dmitry Gutov]

On 05.09.2021 12:39, martin rudalics wrote:
> When running a GTK3 build of Emacs on GNOME shell, the following
> sequence of actions
> 
> (setq frame (make-frame))
> (iconify-frame frame)
> (make-frame-visible frame)
> 
> does not produce a visible frame here.  Rather, the frame stays
> iconified although (frame-visible-p frame) for it returns t.

Same here, except (frame-visible-p frame) returns 'icon.

Re: Deiconifying GTK frames on GNOME shell

[martin rudalics]

 > For me, the sequence works as intended in openbox but not in either
 > gnome-session or gnome-session-classic.

Thanks for responding.  I conclude that the issue doesn't bother you.

martin

Re: Deiconifying GTK frames on GNOME shell

[martin rudalics]

 >> When running a GTK3 build of Emacs on GNOME shell, the following
 >> sequence of actions
 >>
 >> (setq frame (make-frame))
 >> (iconify-frame frame)
 >> (make-frame-visible frame)
 >>
 >> does not produce a visible frame here.  Rather, the frame stays
 >> iconified although (frame-visible-p frame) for it returns t.
 >
 > Same here, except (frame-visible-p frame) returns 'icon.

Even after doing (make-frame-visible frame)?

Just in case can you confirm that the earlier mentioned

(1) In xterm.c swap the calls to

       gtk_widget_show_all (FRAME_GTK_OUTER_WIDGET (f));
       gtk_window_deiconify (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)));

(2) When trying to deiconify an iconified frame make it invisible first
     and only then make it visible.

hack would work?

Thanks, martin

Re: Deiconifying GTK frames on GNOME shell

[Dmitry Gutov]

On 06.09.2021 11:32, martin rudalics wrote:
> Even after doing (make-frame-visible frame)?

Yup.

> Just in case can you confirm that the earlier mentioned
> 
> (1) In xterm.c swap the calls to
> 
>        gtk_widget_show_all (FRAME_GTK_OUTER_WIDGET (f));
>        gtk_window_deiconify (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)));
> 
> (2) When trying to deiconify an iconified frame make it invisible first
>      and only then make it visible.
> 
> hack would work?

Yes, that seems to work. Consistently.

If I don't do (1), BTW, I can do (2) twice, and that also makes the 
frame visible.

Meaning

   (make-frame-invisible frame)
   (make-frame-visible frame)
   (make-frame-invisible frame)
   (make-frame-visible frame)

If I do (1), then doing (2) only once is sufficient.

Re: Deiconifying GTK frames on GNOME shell

[martin rudalics]

 >> (1) In xterm.c swap the calls to
 >>
 >>        gtk_widget_show_all (FRAME_GTK_OUTER_WIDGET (f));
 >>        gtk_window_deiconify (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)));
 >>
 >> (2) When trying to deiconify an iconified frame make it invisible first
 >>      and only then make it visible.
 >>
 >> hack would work?
 >
 > Yes, that seems to work. Consistently.

Thanks for checking.

 > If I don't do (1), BTW, I can do (2) twice, and that also makes the frame visible.
 >
 > Meaning
 >
 >    (make-frame-invisible frame)
 >    (make-frame-visible frame)
 >    (make-frame-invisible frame)
 >    (make-frame-visible frame)
 >
 > If I do (1), then doing (2) only once is sufficient.

Interesting.  I will check when I switch to GNOME next time.

Many thanks, martin

Re: Deiconifying GTK frames on GNOME shell

[Madhu]

* martin rudalics <635efeee-ffd5-28e3-d966-1086990b1aac@gmx.at> :
Wrote on Tue, 7 Sep 2021 10:16:02 +0200:

>>> (1) In xterm.c swap the calls to
>>>        gtk_widget_show_all (FRAME_GTK_OUTER_WIDGET (f));
>>>        gtk_window_deiconify (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)));
>>> (2) When trying to deiconify an iconified frame make it invisible first
>>>      and only then make it visible.
>>> hack would work?
>> Yes, that seems to work. Consistently.
> Thanks for checking.
>
>> If I don't do (1), BTW, I can do (2) twice, and that also makes the
>> frame visible.
>>    (make-frame-invisible frame)
>>    (make-frame-visible frame)
>>    (make-frame-invisible frame)
>>    (make-frame-visible frame)
>>
>> If I do (1), then doing (2) only once is sufficient.
>
> Interesting.  I will check when I switch to GNOME next time.

[I thought I posted this workaround that I've been using at least since
- either on a bugreport or in a message to you - but I can't spot it in
my XMAIL]

(defadvice make-frame-visible (around mutter-workaround (&optional frame) activate)
  (if (or (eql (frame-parameter frame 'visibility) 'icon)
	   (eql (frame-parameter frame 'visibility) nil))
      (set-frame-parameter frame 'visibility nil))
  ad-do-it)

Re: Deiconifying GTK frames on GNOME shell

[martin rudalics]

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

 > If I don't do (1), BTW, I can do (2) twice, and that also makes the frame visible.
 >
 > Meaning
 >
 >    (make-frame-invisible frame)
 >    (make-frame-visible frame)
 >    (make-frame-invisible frame)
 >    (make-frame-visible frame)

I attach a patch based on this recipe but it causes (occasionally
terrible) flickers on other desktops so I would have to special-case it
on mutter anyway.  I still have no good idea what's really going on
here.

martin

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: viv.diff --]
[-- Type: text/x-patch; name="viv.diff", Size: 5562 bytes --]

diff --git a/src/w32term.c b/src/w32term.c
index ad4d1a3282..a1b20236d8 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -6903,11 +6903,6 @@ w32_make_frame_invisible (struct frame *f)

   my_show_window (f, FRAME_W32_WINDOW (f), SW_HIDE);

-  /* We can't distinguish this from iconification
-     just by the event that we get from the server.
-     So we can't win using the usual strategy of letting
-     FRAME_SAMPLE_VISIBILITY set this.  So do it by hand,
-     and synchronize with the server to make sure we agree.  */
   SET_FRAME_VISIBLE (f, 0);
   SET_FRAME_ICONIFIED (f, false);

diff --git a/src/xdisp.c b/src/xdisp.c
index e853c8c223..66535109b2 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -15675,9 +15675,6 @@ redisplay_internal (void)
       FRAME_TTY (sf)->previous_frame = sf;
     }

-  /* Set the visible flags for all frames.  Do this before checking for
-     resized or garbaged frames; they want to know if their frames are
-     visible.  See the comment in frame.h for FRAME_SAMPLE_VISIBILITY.  */
   number_of_visible_frames = 0;

   FOR_EACH_FRAME (tail, frame)
diff --git a/src/xterm.c b/src/xterm.c
index 1887c3255d..0de6be762c 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -8239,12 +8239,33 @@ handle_one_xevent (struct x_display_info *dpyinfo,
       f = x_window_to_frame (dpyinfo, event->xexpose.window);
       if (f)
         {
+	  if (CONSP (frame_size_history))
+	    {
+	      frame_size_history_extra
+		(f,
+		 FRAME_VISIBLE_P (f)
+		 ? build_string ("Expose, visible")
+		 : build_string ("Expose, not visible"),
+		 FRAME_PIXEL_WIDTH (f), FRAME_PIXEL_HEIGHT (f),
+		 -1, -1, event->xexpose.width, event->xexpose.height);
+	    }
+
           if (!FRAME_VISIBLE_P (f))
             {
               block_input ();
 	      /* The following two are commented out to avoid that a
 		 plain invisible frame gets reported as iconified.  That
-		 problem occurred first for Emacs 26 and is described in
+		 problem occurred first for Emacs 26 with GTK3 and is
+		 the result of the following actions:
+
+		 (1) x_make_frame_invisible sets f->visible to 0.
+
+		 (2) We get an (unexpected) Expose event for f and here
+		     set f->visible to 1.
+
+		 (3) The subsequent UnmapNotify event finds f->visible
+		     is 1 and sets f->iconified true.
+
 		 https://lists.gnu.org/archive/html/emacs-devel/2017-02/msg00133.html.  */
 /** 	      SET_FRAME_VISIBLE (f, 1); **/
 /** 	      SET_FRAME_ICONIFIED (f, false); **/
@@ -8835,26 +8856,24 @@ handle_one_xevent (struct x_display_info *dpyinfo,
       goto OTHER;

     case FocusIn:
-#ifndef USE_GTK
       /* Some WMs (e.g. Mutter in Gnome Shell), don't unmap
          minimized/iconified windows; thus, for those WMs we won't get
          a MapNotify when unminimizing/deconifying.  Check here if we
-         are deiconizing a window (Bug42655).
-
-	 But don't do that on GTK since it may cause a plain invisible
-	 frame get reported as iconified, compare
-	 https://lists.gnu.org/archive/html/emacs-devel/2017-02/msg00133.html.
-	 That is fixed above but bites us here again.  */
+         are deiconifying a window (Bug42655).  */
       f = any;
+      /* Should we handle invisible frames here too?  */
       if (f && FRAME_ICONIFIED_P (f))
 	{
+	  if (CONSP (frame_size_history))
+	    frame_size_history_plain
+	      (f, build_string ("FocusIn, was iconified"));
+
           SET_FRAME_VISIBLE (f, 1);
           SET_FRAME_ICONIFIED (f, false);
           f->output_data.x->has_been_visible = true;
           inev.ie.kind = DEICONIFY_EVENT;
           XSETFRAME (inev.ie.frame_or_window, f);
         }
-#endif /* USE_GTK */

       x_detect_focus_change (dpyinfo, any, event, &inev.ie);
       goto OTHER;
@@ -9344,9 +9363,24 @@ handle_one_xevent (struct x_display_info *dpyinfo,

     case VisibilityNotify:
       f = x_top_window_to_frame (dpyinfo, event->xvisibility.window);
-      if (f && (event->xvisibility.state == VisibilityUnobscured
-		|| event->xvisibility.state == VisibilityPartiallyObscured))
-	SET_FRAME_VISIBLE (f, 1);
+      if (f)
+	{
+	  if (event->xvisibility.state == VisibilityUnobscured
+	      || event->xvisibility.state == VisibilityPartiallyObscured)
+	    {
+	      if (CONSP (frame_size_history))
+		frame_size_history_plain
+		  (f, build_string ("VisibilityNotify, visible"));
+
+	      SET_FRAME_VISIBLE (f, 1);
+	    }
+	  else
+	    {
+	      if (CONSP (frame_size_history))
+		frame_size_history_plain
+		  (f, build_string ("VisibilityNotify, invisible"));
+	    }
+	}

       goto OTHER;

@@ -11918,11 +11952,6 @@ x_make_frame_invisible (struct frame *f)

   x_sync (f);

-  /* We can't distinguish this from iconification
-     just by the event that we get from the server.
-     So we can't win using the usual strategy of letting
-     FRAME_SAMPLE_VISIBILITY set this.  So do it by hand,
-     and synchronize with the server to make sure we agree.  */
   SET_FRAME_VISIBLE (f, 0);
   SET_FRAME_ICONIFIED (f, false);

@@ -11937,7 +11966,18 @@ x_make_frame_invisible (struct frame *f)
 x_make_frame_visible_invisible (struct frame *f, bool visible)
 {
   if (visible)
+#if defined (USE_GTK)
+    if (FRAME_ICONIFIED_P (f))
+      {
+	x_make_frame_visible (f);
+	x_make_frame_invisible (f);
+	x_make_frame_visible (f);
+      }
+  else
     x_make_frame_visible (f);
+#else
+  x_make_frame_visible (f);
+#endif
   else
     x_make_frame_invisible (f);
 }

Re: Deiconifying GTK frames on GNOME shell

[martin rudalics]

 > [I thought I posted this workaround that I've been using at least since

... when? ...

 > - either on a bugreport or in a message to you - but I can't spot it in
 > my XMAIL]

Have we been discussing this issue before?  Have you (or anyone else)
filed a report against it?  No pointers anywhere?

 > (defadvice make-frame-visible (around mutter-workaround (&optional frame) activate)
 >    (if (or (eql (frame-parameter frame 'visibility) 'icon)
 > 	   (eql (frame-parameter frame 'visibility) nil))
 >        (set-frame-parameter frame 'visibility nil))
 >    ad-do-it)

This works here for making the frame visible again.  It fails for
`raise-frame' and `select-frame-set-input-focus' but I think it should
be possible to fix these with similar advices too.

I don't understand yet how this x_make_frame_visible_invisible stuff is
supposed to work in the first place and I haven't even found the commit
that introduced it yet.  Maybe it got fixed or broken during

https://lists.gnu.org/r/emacs-devel/2018-03/msg00863.html

so there should be also a timeout around somewhere.

IIUC this could be a serious issue with frame switching on mutter so
maybe either other people use a workaround like yours or they use a
version of mutter where this problem doesn't happen.

martin

Re: Deiconifying GTK frames on GNOME shell

[Madhu]

* martin rudalics <366d9251-3609-9e0b-3d35-8af409af706a@gmx.at> :
Wrote on Fri, 10 Sep 2021 10:34:17 +0200:

>> [I thought I posted this workaround that I've been using at least since
> ... when? ...

a year at least (i have it in my October 2020 backup)

>> - either on a bugreport or in a message to you - but I can't spot it in
>> my XMAIL]
>
> Have we been discussing this issue before?  Have you (or anyone else)
> filed a report against it?  No pointers anywhere?

Apparently not.

I've noticed the problem since march last year - the mutter-wayland folk
i chatted with at that time disclaimed any notion of "iconify", saying
hiding a window does not change the state and this is required for
live-previews when switching workspaces, etc. though they claimed full
support for ewmh NET_WM_STATE_HIDDEN

>> (defadvice make-frame-visible (around mutter-workaround (&optional frame) activate)
>>    (if (or (eql (frame-parameter frame 'visibility) 'icon)
>> 	   (eql (frame-parameter frame 'visibility) nil))
>>        (set-frame-parameter frame 'visibility nil))
>>    ad-do-it)
>
> This works here for making the frame visible again.  It fails for
> `raise-frame' and `select-frame-set-input-focus' but I think it should
> be possible to fix these with similar advices too.

no, I couldn't figure out how to make raise-frame (select a different
frame and set input focus) work on mutter-wayland with gtkonly emacs.
gdk's calls do not work.

I think that's being punted, and relief is expected from some
"xdg-activation protocol"

> I don't understand yet how this x_make_frame_visible_invisible stuff is
> supposed to work in the first place and I haven't even found the commit
> that introduced it yet.  Maybe it got fixed or broken during

The mutter model means invisible frames are still supposed to keep
rendering - they just wont be composited on the main workspace.

> https://lists.gnu.org/r/emacs-devel/2018-03/msg00863.html
>
> so there should be also a timeout around somewhere.
>
> IIUC this could be a serious issue with frame switching on mutter so
> maybe either other people use a workaround like yours or they use a
> version of mutter where this problem doesn't happen.

[I try use it once in a while - try to get it to do what I want,
encounter problems, give up for a month, etc.]

Re: Deiconifying GTK frames on GNOME shell

[martin rudalics]

 >>> [I thought I posted this workaround that I've been using at least since
 >> ... when? ...
 >
 > a year at least (i have it in my October 2020 backup)

I wonder whether you had written it in response to some change in either
Emacs or GNOME.

 > I've noticed the problem since march last year - the mutter-wayland folk
 > i chatted with at that time disclaimed any notion of "iconify", saying
 > hiding a window does not change the state

Remarkable.  We should and do get Expose events for such windows though.
But we lack UnmapNotify and VisibilityNotify events for them.  One major
problem is that they seem to care about Wayland only now.  Have you ever
tried the pgtk branch and how it behaves in this regard?

 > and this is required for
 > live-previews when switching workspaces, etc. though they claimed full
 > support for ewmh NET_WM_STATE_HIDDEN

And IIUC we have troubles with that as well.  x_get_current_wm_state is
not reliable in my experience (or maybe we don't call it often enough).

 > no, I couldn't figure out how to make raise-frame (select a different
 > frame and set input focus) work on mutter-wayland with gtkonly emacs.

`raise-frame' calls Fmake_frame_visible.  Are advices not working when
called from C?  Anyhow, you should try to do those advices in C in the
first place - that's what I tried in the patch I sent to Dmitry and it
works for `raise-frame' here (but causes havoc under xfwm).

 > gdk's calls do not work.

In general?  We do use them all the time.

 > I think that's being punted, and relief is expected from some
 > "xdg-activation protocol"

Has that been implemented already?  The pgtk branch should probably know
about it first.

 > The mutter model means invisible frames are still supposed to keep
 > rendering - they just wont be composited on the main workspace.

But how comes then that when users iconify a frame by clicking at the
title bar icon or when they switch workspaces they get problems when
deiconyfing a frame or switching workspace again.  All that time Emacs
should think of the frame as being exposed, visible, ...  Is that
x_get_current_wm_state striking again so Emacs _is_ somehow aware of the
frame not being really visible and waits for a signal that the frame has
become visible again?

Basically we can adopt a model where Emacs thinks all the time that a
frame is not visible and always asks to make it visible when it thinks
that the frame should be visible.  Just that `frame-list' could never
tell in such a model what the visible and invisible frames are ...

 > [I try use it once in a while - try to get it to do what I want,
 > encounter problems, give up for a month, etc.]

One evidence has become clear enough though: Our mutter clients don't
use multiple frames.

Thanks, martin

Re: Deiconifying GTK frames on GNOME shell

[Dmitry Gutov]

On 10.09.2021 11:33, martin rudalics wrote:
>  > If I don't do (1), BTW, I can do (2) twice, and that also makes the 
> frame visible.
>  >
>  > Meaning
>  >
>  >    (make-frame-invisible frame)
>  >    (make-frame-visible frame)
>  >    (make-frame-invisible frame)
>  >    (make-frame-visible frame)
> 
> I attach a patch based on this recipe but it causes (occasionally
> terrible) flickers on other desktops so I would have to special-case it
> on mutter anyway.  I still have no good idea what's really going on
> here.

The patch seems to work, fixing the scenario above. Thanks.

Not sure at which step the flickers are supposed to appear - probably in 
some more complex scenarios?

Perhaps related to the topic of the discussion: I very rarely use 
multiple frames myself (child frames excepted), but I'm happy to help 
troubleshoot bugs under GNOME anyway.

Re: Deiconifying GTK frames on GNOME shell

[martin rudalics]

 > Not sure at which step the flickers are supposed to appear - probably in some more complex scenarios?

They occur with xfwm and, in a less annoying way, with plasma.  I
haven't seen them with mutter.

 > Perhaps related to the topic of the discussion: I very rarely use
 > multiple frames myself (child frames excepted), but I'm happy to help
 > troubleshoot bugs under GNOME anyway.

Thanks.  Users that should be bitten most by this are those using
`frame-auto-hide-function'.

martin

Re: Deiconifying GTK frames on GNOME shell

[Madhu]

[-- Attachment #1: Type: Text/Plain, Size: 2333 bytes --]

[sorry for the long delay]

*  martin rudalics <af8095d4-7b2f-32aa-342d-fcda36964257@gmx.at>
Wrote on Sat, 11 Sep 2021 10:38:12 +0200

[snip]

> > No, I couldn't figure out how to make raise-frame (select a different
> > frame and set input focus) work on mutter-wayland with gtkonly emacs.
>
> `raise-frame' calls Fmake_frame_visible.  Are advices not working when
> called from C?  Anyhow, you should try to do those advices in C in the
> first place - that's what I tried in the patch I sent to Dmitry and it
> works for `raise-frame' here (but causes havoc under xfwm).
>
> > gdk's calls do not work.
>
> In general?  We do use them all the time.

No, just for raising the window. The problem was in the pgtk branch:
select-frame-set-input-focus would not select the frame raise it and
set the input focus.

> > I think that's being punted, and relief is expected from some
> > "xdg-activation protocol"
>
> Has that been implemented already?  The pgtk branch should probably
> know about it first.

I couldn't spot it, but there is apparently a workaround for gtk3,
mentioned in the issue "Allow raising application accessory windows
under Wayland (#730)"
https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/730 (needs
javascript)

a solution is mentioned by @chergert (3 years ago) "Okay, so this
appears to work fine to raise the secondary window if i just fake the
timestamp. Not what I want to do, but it means that plumbing can
support this just fine (which I thought was broken)."

which for him meant, instead of using gtk_window_present(), to instead use
#+BEGIN_SRC
  /* TODO: We need the last event time to do this properly. Until then,
   * we'll just fake some timing info to workaround wayland issues.
   */
  gtk_window_present_with_time (window, g_get_monotonic_time () / 1000L);
#+END_SRC

(from gnome-builder/src/libide/gui/ide-gui-global.c)


Implementing that in pgtk with the attached patch seems to work. To
test:

emacs -Q -l test.el

(setq $a (make-frame)); move the new frame out of the way and
;; go back to the first frame.
(select-frame-set-input-focus $a)
;; after the patch this sets the input focus - on mutter 41 the cursor
;; is still over the old window

I'm hoping a pgtk user can test this/comment on if this is a right
approach?

---Madhu

https://bugzilla.redhat.com/show_bug.cgi?id=1349225

[-- Attachment #2: pgtk-workaround-frame-focus-on-wayland.patch --]
[-- Type: Text/X-Patch, Size: 873 bytes --]

From 8f3fcc9e80fbff04165c6a3f8bcc05b91c270268 Mon Sep 17 00:00:00 2001
From: Madhu <enometh@net.meer>
Date: Mon, 6 Dec 2021 20:55:45 +0530
Subject: [PATCH] pgtkterm: workaround frame focus on wayland

* src/pgktterm.c: (pgtk_focus_frame): force wayland to raise the frame
with a call to gtk_window_present_with_time with a faked up timestamp.
---
 src/pgtkterm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/pgtkterm.c b/src/pgtkterm.c
index bf863c8474..499a4dd03a 100644
--- a/src/pgtkterm.c
+++ b/src/pgtkterm.c
@@ -4381,6 +4381,10 @@ pgtk_focus_frame (struct frame *f, bool noactivate)
     {
       block_input ();
       gtk_widget_grab_focus (wid);
+      if (FRAME_GTK_OUTER_WIDGET (f))
+	{
+	  gtk_window_present_with_time (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)), g_get_monotonic_time () / 1000L);
+	}
       unblock_input ();
     }
 }
-- 
2.31.0

Re: Deiconifying GTK frames on GNOME shell

[martin rudalics]

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

 > Implementing that in pgtk with the attached patch seems to work. To
 > test:
 >
 > emacs -Q -l test.el
 >
 > (setq $a (make-frame)); move the new frame out of the way and
 > ;; go back to the first frame.
 > (select-frame-set-input-focus $a)
 > ;; after the patch this sets the input focus - on mutter 41 the cursor
 > ;; is still over the old window
 >
 > I'm hoping a pgtk user can test this/comment on if this is a right
 > approach?

Yuuki Harano, Po Lu - any comments on that patch?

TIA, martin

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pgtk-workaround-frame-focus-on-wayland.patch --]
[-- Type: text/x-patch; name="pgtk-workaround-frame-focus-on-wayland.patch", Size: 902 bytes --]

>From 8f3fcc9e80fbff04165c6a3f8bcc05b91c270268 Mon Sep 17 00:00:00 2001
From: Madhu <enometh@net.meer>
Date: Mon, 6 Dec 2021 20:55:45 +0530
Subject: [PATCH] pgtkterm: workaround frame focus on wayland

* src/pgktterm.c: (pgtk_focus_frame): force wayland to raise the frame
with a call to gtk_window_present_with_time with a faked up timestamp.
---
 src/pgtkterm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/pgtkterm.c b/src/pgtkterm.c
index bf863c8474..499a4dd03a 100644
--- a/src/pgtkterm.c
+++ b/src/pgtkterm.c
@@ -4381,6 +4381,10 @@ pgtk_focus_frame (struct frame *f, bool noactivate)
     {
       block_input ();
       gtk_widget_grab_focus (wid);
+      if (FRAME_GTK_OUTER_WIDGET (f))
+	{
+	  gtk_window_present_with_time (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)), g_get_monotonic_time () / 1000L);
+	}
       unblock_input ();
     }
 }
--
2.31.0

Re: Deiconifying GTK frames on GNOME shell

[Po Lu]

martin rudalics <rudalics@gmx.at> writes:

> Yuuki Harano, Po Lu - any comments on that patch?
>
> TIA, martin

Could you explain what it is supposed to fix?

I thought the reason frames couldn't be raised under PGTK was that
`frame_raise_lower_hook' is unimplemented there, which someone will
certainly get to soon.

Thanks.


> +	  gtk_window_present_with_time (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)), g_get_monotonic_time () / 1000L);

I think GDK_CURRENT_TIME should work equally well here, right?

Re: Deiconifying GTK frames on GNOME shell

[Yuuki Harano]

On Wed, 8 Dec 2021 12:00:56 +0100,
	martin rudalics <rudalics@gmx.at> wrote:
>> I'm hoping a pgtk user can test this/comment on if this is a right
>> approach?
> 
> Yuuki Harano, Po Lu - any comments on that patch?

I think it's good.

It's working on GNOME(Wayland), but not Wayfire.
I don't test on Plasma and Sway.

Also, I tested with gtk_window_present instead of gtk_window_present_with_time.
It did not work on GNOME(Wayland).


On Wed, 08 Dec 2021 19:09:30 +0800,
	Po Lu <luangruo@yahoo.com> wrote:
> Could you explain what it is supposed to fix?

See this thread:
 https://lists.gnu.org/archive/html/emacs-devel/2021-12/msg00549.html
 https://lists.gnu.org/archive/html/emacs-devel/2021-09/msg00909.html

-- 
Yuuki Harano

Re: Deiconifying GTK frames on GNOME shell

[Madhu]

*  Po Lu <luangruo@yahoo.com> <87ilvze4k5.fsf@yahoo.com>
Wrote on Wed, 08 Dec 2021 19:09:30 +0800
> martin rudalics <rudalics@gmx.at> writes:
>> Yuuki Harano, Po Lu - any comments on that patch?
> Could you explain what it is supposed to fix?

Hello, it is meant to make (select-frame-set-input-focus) "work" on
wayland for the time being.  It is not a correct fix.  (x-focus-frame
$a t) will raise the frame when it should not be raised.

> I thought the reason frames couldn't be raised under PGTK was that
> `frame_raise_lower_hook' is unimplemented there, which someone will
> certainly get to soon.

The problem I believe is that the gdk/gtk apis to raise/lower windows
just doesn't work on wayland. Implementing frame_raise_lower_hook with, say
      gdk_window_raise (gtk_widget_get_window (GTK_WIDGET (FRAME_GTK_OUTER_WIDGET (f))));
has no effect.

I linked to two urls pointing to the bug reports on frame stacking
order. (Maybe I'm misunderstanding but there are at least a dozen
more reports)

it is a wrong fix, but on the other hand select-frame-set-input-focus
seems to be crucial (for me) for using more than 2 frames

>> +	  gtk_window_present_with_time (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)), g_get_monotonic_time () / 1000L);
>
> I think GDK_CURRENT_TIME should work equally well here, right?

I'll check shortly, Thanks,

---Best Regards
Madhu

Re: Deiconifying GTK frames on GNOME shell

[Po Lu]

Yuuki Harano <masm+emacs@masm11.me> writes:

> I think it's good.
>
> It's working on GNOME(Wayland), but not Wayfire.  I don't test on
> Plasma and Sway.
>
> Also, I tested with gtk_window_present instead of
> gtk_window_present_with_time.  It did not work on GNOME(Wayland).

> See this thread:
>  https://lists.gnu.org/archive/html/emacs-devel/2021-12/msg00549.html
>  https://lists.gnu.org/archive/html/emacs-devel/2021-09/msg00909.html

Thanks, but could you test with GDK_CURRENT_TIME as the `time' argument
to `gtk_window_present_with_time'?

Re: Deiconifying GTK frames on GNOME shell

[Po Lu]

Madhu <enometh@meer.net> writes:

> Hello, it is meant to make (select-frame-set-input-focus) "work" on
> wayland for the time being.  It is not a correct fix.  (x-focus-frame
> $a t) will raise the frame when it should not be raised.

That's because the GDK developers, in their infinite wisdom, think that
windows shouldn't give the input focus to themselves without also
presenting themselves.

> The problem I believe is that the gdk/gtk apis to raise/lower windows
> just doesn't work on wayland. Implementing frame_raise_lower_hook with, say
>       gdk_window_raise (gtk_widget_get_window (GTK_WIDGET (FRAME_GTK_OUTER_WIDGET (f))));
> has no effect.

If it doesn't work on Wayland, at least it will work on X-Windows, or
Broadway, or some other window system supported by GDK.

I see no reason to keep it unimplemented.

Thanks.

Re: Deiconifying GTK frames on GNOME shell

[Madhu]

*  Po Lu <luangruo@yahoo.com> <87ilvyd3kd.fsf@yahoo.com>
Wrote on Thu, 09 Dec 2021 08:28:34 +0800

> Thanks, but could you test with GDK_CURRENT_TIME as the `time' argument
> to `gtk_window_present_with_time'?

It doesn't seem to work. The gitlab gnome-shell issue #730 seems to
think it is a GTK issue

Re: Deiconifying GTK frames on GNOME shell

[Madhu]

[-- Attachment #1: Type: Text/Plain, Size: 995 bytes --]

*  Po Lu <luangruo@yahoo.com> <87ee6md3g9.fsf@yahoo.com>
Wrote on Thu, 09 Dec 2021 08:31:02 +0800
> Madhu <enometh@meer.net> writes:
>> Hello, it is meant to make (select-frame-set-input-focus) "work" on
>> wayland for the time being.  It is not a correct fix.  (x-focus-frame
>> $a t) will raise the frame when it should not be raised.
>
> That's because the GDK developers, in their infinite wisdom, think that
> windows shouldn't give the input focus to themselves without also
> presenting themselves.
>
>> The problem I believe is that the gdk/gtk apis to raise/lower windows
>> just doesn't work on wayland. Implementing frame_raise_lower_hook with, say
>>       gdk_window_raise (gtk_widget_get_window (GTK_WIDGET (FRAME_GTK_OUTER_WIDGET (f))));
>> has no effect.
>
> If it doesn't work on Wayland, at least it will work on X-Windows, or
> Broadway, or some other window system supported by GDK.
>
> I see no reason to keep it unimplemented.

FWIW this is the patch I tried, which failed

[-- Attachment #2: 0001-src-pgtkterm.c-implement-frame-raise-lower-FAIL.patch --]
[-- Type: Text/X-Patch, Size: 2057 bytes --]

From e304e644aeab0732ebe3f6891e155d0939575122 Mon Sep 17 00:00:00 2001
From: Madhu <enometh@net.meer>
Date: Wed, 8 Dec 2021 21:39:59 +0530
Subject: [PATCH] src/pgtkterm.c - implement frame raise lower - FAIL

* src/pgtkterm.c: (pgtk_raise_frame, pgtk_lower_frame,
pgtk_frame_raise_lower): new.  pgtk_create_terminal: hook into
terminal->frame_raise_lower_hook
---
 src/pgtkterm.c | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/src/pgtkterm.c b/src/pgtkterm.c
index 0d9b342e6e..fd193f9e0b 100644
--- a/src/pgtkterm.c
+++ b/src/pgtkterm.c
@@ -195,6 +195,43 @@ frame_set_mouse_pixel_position (struct frame *f, int pix_x, int pix_y)
 {
 }
 
+
+/* Raise frame F.  */
+
+static void
+pgtk_raise_frame (struct frame *f)
+{
+
+  if (FRAME_VISIBLE_P (f)) {
+    block_input ();
+    if (FRAME_GTK_OUTER_WIDGET (f))
+      gdk_window_raise (gtk_widget_get_window (GTK_WIDGET (FRAME_GTK_OUTER_WIDGET (f))));
+    unblock_input ();
+  }
+}
+
+/* Lower frame F.  */
+
+static void
+pgtk_lower_frame (struct frame *f)
+{
+  if (FRAME_VISIBLE_P (f))
+    {
+      block_input ();
+      gdk_window_lower (gtk_widget_get_window (GTK_WIDGET (FRAME_GTK_OUTER_WIDGET (f))));
+      unblock_input ();
+    }
+}
+
+static void
+pgtk_frame_raise_lower (struct frame *f, bool raise_flag)
+{
+  if (raise_flag)
+    pgtk_raise_frame (f);
+  else
+    pgtk_lower_frame (f);
+}
+
 /* Free X resources of frame F.  */
 
 void
@@ -4675,7 +4712,7 @@ pgtk_create_terminal (struct pgtk_display_info *dpyinfo)
   terminal->mouse_position_hook = pgtk_mouse_position;
   terminal->frame_rehighlight_hook = XTframe_rehighlight;
   terminal->buffer_flipping_unblocked_hook = pgtk_buffer_flipping_unblocked_hook;
-  /* terminal->frame_raise_lower_hook = pgtk_frame_raise_lower; */
+  terminal->frame_raise_lower_hook = pgtk_frame_raise_lower;
   terminal->frame_visible_invisible_hook = pgtk_make_frame_visible_invisible;
   terminal->fullscreen_hook = pgtk_fullscreen_hook;
   terminal->menu_show_hook = pgtk_menu_show;
-- 
2.31.0

Re: Deiconifying GTK frames on GNOME shell

[Po Lu]

Madhu <enometh@meer.net> writes:

> *  Po Lu <luangruo@yahoo.com> <87ilvyd3kd.fsf@yahoo.com>
> Wrote on Thu, 09 Dec 2021 08:28:34 +0800
>
>> Thanks, but could you test with GDK_CURRENT_TIME as the `time' argument
>> to `gtk_window_present_with_time'?
>
> It doesn't seem to work. The gitlab gnome-shell issue #730 seems to
> think it is a GTK issue

Then I think your patch is OK, bar some formatting problems: sentences
in commit messages aside from the title should end with punctuation, and
you should try to keep code within 70 columns.

It should be exempt from copyright assignment, adding or changing less
than 15 lines of code, but you may have exceeded that limit already.

Eli or Lars, has Madhu completed the copyright assignment?  And if not,
is this change exempt from copyright assignment?  Thanks.

Re: Deiconifying GTK frames on GNOME shell

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: rudalics@gmx.at,  masm+emacs@masm11.me,  emacs-devel@gnu.org, Eli
>  Zaretskii <eliz@gnu.org>, Lars Ingebrigtsen <larsi@gnus.org>
> Date: Thu, 09 Dec 2021 10:46:27 +0800
> 
> It should be exempt from copyright assignment, adding or changing less
> than 15 lines of code, but you may have exceeded that limit already.
> 
> Eli or Lars, has Madhu completed the copyright assignment?

Yes, his assignment is on file.