bug#61525: 29.0.60; delete-frame will raise frames in another virtual desktop

bug#61525: 29.0.60; delete-frame will raise frames in another virtual desktop

[Kai Ma]

On MacOS, delete-frame can raise a frame in another virtual desktop,
which will cause switching between desktops.  This is annoying,
especially for emacsclient users.

To quote frame.c:

#ifdef NS_IMPL_COCOA
      else
	/* Under NS, there is no system mechanism for choosing a new
	   window to get focus -- it is left to application code.
	   So the portion of THIS application interfacing with NS
	   needs to know about it.  We call Fraise_frame, but the
	   purpose is really to transfer focus.  */
	Fraise_frame (frame1);
#endif

However, this has an undesired side effect: the desktop will be
switched.

Steps to reproduce:

1. Run emacs.
2. C-x 5 2, and move the new frame to another desktop.
3. C-x 5 0.  Now you see the desktop is switched.

This is a known issue for some time [1] and there is a patch that simply
disables raise_frame [2].  I'm not sure whether this patch is entirely
correct (and I think not).  But I do think the behavior should be
improved; at least the undesired desktop switching should be avoided.

Best regards,
Kai

[1] https://xenodium.com/no-emacs-frame-refocus-on-macos/
[2] https://github.com/d12frosted/homebrew-emacs-plus/blob/master/patches/emacs-28/no-frame-refocus-cocoa.patch

bug#61525: 29.0.60; delete-frame will raise frames in another virtual desktop

[Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Kai Ma <justksqsf@gmail.com> writes:

> On MacOS, delete-frame can raise a frame in another virtual desktop,
> which will cause switching between desktops.  This is annoying,
> especially for emacsclient users.
>
> To quote frame.c:
>
> #ifdef NS_IMPL_COCOA
>       else
> 	/* Under NS, there is no system mechanism for choosing a new
> 	   window to get focus -- it is left to application code.
> 	   So the portion of THIS application interfacing with NS
> 	   needs to know about it.  We call Fraise_frame, but the
> 	   purpose is really to transfer focus.  */
> 	Fraise_frame (frame1);
> #endif
>
> However, this has an undesired side effect: the desktop will be
> switched.
>
> Steps to reproduce:
>
> 1. Run emacs.
> 2. C-x 5 2, and move the new frame to another desktop.
> 3. C-x 5 0.  Now you see the desktop is switched.
>
> This is a known issue for some time [1] and there is a patch that simply
> disables raise_frame [2].  I'm not sure whether this patch is entirely
> correct (and I think not).  But I do think the behavior should be
> improved; at least the undesired desktop switching should be avoided.
>

One downside of removing raise_frame is that, after you press C-x 5 0,
you cannot start typing right away without first focusing on another
Emacs frame manually.  People would consider this a regression from
previous versions and something that doesn't happen on GNU/Linux.

A possibly better approach could be:

Once the "other" frame to select is chosen by the code that is just
above in frame.c, call some Obj-C code that extracts the EmacsView and
makes it the first responder:

EmacsView *view = FRAME_NS_VIEW (frame1);
[self makeFirstResponder:view];

I haven't tested this, but perhaps this makes the other frame receive
focus without switching desktops in a multi-desktop configuration.

bug#61525: 29.0.60; delete-frame will raise frames in another virtual desktop

[Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Daniel Martín via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

>
> One downside of removing raise_frame is that, after you press C-x 5 0,
> you cannot start typing right away without first focusing on another
> Emacs frame manually.  People would consider this a regression from
> previous versions and something that doesn't happen on GNU/Linux.
>
> A possibly better approach could be:
>
> Once the "other" frame to select is chosen by the code that is just
> above in frame.c, call some Obj-C code that extracts the EmacsView and
> makes it the first responder:
>
> EmacsView *view = FRAME_NS_VIEW (frame1);
> [self makeFirstResponder:view];
>
> I haven't tested this, but perhaps this makes the other frame receive
> focus without switching desktops in a multi-desktop configuration.

The attached patch avoids the unwanted side effects of raise-frame by
making the other frame the key window, instead.  Could you give it a
try?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-NS-Do-not-raise-a-different-frame-when-closing-a-fra.patch --]
[-- Type: text/x-patch, Size: 2313 bytes --]

From 50cb6ed653b17bb5f712872f3f664e1273e498d8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=ADn?= <mardani29@yahoo.es>
Date: Wed, 15 Feb 2023 16:33:14 +0100
Subject: [PATCH] [NS] Do not raise a different frame when closing a frame

* src/frame.h: Declare an NS-only function to make a frame the key
window.
* src/nsfns.m (ns_make_frame_key_window): Implement it.
* src/frame.c (delete_frame): Call ns_make_frame_key_window instead of
Fraise_frame.  (Bug#61525)
---
 src/frame.c | 5 ++---
 src/frame.h | 5 +++++
 src/nsfns.m | 5 +++++
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/frame.c b/src/frame.c
index 983424b0bee..b2319a35aed 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -2152,9 +2152,8 @@ delete_frame (Lisp_Object frame, Lisp_Object force)
 	/* Under NS, there is no system mechanism for choosing a new
 	   window to get focus -- it is left to application code.
 	   So the portion of THIS application interfacing with NS
-	   needs to know about it.  We call Fraise_frame, but the
-	   purpose is really to transfer focus.  */
-	Fraise_frame (frame1);
+	   needs to make the frame we switch to the key window.  */
+	ns_make_frame_key_window (XFRAME (frame1));
 #endif
 
       do_switch_frame (frame1, 1, Qnil);
diff --git a/src/frame.h b/src/frame.h
index b95b94c7685..2eb9de81f0a 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -1380,6 +1380,11 @@ window_system_available (struct frame *f)
 extern void frame_size_history_plain (struct frame *, Lisp_Object);
 extern void frame_size_history_extra (struct frame *, Lisp_Object,
 				      int, int, int, int, int, int);
+#ifdef NS_IMPL_COCOA
+/* Implemented in nsfns.m.  */
+extern void ns_make_frame_key_window (struct frame *f);
+#endif
+
 extern Lisp_Object Vframe_list;
 
 /* Value is a pointer to the selected frame.  If the selected frame
diff --git a/src/nsfns.m b/src/nsfns.m
index 8804a7df7cf..cd41c6095a0 100644
--- a/src/nsfns.m
+++ b/src/nsfns.m
@@ -685,6 +685,11 @@ Turn the input menu (an NSMenu) into a lisp list for tracking on lisp side.
   SET_FRAME_GARBAGED (f);
 }
 
+void ns_make_frame_key_window (struct frame *f)
+{
+  [[FRAME_NS_VIEW (f) window] makeKeyWindow];
+}
+
 /* tabbar support */
 static void
 ns_set_tab_bar_lines (struct frame *f, Lisp_Object value, Lisp_Object oldval)
-- 
2.34.1

bug#61525: 29.0.60; delete-frame will raise frames in another virtual desktop

[Kai Ma]

> On Feb 16, 2023, at 00:41, Daniel Martín <mardani29@yahoo.es> wrote:
> 
> Daniel Martín via "Bug reports for GNU Emacs, the Swiss army knife of
> text editors" <bug-gnu-emacs@gnu.org> writes:
> 
>> 
>> One downside of removing raise_frame is that, after you press C-x 5 0,
>> you cannot start typing right away without first focusing on another
>> Emacs frame manually.  People would consider this a regression from
>> previous versions and something that doesn't happen on GNU/Linux.
>> 
>> A possibly better approach could be:
>> 
>> Once the "other" frame to select is chosen by the code that is just
>> above in frame.c, call some Obj-C code that extracts the EmacsView and
>> makes it the first responder:
>> 
>> EmacsView *view = FRAME_NS_VIEW (frame1);
>> [self makeFirstResponder:view];
>> 
>> I haven't tested this, but perhaps this makes the other frame receive
>> focus without switching desktops in a multi-desktop configuration.
> 
> The attached patch avoids the unwanted side effects of raise-frame by
> making the other frame the key window, instead.  Could you give it a
> try?
> 
> <0001-NS-Do-not-raise-a-different-frame-when-closing-a-fra.patch>

Thanks for the very fast fix!  I can confirm it works for me on macOS 13.1.

bug#61525: 29.0.60; delete-frame will raise frames in another virtual desktop

[Eli Zaretskii]

> Cc: justksqsf@gmail.com
> Date: Wed, 15 Feb 2023 16:41:41 +0100
> From:  Daniel Martín via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Daniel Martín via "Bug reports for GNU Emacs, the Swiss army knife of
> text editors" <bug-gnu-emacs@gnu.org> writes:
> 
> >
> > One downside of removing raise_frame is that, after you press C-x 5 0,
> > you cannot start typing right away without first focusing on another
> > Emacs frame manually.  People would consider this a regression from
> > previous versions and something that doesn't happen on GNU/Linux.
> >
> > A possibly better approach could be:
> >
> > Once the "other" frame to select is chosen by the code that is just
> > above in frame.c, call some Obj-C code that extracts the EmacsView and
> > makes it the first responder:
> >
> > EmacsView *view = FRAME_NS_VIEW (frame1);
> > [self makeFirstResponder:view];
> >
> > I haven't tested this, but perhaps this makes the other frame receive
> > focus without switching desktops in a multi-desktop configuration.
> 
> The attached patch avoids the unwanted side effects of raise-frame by
> making the other frame the key window, instead.  Could you give it a
> try?

Thanks.

> +void ns_make_frame_key_window (struct frame *f)
> +{
> +  [[FRAME_NS_VIEW (f) window] makeKeyWindow];
> +}

Is this new call guaranteed to exist and work well on all the
supported OS versions where we have the NS build?  I wouldn't want to
fix this on some systems and break it on others at the same time.

Alternatively, can we come up with a change that does both what the
old code did and this addition?  That old code did work at some point,
I presume?

bug#61525: 29.0.60; delete-frame will raise frames in another virtual desktop

[Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> +void ns_make_frame_key_window (struct frame *f)
>> +{
>> +  [[FRAME_NS_VIEW (f) window] makeKeyWindow];
>> +}
>
> Is this new call guaranteed to exist and work well on all the
> supported OS versions where we have the NS build?  I wouldn't want to
> fix this on some systems and break it on others at the same time.

makeKeyWindow is a very old API that should be available on every macOS
and GNUstep we support, AFAIK.  I see usages of that API in other parts
of the NS build, in things as central as frame creation, and they are
not protected by any version or system check.

>
> Alternatively, can we come up with a change that does both what the
> old code did and this addition?  That old code did work at some point,
> I presume?

This is not the case of the old code breaking at some point.  It's a
difference in behavior between the GNU/Linux version of Emacs and the NS
version of Emacs.  In GNU/Linux, if you place an Emacs frame in a
separate desktop and press C-x 5 0, the window manager does not
automatically switch to the desktop where the other Emacs frames reside.

I don't know the reason why the NS build has been calling raise-frame
for so long.  I presume it was considered a good enough way to refocus
to another frame after closing one, in a world where virtual desktops
were not very common and the behavior reported by the OP was ignored.

I've attached a revised version of the patch, to fix a couple of code
style issues.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-NS-Do-not-raise-a-different-frame-when-closing-a-fra.patch --]
[-- Type: text/x-patch, Size: 2314 bytes --]

From 07035d7bfadff32d6ff954c221331f2060e0f5fc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=ADn?= <mardani29@yahoo.es>
Date: Wed, 15 Feb 2023 16:33:14 +0100
Subject: [PATCH] [NS] Do not raise a different frame when closing a frame

* src/frame.h: Declare an NS-only function to make a frame the key
window.
* src/nsfns.m (ns_make_frame_key_window): Implement it.
* src/frame.c (delete_frame): Call ns_make_frame_key_window instead of
Fraise_frame.  (Bug#61525)
---
 src/frame.c | 5 ++---
 src/frame.h | 5 +++++
 src/nsfns.m | 6 ++++++
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/src/frame.c b/src/frame.c
index 983424b0bee..b2319a35aed 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -2152,9 +2152,8 @@ delete_frame (Lisp_Object frame, Lisp_Object force)
 	/* Under NS, there is no system mechanism for choosing a new
 	   window to get focus -- it is left to application code.
 	   So the portion of THIS application interfacing with NS
-	   needs to know about it.  We call Fraise_frame, but the
-	   purpose is really to transfer focus.  */
-	Fraise_frame (frame1);
+	   needs to make the frame we switch to the key window.  */
+	ns_make_frame_key_window (XFRAME (frame1));
 #endif
 
       do_switch_frame (frame1, 1, Qnil);
diff --git a/src/frame.h b/src/frame.h
index b95b94c7685..30c1de20fde 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -1380,6 +1380,11 @@ window_system_available (struct frame *f)
 extern void frame_size_history_plain (struct frame *, Lisp_Object);
 extern void frame_size_history_extra (struct frame *, Lisp_Object,
 				      int, int, int, int, int, int);
+#ifdef NS_IMPL_COCOA
+/* Implemented in nsfns.m.  */
+extern void ns_make_frame_key_window (struct frame *);
+#endif
+
 extern Lisp_Object Vframe_list;
 
 /* Value is a pointer to the selected frame.  If the selected frame
diff --git a/src/nsfns.m b/src/nsfns.m
index 8804a7df7cf..96434230cc6 100644
--- a/src/nsfns.m
+++ b/src/nsfns.m
@@ -685,6 +685,12 @@ Turn the input menu (an NSMenu) into a lisp list for tracking on lisp side.
   SET_FRAME_GARBAGED (f);
 }
 
+void
+ns_make_frame_key_window (struct frame *f)
+{
+  [[FRAME_NS_VIEW (f) window] makeKeyWindow];
+}
+
 /* tabbar support */
 static void
 ns_set_tab_bar_lines (struct frame *f, Lisp_Object value, Lisp_Object oldval)
-- 
2.34.1

bug#61525: 29.0.60; delete-frame will raise frames in another virtual desktop

[Eli Zaretskii]

> From: Daniel Martín <mardani29@yahoo.es>
> Cc: 61525@debbugs.gnu.org,  justksqsf@gmail.com
> Date: Thu, 16 Feb 2023 00:48:59 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> +void ns_make_frame_key_window (struct frame *f)
> >> +{
> >> +  [[FRAME_NS_VIEW (f) window] makeKeyWindow];
> >> +}
> >
> > Is this new call guaranteed to exist and work well on all the
> > supported OS versions where we have the NS build?  I wouldn't want to
> > fix this on some systems and break it on others at the same time.
> 
> makeKeyWindow is a very old API that should be available on every macOS
> and GNUstep we support, AFAIK.  I see usages of that API in other parts
> of the NS build, in things as central as frame creation, and they are
> not protected by any version or system check.

Does this include the behavior?  That is, does that call behave the
same on all those versions?

> I don't know the reason why the NS build has been calling raise-frame
> for so long.  I presume it was considered a good enough way to refocus
> to another frame after closing one, in a world where virtual desktops
> were not very common and the behavior reported by the OP was ignored.

Did you try looking at Git history for this code?  Maybe the log
messages of the relevant commits and/or bug reports and/or discussions
on emacs-devel around the dates of the commits tell something about
the reasons?  I've seen too many cases where changing old code
introduced regressions because some aspect of the behavior was
disregarded, and would like to avoid that, certainly if this is for
the release branch.

Thanks.

bug#61525: 29.0.60; delete-frame will raise frames in another virtual desktop

[Kai Ma]

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


> On Feb 16, 2023, at 17:10, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> Did you try looking at Git history for this code?  Maybe the log
> messages of the relevant commits and/or bug reports and/or discussions
> on emacs-devel around the dates of the commits tell something about
> the reasons?  I've seen too many cases where changing old code
> introduced regressions because some aspect of the behavior was
> disregarded, and would like to avoid that, certainly if this is for
> the release branch.

FWIW, the relevant commits:
 . edfda78355 first appeared as part of do_switch_frame
 . 06302656f3 changed
 . ac71ced75b moved to delete_frame


[-- Attachment #2: Type: text/html, Size: 5973 bytes --]

bug#61525: 29.0.60; delete-frame will raise frames in another virtual desktop

[Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>
> Does this include the behavior?  That is, does that call behave the
> same on all those versions?
>

I'd say yes, although as Apple software is non-free, it's difficult to
be 100% sure.

>
> Did you try looking at Git history for this code?  Maybe the log
> messages of the relevant commits and/or bug reports and/or discussions
> on emacs-devel around the dates of the commits tell something about
> the reasons?  I've seen too many cases where changing old code
> introduced regressions because some aspect of the behavior was
> disregarded, and would like to avoid that, certainly if this is for
> the release branch.

I'm OK to wait for if someone perhaps has a better idea to solve this
issue.  In any case, as the pretest is so close, I think that any change
in the area should go to Emacs 30.

bug#61525: 29.0.60; delete-frame will raise frames in another virtual desktop

[Eli Zaretskii]

> From: Daniel Martín <mardani29@yahoo.es>
> Cc: 61525@debbugs.gnu.org,  justksqsf@gmail.com
> Date: Thu, 16 Feb 2023 23:40:47 +0100
> 
> > Did you try looking at Git history for this code?  Maybe the log
> > messages of the relevant commits and/or bug reports and/or discussions
> > on emacs-devel around the dates of the commits tell something about
> > the reasons?  I've seen too many cases where changing old code
> > introduced regressions because some aspect of the behavior was
> > disregarded, and would like to avoid that, certainly if this is for
> > the release branch.
> 
> I'm OK to wait for if someone perhaps has a better idea to solve this
> issue.  In any case, as the pretest is so close, I think that any change
> in the area should go to Emacs 30.

It's fine by me to install this on master.

bug#61525: 29.0.60; delete-frame will raise frames in another virtual desktop

[Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Daniel Martín <mardani29@yahoo.es> writes:
>
>>> +void ns_make_frame_key_window (struct frame *f)
>>> +{
>>> +  [[FRAME_NS_VIEW (f) window] makeKeyWindow];
>>> +}
>>
>> Is this new call guaranteed to exist and work well on all the
>> supported OS versions where we have the NS build?  I wouldn't want to
>> fix this on some systems and break it on others at the same time.
>
> makeKeyWindow is a very old API that should be available on every macOS
> and GNUstep we support, AFAIK.  I see usages of that API in other parts
> of the NS build, in things as central as frame creation, and they are
> not protected by any version or system check.
>

Got a report about some problems closing frames when Emacs runs inside a
macOS terminal.  So here's a new version of the patch that protects the
code that makes the other frame the key window, so that it only runs
when the frame is a GUI frame.

Kai Ma, could you give it a try and see if everything works well now?
Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-PATCH-NS-Do-not-raise-a-different-frame-when-closing.patch --]
[-- Type: text/x-patch, Size: 2631 bytes --]

From 74a22d417beb86bccee8886660caa0d12b052576 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=ADn?= <mardani29@yahoo.es>
Date: Fri, 18 Aug 2023 15:03:21 +0200
Subject: [PATCH] [PATCH] [NS] Do not raise a different frame when closing a
 frame

* src/frame.h: Declare an NS-only function to make a frame the key
window.
* src/nsfns.m (ns_make_frame_key_window): Implement it.
* src/frame.c (delete_frame): Call ns_make_frame_key_window instead of
Fraise_frame.  (Bug#61525)
---
 src/frame.c | 15 +++++++++------
 src/frame.h |  4 ++++
 src/nsfns.m |  6 ++++++
 3 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/src/frame.c b/src/frame.c
index da00cbf4bce..addeb013b4a 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -2212,12 +2212,15 @@ delete_frame (Lisp_Object frame, Lisp_Object force)
 	}
 #ifdef NS_IMPL_COCOA
       else
-	/* Under NS, there is no system mechanism for choosing a new
-	   window to get focus -- it is left to application code.
-	   So the portion of THIS application interfacing with NS
-	   needs to know about it.  We call Fraise_frame, but the
-	   purpose is really to transfer focus.  */
-	Fraise_frame (frame1);
+	{
+	  /* Under NS, there is no system mechanism for choosing a new
+	     window to get focus -- it is left to application code.
+	     So the portion of THIS application interfacing with NS
+	     needs to make the frame we switch to the key window.  */
+	  struct frame *f1 = XFRAME (frame1);
+	  if (FRAME_NS_P (f1))
+	    ns_make_frame_key_window (f1);
+	}
 #endif
 
       do_switch_frame (frame1, 0, 1, Qnil);
diff --git a/src/frame.h b/src/frame.h
index c85df378da6..f4726f1c0e5 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -1521,6 +1521,10 @@ window_system_available (struct frame *f)
 extern void frame_size_history_plain (struct frame *, Lisp_Object);
 extern void frame_size_history_extra (struct frame *, Lisp_Object,
 				      int, int, int, int, int, int);
+#ifdef NS_IMPL_COCOA
+/* Implemented in nsfns.m.  */
+extern void ns_make_frame_key_window (struct frame *);
+#endif
 extern Lisp_Object Vframe_list;
 
 /* Value is a pointer to the selected frame.  If the selected frame
diff --git a/src/nsfns.m b/src/nsfns.m
index b846b490ff7..a79892f73b6 100644
--- a/src/nsfns.m
+++ b/src/nsfns.m
@@ -685,6 +685,12 @@ Turn the input menu (an NSMenu) into a lisp list for tracking on lisp side.
   SET_FRAME_GARBAGED (f);
 }
 
+void
+ns_make_frame_key_window (struct frame *f)
+{
+  [[FRAME_NS_VIEW (f) window] makeKeyWindow];
+}
+
 /* tabbar support */
 static void
 ns_set_tab_bar_lines (struct frame *f, Lisp_Object value, Lisp_Object oldval)
-- 
2.40.1

bug#61525: 29.0.60; delete-frame will raise frames in another virtual desktop

[Kai Ma]

> On Aug 18, 2023, at 21:20, Daniel Martín <mardani29@yahoo.es> wrote:
> 
> Daniel Martín <mardani29@yahoo.es> writes:
>> 
>>>> +void ns_make_frame_key_window (struct frame *f)
>>>> +{
>>>> +  [[FRAME_NS_VIEW (f) window] makeKeyWindow];
>>>> +}
>>> 
>>> Is this new call guaranteed to exist and work well on all the
>>> supported OS versions where we have the NS build?  I wouldn't want to
>>> fix this on some systems and break it on others at the same time.
>> 
>> makeKeyWindow is a very old API that should be available on every macOS
>> and GNUstep we support, AFAIK.  I see usages of that API in other parts
>> of the NS build, in things as central as frame creation, and they are
>> not protected by any version or system check.
>> 
> 
> Got a report about some problems closing frames when Emacs runs inside a
> macOS terminal.  So here's a new version of the patch that protects the
> code that makes the other frame the key window, so that it only runs
> when the frame is a GUI frame.
> 
> Kai Ma, could you give it a try and see if everything works well now?
> Thanks.
> 
> <0001-PATCH-NS-Do-not-raise-a-different-frame-when-closing.patch>

Thanks.  Everything works fine now.

bug#61525: 29.0.60; delete-frame will raise frames in another virtual desktop

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Daniel Martín <mardani29@yahoo.es> writes:

> * src/frame.h: Declare an NS-only function to make a frame the key
> window.
> * src/nsfns.m (ns_make_frame_key_window): Implement it.
> * src/frame.c (delete_frame): Call ns_make_frame_key_window instead of
> Fraise_frame.  (Bug#61525)

This commit message extends into column 70.  Please fill it to 64
columns, or else the ChangeLog entries generated will be incorrectly
formatted.

bug#61525: 29.0.60; delete-frame will raise frames in another virtual desktop

[Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Po Lu <luangruo@yahoo.com> writes:

> Daniel Martín <mardani29@yahoo.es> writes:
>
>> * src/frame.h: Declare an NS-only function to make a frame the key
>> window.
>> * src/nsfns.m (ns_make_frame_key_window): Implement it.
>> * src/frame.c (delete_frame): Call ns_make_frame_key_window instead of
>> Fraise_frame.  (Bug#61525)
>
> This commit message extends into column 70.  Please fill it to 64
> columns, or else the ChangeLog entries generated will be incorrectly
> formatted.

I've attached a new patch with the commit message wrapped at 64 columns.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-NS-Do-not-raise-a-different-frame-when-closing-a-fra.patch --]
[-- Type: text/x-patch, Size: 2622 bytes --]

From 3f8d0ee00ab9392d27831d64ef154c92b2b60608 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=ADn?= <mardani29@yahoo.es>
Date: Fri, 18 Aug 2023 15:03:21 +0200
Subject: [PATCH] [NS] Do not raise a different frame when closing a frame

* src/frame.h: Declare an NS-only function to make a frame the
key window.
* src/nsfns.m (ns_make_frame_key_window): Implement it.
* src/frame.c (delete_frame): Call ns_make_frame_key_window
instead of Fraise_frame.  (Bug#61525)
---
 src/frame.c | 15 +++++++++------
 src/frame.h |  4 ++++
 src/nsfns.m |  6 ++++++
 3 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/src/frame.c b/src/frame.c
index da00cbf4bce..addeb013b4a 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -2212,12 +2212,15 @@ delete_frame (Lisp_Object frame, Lisp_Object force)
 	}
 #ifdef NS_IMPL_COCOA
       else
-	/* Under NS, there is no system mechanism for choosing a new
-	   window to get focus -- it is left to application code.
-	   So the portion of THIS application interfacing with NS
-	   needs to know about it.  We call Fraise_frame, but the
-	   purpose is really to transfer focus.  */
-	Fraise_frame (frame1);
+	{
+	  /* Under NS, there is no system mechanism for choosing a new
+	     window to get focus -- it is left to application code.
+	     So the portion of THIS application interfacing with NS
+	     needs to make the frame we switch to the key window.  */
+	  struct frame *f1 = XFRAME (frame1);
+	  if (FRAME_NS_P (f1))
+	    ns_make_frame_key_window (f1);
+	}
 #endif
 
       do_switch_frame (frame1, 0, 1, Qnil);
diff --git a/src/frame.h b/src/frame.h
index c85df378da6..f4726f1c0e5 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -1521,6 +1521,10 @@ window_system_available (struct frame *f)
 extern void frame_size_history_plain (struct frame *, Lisp_Object);
 extern void frame_size_history_extra (struct frame *, Lisp_Object,
 				      int, int, int, int, int, int);
+#ifdef NS_IMPL_COCOA
+/* Implemented in nsfns.m.  */
+extern void ns_make_frame_key_window (struct frame *);
+#endif
 extern Lisp_Object Vframe_list;
 
 /* Value is a pointer to the selected frame.  If the selected frame
diff --git a/src/nsfns.m b/src/nsfns.m
index b846b490ff7..a79892f73b6 100644
--- a/src/nsfns.m
+++ b/src/nsfns.m
@@ -685,6 +685,12 @@ Turn the input menu (an NSMenu) into a lisp list for tracking on lisp side.
   SET_FRAME_GARBAGED (f);
 }
 
+void
+ns_make_frame_key_window (struct frame *f)
+{
+  [[FRAME_NS_VIEW (f) window] makeKeyWindow];
+}
+
 /* tabbar support */
 static void
 ns_set_tab_bar_lines (struct frame *f, Lisp_Object value, Lisp_Object oldval)
-- 
2.40.1

bug#61525: 29.0.60; delete-frame will raise frames in another virtual desktop

[Eli Zaretskii]

> From: Daniel Martín <mardani29@yahoo.es>
> Cc: Eli Zaretskii <eliz@gnu.org>,  61525@debbugs.gnu.org,  justksqsf@gmail.com
> Date: Sat, 19 Aug 2023 10:28:48 +0200
> 
> Po Lu <luangruo@yahoo.com> writes:
> 
> > Daniel Martín <mardani29@yahoo.es> writes:
> >
> >> * src/frame.h: Declare an NS-only function to make a frame the key
> >> window.
> >> * src/nsfns.m (ns_make_frame_key_window): Implement it.
> >> * src/frame.c (delete_frame): Call ns_make_frame_key_window instead of
> >> Fraise_frame.  (Bug#61525)
> >
> > This commit message extends into column 70.  Please fill it to 64
> > columns, or else the ChangeLog entries generated will be incorrectly
> > formatted.
> 
> I've attached a new patch with the commit message wrapped at 64 columns.

Thanks, installed on the master branch, and closing the bug.

bug#61525: 29.0.60; delete-frame will raise frames in another virtual desktop

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: justksqsf@gmail.com
>> Date: Wed, 15 Feb 2023 16:41:41 +0100
>> From:  Daniel Martín via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> Daniel Martín via "Bug reports for GNU Emacs, the Swiss army knife of
>> text editors" <bug-gnu-emacs@gnu.org> writes:
>> 
>> >
>> > One downside of removing raise_frame is that, after you press C-x 5 0,
>> > you cannot start typing right away without first focusing on another
>> > Emacs frame manually.  People would consider this a regression from
>> > previous versions and something that doesn't happen on GNU/Linux.
>> >
>> > A possibly better approach could be:
>> >
>> > Once the "other" frame to select is chosen by the code that is just
>> > above in frame.c, call some Obj-C code that extracts the EmacsView and
>> > makes it the first responder:
>> >
>> > EmacsView *view = FRAME_NS_VIEW (frame1);
>> > [self makeFirstResponder:view];
>> >
>> > I haven't tested this, but perhaps this makes the other frame receive
>> > focus without switching desktops in a multi-desktop configuration.
>> 
>> The attached patch avoids the unwanted side effects of raise-frame by
>> making the other frame the key window, instead.  Could you give it a
>> try?
>
> Thanks.
>
>> +void ns_make_frame_key_window (struct frame *f)
>> +{
>> +  [[FRAME_NS_VIEW (f) window] makeKeyWindow];
>> +}

justksqsf@gmail.com, please write:

void
ns_make_frame_key_window (struct frame *f)
{
  ...
}

instead.

> Is this new call guaranteed to exist and work well on all the
> supported OS versions where we have the NS build?  I wouldn't want to
> fix this on some systems and break it on others at the same time.

Yes, this seems to exist in GNUstep (meaning that it should exist in an
old Mac OS as well.)

bug#61525: 29.0.60; delete-frame will raise frames in another virtual desktop

[Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Po Lu via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs@gnu.org> writes:

>
> justksqsf@gmail.com, please write:
>
> void
> ns_make_frame_key_window (struct frame *f)
> {
>   ...
> }
>
> instead.
>

Thanks, I've fixed that in the second version of the patch.