bug#36672: 27.0.50; NS build: Creating child frame leads to empty space

bug#36672: 27.0.50; NS build: Creating child frame leads to empty space

[Andrii Kolomoiets]

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

Assume there are only one space - Desktop
1. emacs -Q
2. M-x toggle-frame-fullscreen
   Now there are two spaces - Desktop and *scratch*
3. eval (make-frame `((parent-frame . ,(window-frame))))
   Now there are three spaces:
   - Desktop
   - Empty space named *scratch* with emacs menu
   - *scratch* with emacs frames but without menu

Although attached patch solves this problem for me these cases still not
handled right way:

1. Removing parent-frame property leaves the frame is same space:

(let ((new-frame (make-frame `((parent-frame . ,(window-frame))))))
  (modify-frame-parameters new-frame `((parent-frame . nil))))

Maybe child frame must go fullscreen if ex-parent frame is in
fullscreen.

2. Setting parent frame after frame creation:

(let ((frame (window-frame))
      (new-frame (make-frame)))
  (modify-frame-parameters new-frame `((parent-frame . ,frame))))

Thanks!

In GNU Emacs 27.0.50 (build 1, x86_64-apple-darwin18.6.0, NS
appkit-1671.50 Version 10.14.5 (Build 18F132))
Windowing system distributor 'Apple', version 10.3.1671
System Description:  Mac OS X 10.14.5


[-- Attachment #2: ns-emacs-spaces.patch --]
[-- Type: application/octet-stream, Size: 1251 bytes --]

diff --git a/src/nsterm.h b/src/nsterm.h
index 9773eb3e66..d16588718e 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -1274,6 +1274,7 @@ extern char gnustep_base_version[];  /* version tracking */
 #if !defined (NS_IMPL_COCOA) || !defined (MAC_OS_X_VERSION_10_7)
 #define NSFullScreenWindowMask                      (1 << 14)
 #define NSWindowCollectionBehaviorFullScreenPrimary (1 << 7)
+#define NSWindowCollectionBehaviorFullScreenAuxiliary (1 << 8)
 #define NSApplicationPresentationFullScreen         (1 << 10)
 #define NSApplicationPresentationAutoHideToolbar    (1 << 11)
 #define NSAppKitVersionNumber10_7                   1138
diff --git a/src/nsterm.m b/src/nsterm.m
index 02331826d9..cc5921090c 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -7365,7 +7375,10 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
   if (NSAppKitVersionNumber >= NSAppKitVersionNumber10_7)
 #endif
-    [win setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary];
+    if (FRAME_PARENT_FRAME (f))
+      [win setCollectionBehavior:NSWindowCollectionBehaviorFullScreenAuxiliary];
+    else
+      [win setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary];
 #endif

   wr = [win frame];

bug#36672: 27.0.50; NS build: Creating child frame leads to empty space

[Alan Third]

On Mon, Jul 15, 2019 at 08:38:31PM +0300, Andrii Kolomoiets wrote:
> Assume there are only one space - Desktop
> 1. emacs -Q
> 2. M-x toggle-frame-fullscreen
>    Now there are two spaces - Desktop and *scratch*
> 3. eval (make-frame `((parent-frame . ,(window-frame))))
>    Now there are three spaces:
>    - Desktop
>    - Empty space named *scratch* with emacs menu
>    - *scratch* with emacs frames but without menu

I hit C-x C-c at this point to exit Emacs and it completely crashed my
session. Not great...

The patch looks good to me, but lets see if we can find solutions to
these other issues. To be honest, given that Apple don’t provide any
way to properly deal with spaces, I’d expect this stuff to be handled
sensibly by default, but I guess that’s too much to ask.

> 1. Removing parent-frame property leaves the frame is same space:
> 
> (let ((new-frame (make-frame `((parent-frame . ,(window-frame))))))
>   (modify-frame-parameters new-frame `((parent-frame . nil))))
> 
> Maybe child frame must go fullscreen if ex-parent frame is in
> fullscreen.

I suppose the best thing to do would be to move it onto the first
space, but there’s no API for that, apparently. Going fullscreen seems
like a reasonable work‐around to me.

> 2. Setting parent frame after frame creation:
> 
> (let ((frame (window-frame))
>       (new-frame (make-frame)))
>   (modify-frame-parameters new-frame `((parent-frame . ,frame))))

What do you see happening in this case? I’ve got spaces turned on and
everything I do just seems to create a new fullscreen space (except
when it crashes my session).

-- 
Alan Third

bug#36672: 27.0.50; NS build: Creating child frame leads to empty space

[martin rudalics]

 >> Assume there are only one space - Desktop
 >> 1. emacs -Q
 >> 2. M-x toggle-frame-fullscreen
 >>     Now there are two spaces - Desktop and *scratch*
 >> 3. eval (make-frame `((parent-frame . ,(window-frame))))
 >>     Now there are three spaces:
 >>     - Desktop
 >>     - Empty space named *scratch* with emacs menu
 >>     - *scratch* with emacs frames but without menu
 >
 > I hit C-x C-c at this point to exit Emacs and it completely crashed my
 > session. Not great...

No backtrace?  It sounds already strange that one apparently gets two
spaces with the same name "*scratch*" here.  Do things return to
"normality" when one deletes the child frame before trying to exit
Emacs?  BTW, does maximizing a frame run into similar problems?

 > The patch looks good to me,

Does it work with two child frames of the same parent?

 > but lets see if we can find solutions to
 > these other issues. To be honest, given that Apple don’t provide any
 > way to properly deal with spaces, I’d expect this stuff to be handled
 > sensibly by default, but I guess that’s too much to ask.
 >
 >> 1. Removing parent-frame property leaves the frame is same space:
 >>
 >> (let ((new-frame (make-frame `((parent-frame . ,(window-frame))))))
 >>    (modify-frame-parameters new-frame `((parent-frame . nil))))
 >>
 >> Maybe child frame must go fullscreen if ex-parent frame is in
 >> fullscreen.

Does making it fullscreen then give it a separate menu?  Note also
that the new parent could be another frame (even a child frame).

 > I suppose the best thing to do would be to move it onto the first
 > space, but there’s no API for that, apparently.

What happens when one sets 'parent-frame' to nil and removes the
'NSWindowCollectionBehaviorFullScreenAuxiliary' simultaneously?

 > Going fullscreen seems
 > like a reasonable work‐around to me.
 >
 >> 2. Setting parent frame after frame creation:
 >>
 >> (let ((frame (window-frame))
 >>        (new-frame (make-frame)))
 >>    (modify-frame-parameters new-frame `((parent-frame . ,frame))))
 >
 > What do you see happening in this case? I’ve got spaces turned on and
 > everything I do just seems to create a new fullscreen space (except
 > when it crashes my session).

Can't one set the 'NSWindowCollectionBehaviorFullScreenAuxiliary' just
as with the initial frame here?

martin

bug#36672: 27.0.50; NS build: Creating child frame leads to empty space

[Andrii Kolomoiets]

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

On Jul 16, 2019, at 22:28, Alan Third <alan@idiocy.org> wrote:
> 
> On Mon, Jul 15, 2019 at 08:38:31PM +0300, Andrii Kolomoiets wrote:
>> Assume there are only one space - Desktop
>> 1. emacs -Q
>> 2. M-x toggle-frame-fullscreen
>>   Now there are two spaces - Desktop and *scratch*
>> 3. eval (make-frame `((parent-frame . ,(window-frame))))
>>   Now there are three spaces:
>>   - Desktop
>>   - Empty space named *scratch* with emacs menu
>>   - *scratch* with emacs frames but without menu
> 
> I hit C-x C-c at this point to exit Emacs and it completely crashed my
> session. Not great...
> 
> The patch looks good to me, but lets see if we can find solutions to
> these other issues. To be honest, given that Apple don’t provide any
> way to properly deal with spaces, I’d expect this stuff to be handled
> sensibly by default, but I guess that’s too much to ask.
> 
>> 1. Removing parent-frame property leaves the frame is same space:
>> 
>> (let ((new-frame (make-frame `((parent-frame . ,(window-frame))))))
>>  (modify-frame-parameters new-frame `((parent-frame . nil))))
>> 
>> Maybe child frame must go fullscreen if ex-parent frame is in
>> fullscreen.
> 
> I suppose the best thing to do would be to move it onto the first
> space, but there’s no API for that, apparently. Going fullscreen seems
> like a reasonable work‐around to me.
> 

I manage to make this code work. Please see attached updated patch.
But this patch is not completely ready as it missing compilation conditions.
Just hope you find it useful.

>> 2. Setting parent frame after frame creation:
>> 
>> (let ((frame (window-frame))
>>      (new-frame (make-frame)))
>>  (modify-frame-parameters new-frame `((parent-frame . ,frame))))
> 
> What do you see happening in this case? I’ve got spaces turned on and
> everything I do just seems to create a new fullscreen space (except
> when it crashes my session).
> 

I see same result as in the first case: Desktop, empty blank space
with Emacs menu, space with Emacs frame and no menu/titlebar.

Still can't make this case work. Probably window animation stand in the way:

nsterm.m  : 1869: [  376]  ns_set_parent_frame
nsterm.m  : 1912: [  377]  | child frame must not be in fullscreen
2019-07-17 21:20:49.027 Emacs[91925:3597098] not in fullscreen state

If ns_set_parent_frame is called while window is still in process of going
full screen then maybe it must be deferred until animation ends.

[-- Attachment #2: ns-emacs-spaces-wip.patch --]
[-- Type: application/octet-stream, Size: 2923 bytes --]

diff --git a/src/nsterm.h b/src/nsterm.h
index 9773eb3e66..d16588718e 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -1274,6 +1274,7 @@ extern char gnustep_base_version[];  /* version tracking */
 #if !defined (NS_IMPL_COCOA) || !defined (MAC_OS_X_VERSION_10_7)
 #define NSFullScreenWindowMask                      (1 << 14)
 #define NSWindowCollectionBehaviorFullScreenPrimary (1 << 7)
+#define NSWindowCollectionBehaviorFullScreenAuxiliary (1 << 8)
 #define NSApplicationPresentationFullScreen         (1 << 10)
 #define NSApplicationPresentationAutoHideToolbar    (1 << 11)
 #define NSAppKitVersionNumber10_7                   1138
diff --git a/src/nsterm.m b/src/nsterm.m
index 02331826d9..8725520d4a 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1884,6 +1884,8 @@ so some key presses (TAB) are swallowed by the system.  */
 
       if ([child parentWindow] != nil)
         {
+          parent = [child parentWindow];
+
           [[child parentWindow] removeChildWindow:child];
 #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 101000
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 101000
@@ -1891,10 +1893,28 @@ so some key presses (TAB) are swallowed by the system.  */
 #endif
               [child setAccessibilitySubrole:NSAccessibilityStandardWindowSubrole];
 #endif
+          if (NILP (new_value)){
+            NSTRACE ("child setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary");
+            [child setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary];
+            // TODO check parents of parent
+            if (([parent styleMask] & NSWindowStyleMaskFullScreen) != 0){
+              // if current parent in fullscreen and not new parent make child fullscreen
+              NSTRACE ("make child fullscreen");
+              [child toggleFullScreen:child];
+            }
+          }
         }
 
       if (!NILP (new_value))
         {
+          // child frame must not be in fullscreen
+          if (([child styleMask] & NSWindowStyleMaskFullScreen) != 0){
+            NSTRACE ("child frame must not be in fullscreen");
+            [child toggleFullScreen:child];
+          }
+          NSTRACE ("child setCollectionBehavior:NSWindowCollectionBehaviorFullScreenAuxiliary");
+          [child setCollectionBehavior:NSWindowCollectionBehaviorFullScreenAuxiliary];
+
           parent = [FRAME_NS_VIEW (p) window];
 
           [parent addChildWindow: child
@@ -7365,7 +7385,10 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
   if (NSAppKitVersionNumber >= NSAppKitVersionNumber10_7)
 #endif
-    [win setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary];
+    if (FRAME_PARENT_FRAME (f))
+      [win setCollectionBehavior:NSWindowCollectionBehaviorFullScreenAuxiliary];
+    else
+      [win setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary];
 #endif
 
   wr = [win frame];

bug#36672: 27.0.50; NS build: Creating child frame leads to empty space

[Andrii Kolomoiets]

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


> On Jul 16, 2019, at 22:28, Alan Third <alan@idiocy.org> wrote:
> 
> The patch looks good to me, but lets see if we can find solutions to
> these other issues.

I made some progress on this issue.

The idea is to wait for fullscreen transition to complete:
- Set property isFullScreenTransition on windowWillEnterFullScreen and
  windowWillExitFullScreen events;
- Clear that property on windowDidEnterFullScreen and
  windowDidExitFullScreen events;
- Wait for property to be cleared when we need to.

Removing parent-frame property causes child frame to enter fullscreen
if some of parent frames is in fullscreen.

Setting parent-frame property to fullscreen window causes window to
leave fullscreen.

As a side effect (sleep-for 0.5) in toggle-frame-fullscreen is no
longer needed (bug#28496).

But I need help with some questions:

1. Is it the rigth way to wait for some NS event by calling
   wait_reading_process_output or is there a better way?

2. I commented some code in windowDidMove with FIXME comment.
   After windowDidMove event is triggered, windowDidExitFullScreen is
   not received and fullscreen transition is never "completed".
   With that code commented windowDidExitFullScreen is received.
   Is this code somehow breaks the events queue?

Actually it is the first time I'm working with the C side of Emacs.
And with AppKit. And with C :) Please let me know if my decisions
and conclusions are wrong.

Thanks!

[-- Attachment #2: ns-emacs-spaces.patch --]
[-- Type: application/octet-stream, Size: 9341 bytes --]

diff --git a/lisp/frame.el b/lisp/frame.el
index fd7e872fb6..325b4c07c5 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2629,11 +2629,7 @@ toggle-frame-fullscreen
 	      (set-frame-parameter frame 'fullscreen fullscreen-restore)
 	    (set-frame-parameter frame 'fullscreen nil)))
       (modify-frame-parameters
-       frame `((fullscreen . fullboth) (fullscreen-restore . ,fullscreen))))
-    ;; Manipulating a frame without waiting for the fullscreen
-    ;; animation to complete can cause a crash, or other unexpected
-    ;; behavior, on macOS (bug#28496).
-    (when (featurep 'cocoa) (sleep-for 0.5))))
+       frame `((fullscreen . fullboth) (fullscreen-restore . ,fullscreen))))))
 
 \f
 ;;;; Key bindings
diff --git a/src/nsterm.h b/src/nsterm.h
index 9773eb3e66..9391e31b48 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -417,6 +417,7 @@ #define NSTRACE_UNSILENCE()
    int maximized_width, maximized_height;
    NSWindow *nonfs_window;
    BOOL fs_is_native;
+   BOOL is_fullscreen_transition;
 @public
    struct frame *emacsframe;
    int rows, cols;
@@ -448,6 +449,8 @@ #define NSTRACE_UNSILENCE()
 - (void) toggleFullScreen: (id) sender;
 - (BOOL) fsIsNative;
 - (BOOL) isFullscreen;
+- (BOOL) isFullScreenTransition;
+- (void) waitFullScreenTransition;
 #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
 - (void) updateCollectionBehavior;
 #endif
@@ -1274,6 +1277,7 @@ #define SCREENMAXBOUND(x) (IN_BOUND (-SCREENMAX, x, SCREENMAX))
 #if !defined (NS_IMPL_COCOA) || !defined (MAC_OS_X_VERSION_10_7)
 #define NSFullScreenWindowMask                      (1 << 14)
 #define NSWindowCollectionBehaviorFullScreenPrimary (1 << 7)
+#define NSWindowCollectionBehaviorFullScreenAuxiliary (1 << 8)
 #define NSApplicationPresentationFullScreen         (1 << 10)
 #define NSApplicationPresentationAutoHideToolbar    (1 << 11)
 #define NSAppKitVersionNumber10_7                   1138
diff --git a/src/nsterm.m b/src/nsterm.m
index 02331826d9..df6a0aab80 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1495,9 +1495,11 @@ -(void)remove
 
       /* Making a new frame from a fullscreen frame will make the new frame
          fullscreen also.  So skip handleFS as this will print an error.  */
-      if ([view fsIsNative] && f->want_fullscreen == FULLSCREEN_BOTH
-          && [view isFullscreen])
+      if ([view fsIsNative] && [view isFullscreen]) {
+        // maybe it is not necessary to wait
+        [view waitFullScreenTransition];
         return;
+      }
 
       if (f->want_fullscreen != FULLSCREEN_NONE)
         {
@@ -1882,19 +1884,52 @@ so some key presses (TAB) are swallowed by the system.  */
       block_input ();
       child = [FRAME_NS_VIEW (f) window];
 
+#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+      EmacsView *view = (EmacsView *)FRAME_NS_VIEW (f);
+#endif
+
       if ([child parentWindow] != nil)
         {
+#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+          parent = [child parentWindow];
+#endif
+
           [[child parentWindow] removeChildWindow:child];
 #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 101000
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 101000
           if ([child respondsToSelector:@selector(setAccessibilitySubrole:)])
 #endif
               [child setAccessibilitySubrole:NSAccessibilityStandardWindowSubrole];
+#endif
+#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+          if (NILP (new_value)) {
+            NSTRACE ("child setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary");
+            [child setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary];
+            // if current parent in fullscreen and no new parent make child fullscreen
+            while (parent) {
+              if (([parent styleMask] & NSWindowStyleMaskFullScreen) != 0){
+                [view toggleFullScreen:child];
+                break;
+              }
+              // check all parents
+              parent = [parent parentWindow];
+            }
+          }
 #endif
         }
 
       if (!NILP (new_value))
         {
+#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+          // child frame must not be in fullscreen
+          if ([view fsIsNative] && [view isFullscreen]){
+            // in case child is going fullscreen
+            [view waitFullScreenTransition];
+            [view toggleFullScreen:child];
+          }
+          NSTRACE ("child setCollectionBehavior:NSWindowCollectionBehaviorFullScreenAuxiliary");
+          [child setCollectionBehavior:NSWindowCollectionBehaviorFullScreenAuxiliary];
+#endif
           parent = [FRAME_NS_VIEW (p) window];
 
           [parent addChildWindow: child
@@ -7336,6 +7371,7 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
 #endif
     fs_is_native = ns_use_native_fullscreen;
 #endif
+  is_fullscreen_transition = NO;
 
   maximized_width = maximized_height = -1;
   nonfs_window = nil;
@@ -7365,7 +7401,10 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
   if (NSAppKitVersionNumber >= NSAppKitVersionNumber10_7)
 #endif
-    [win setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary];
+    if (FRAME_PARENT_FRAME (f))
+      [win setCollectionBehavior:NSWindowCollectionBehaviorFullScreenAuxiliary];
+    else
+      [win setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary];
 #endif
 
   wr = [win frame];
@@ -7496,11 +7535,12 @@ - (void)windowDidMove: sender
       emacsframe->top_pos =
         NS_PARENT_WINDOW_TOP_POS (emacsframe) - (r.origin.y + r.size.height);
 
-      if (emacs_event)
-        {
-          emacs_event->kind = MOVE_FRAME_EVENT;
-          EV_TRAILER ((id)nil);
-        }
+      // FIXME: after event part below didExitFullScreen is not received
+      // if (emacs_event)
+      //   {
+      //     emacs_event->kind = MOVE_FRAME_EVENT;
+      //     EV_TRAILER ((id)nil);
+      //   }
     }
 }
 
@@ -7700,6 +7740,7 @@ - (NSApplicationPresentationOptions)window:(NSWindow *)window
 - (void)windowWillEnterFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowWillEnterFullScreen:]");
+  is_fullscreen_transition = YES;
   [self windowWillEnterFullScreen];
 }
 - (void)windowWillEnterFullScreen /* provided for direct calls */
@@ -7712,6 +7753,7 @@ - (void)windowDidEnterFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowDidEnterFullScreen:]");
   [self windowDidEnterFullScreen];
+  is_fullscreen_transition = NO;
 }
 
 - (void)windowDidEnterFullScreen /* provided for direct calls */
@@ -7750,6 +7792,7 @@ - (void)windowDidEnterFullScreen /* provided for direct calls */
 - (void)windowWillExitFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowWillExitFullScreen:]");
+  is_fullscreen_transition = YES;
   [self windowWillExitFullScreen];
 }
 
@@ -7769,6 +7812,7 @@ - (void)windowDidExitFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowDidExitFullScreen:]");
   [self windowDidExitFullScreen];
+  is_fullscreen_transition = NO;
 }
 
 - (void)windowDidExitFullScreen /* provided for direct calls */
@@ -7798,6 +7842,21 @@ - (void)windowDidExitFullScreen /* provided for direct calls */
     [[self window] performZoom:self];
 }
 
+- (BOOL)isFullScreenTransition
+{
+  return is_fullscreen_transition;
+}
+
+- (void)waitFullScreenTransition
+{
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+  while ([self isFullScreenTransition]){
+    NSTRACE ("wait for fullscreen");
+    wait_reading_process_output (0, 300000000, 0, 1, Qnil, NULL, 0);
+  }
+#endif
+}
+
 - (BOOL)fsIsNative
 {
   return fs_is_native;
@@ -7835,10 +7894,17 @@ - (void)updateCollectionBehavior
     {
       NSWindow *win = [self window];
       NSWindowCollectionBehavior b = [win collectionBehavior];
-      if (ns_use_native_fullscreen)
-        b |= NSWindowCollectionBehaviorFullScreenPrimary;
-      else
+      if (ns_use_native_fullscreen) {
+          if ([win parentWindow]) {
+            b &= ~NSWindowCollectionBehaviorFullScreenPrimary;
+            b |= NSWindowCollectionBehaviorFullScreenAuxiliary;
+          } else {
+            b |= NSWindowCollectionBehaviorFullScreenPrimary;
+            b &= ~NSWindowCollectionBehaviorFullScreenAuxiliary;
+          }
+      } else {
         b &= ~NSWindowCollectionBehaviorFullScreenPrimary;
+      }
 
       [win setCollectionBehavior: b];
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
@@ -7863,9 +7929,14 @@ - (void)toggleFullScreen: (id)sender
     {
 #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
-      if ([[self window] respondsToSelector: @selector(toggleFullScreen:)])
+      if ([[self window] respondsToSelector: @selector(toggleFullScreen:)]){
 #endif
         [[self window] toggleFullScreen:sender];
+        // wait for fullscreen animation complete (bug#28496)
+        [self waitFullScreenTransition];
+#endif
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+      }
 #endif
       return;
     }

bug#36672: 27.0.50; NS build: Creating child frame leads to empty space

[Andrii Kolomoiets]

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

Andrii Kolomoiets <andreyk.mad@gmail.com> writes:

>> On Jul 16, 2019, at 22:28, Alan Third <alan@idiocy.org> wrote:
>> 
>> The patch looks good to me, but lets see if we can find solutions to
>> these other issues.
>
> I made some progress on this issue.
>
> The idea is to wait for fullscreen transition to complete:
> - Set property isFullScreenTransition on windowWillEnterFullScreen and
>   windowWillExitFullScreen events;
> - Clear that property on windowDidEnterFullScreen and
>   windowDidExitFullScreen events;
> - Wait for property to be cleared when we need to.

Updated patch to work with latest master


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ns-emacs-spaces-updated.patch --]
[-- Type: text/x-patch, Size: 9098 bytes --]

diff --git a/lisp/frame.el b/lisp/frame.el
index 16ee7580f8..7ed3e23f3c 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2676,11 +2676,7 @@ toggle-frame-fullscreen
 	      (set-frame-parameter frame 'fullscreen fullscreen-restore)
 	    (set-frame-parameter frame 'fullscreen nil)))
       (modify-frame-parameters
-       frame `((fullscreen . fullboth) (fullscreen-restore . ,fullscreen))))
-    ;; Manipulating a frame without waiting for the fullscreen
-    ;; animation to complete can cause a crash, or other unexpected
-    ;; behavior, on macOS (bug#28496).
-    (when (featurep 'cocoa) (sleep-for 0.5))))
+       frame `((fullscreen . fullboth) (fullscreen-restore . ,fullscreen))))))
 
 \f
 ;;;; Key bindings
diff --git a/src/nsterm.h b/src/nsterm.h
index 7c6197f128..243d66be60 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -417,6 +417,7 @@ #define NSTRACE_UNSILENCE()
    int maximized_width, maximized_height;
    NSWindow *nonfs_window;
    BOOL fs_is_native;
+   BOOL is_fullscreen_transition;
 #ifdef NS_IMPL_COCOA
    CGContextRef drawingBuffer;
 #endif
@@ -451,6 +452,8 @@ #define NSTRACE_UNSILENCE()
 - (void) toggleFullScreen: (id) sender;
 - (BOOL) fsIsNative;
 - (BOOL) isFullscreen;
+- (BOOL) isFullScreenTransition;
+- (void) waitFullScreenTransition;
 #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
 - (void) updateCollectionBehavior;
 #endif
@@ -1270,6 +1273,7 @@ #define SCREENMAXBOUND(x) (IN_BOUND (-SCREENMAX, x, SCREENMAX))
 #if !defined (NS_IMPL_COCOA) || !defined (MAC_OS_X_VERSION_10_7)
 #define NSFullScreenWindowMask                      (1 << 14)
 #define NSWindowCollectionBehaviorFullScreenPrimary (1 << 7)
+#define NSWindowCollectionBehaviorFullScreenAuxiliary (1 << 8)
 #define NSApplicationPresentationFullScreen         (1 << 10)
 #define NSApplicationPresentationAutoHideToolbar    (1 << 11)
 #define NSAppKitVersionNumber10_7                   1138
diff --git a/src/nsterm.m b/src/nsterm.m
index 2cf6774a1f..b290fd3d95 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1588,9 +1588,11 @@ -(void)remove
 
       /* Making a new frame from a fullscreen frame will make the new frame
          fullscreen also.  So skip handleFS as this will print an error.  */
-      if ([view fsIsNative] && f->want_fullscreen == FULLSCREEN_BOTH
-          && [view isFullscreen])
+      if ([view fsIsNative] && [view isFullscreen]) {
+        // maybe it is not necessary to wait
+        [view waitFullScreenTransition];
         return;
+      }
 
       if (f->want_fullscreen != FULLSCREEN_NONE)
         {
@@ -1976,19 +1978,52 @@ so some key presses (TAB) are swallowed by the system.  */
       block_input ();
       child = [FRAME_NS_VIEW (f) window];
 
+#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+      EmacsView *view = (EmacsView *)FRAME_NS_VIEW (f);
+#endif
+
       if ([child parentWindow] != nil)
         {
+#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+          parent = [child parentWindow];
+#endif
+
           [[child parentWindow] removeChildWindow:child];
 #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 101000
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 101000
           if ([child respondsToSelector:@selector(setAccessibilitySubrole:)])
 #endif
               [child setAccessibilitySubrole:NSAccessibilityStandardWindowSubrole];
+#endif
+#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+          if (NILP (new_value)) {
+            NSTRACE ("child setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary");
+            [child setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary];
+            // if current parent in fullscreen and no new parent make child fullscreen
+            while (parent) {
+              if (([parent styleMask] & NSWindowStyleMaskFullScreen) != 0){
+                [view toggleFullScreen:child];
+                break;
+              }
+              // check all parents
+              parent = [parent parentWindow];
+            }
+          }
 #endif
         }
 
       if (!NILP (new_value))
         {
+#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+          // child frame must not be in fullscreen
+          if ([view fsIsNative] && [view isFullscreen]){
+            // in case child is going fullscreen
+            [view waitFullScreenTransition];
+            [view toggleFullScreen:child];
+          }
+          NSTRACE ("child setCollectionBehavior:NSWindowCollectionBehaviorFullScreenAuxiliary");
+          [child setCollectionBehavior:NSWindowCollectionBehaviorFullScreenAuxiliary];
+#endif
           parent = [FRAME_NS_VIEW (p) window];
 
           [parent addChildWindow: child
@@ -7411,6 +7446,7 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
 #endif
     fs_is_native = ns_use_native_fullscreen;
 #endif
+  is_fullscreen_transition = NO;
 
   maximized_width = maximized_height = -1;
   nonfs_window = nil;
@@ -7442,7 +7478,10 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
   if (NSAppKitVersionNumber >= NSAppKitVersionNumber10_7)
 #endif
-    [win setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary];
+    if (FRAME_PARENT_FRAME (f))
+      [win setCollectionBehavior:NSWindowCollectionBehaviorFullScreenAuxiliary];
+    else
+      [win setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary];
 #endif
 
   wr = [win frame];
@@ -7565,11 +7604,12 @@ - (void)windowDidMove: sender
       emacsframe->top_pos =
         NS_PARENT_WINDOW_TOP_POS (emacsframe) - (r.origin.y + r.size.height);
 
-      if (emacs_event)
-        {
-          emacs_event->kind = MOVE_FRAME_EVENT;
-          EV_TRAILER ((id)nil);
-        }
+      // FIXME: after event part below didExitFullScreen is not received
+      // if (emacs_event)
+      //   {
+      //     emacs_event->kind = MOVE_FRAME_EVENT;
+      //     EV_TRAILER ((id)nil);
+      //   }
     }
 }
 
@@ -7769,6 +7809,7 @@ - (NSApplicationPresentationOptions)window:(NSWindow *)window
 - (void)windowWillEnterFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowWillEnterFullScreen:]");
+  is_fullscreen_transition = YES;
   [self windowWillEnterFullScreen];
 }
 - (void)windowWillEnterFullScreen /* provided for direct calls */
@@ -7781,6 +7822,7 @@ - (void)windowDidEnterFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowDidEnterFullScreen:]");
   [self windowDidEnterFullScreen];
+  is_fullscreen_transition = NO;
 }
 
 - (void)windowDidEnterFullScreen /* provided for direct calls */
@@ -7819,6 +7861,7 @@ - (void)windowDidEnterFullScreen /* provided for direct calls */
 - (void)windowWillExitFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowWillExitFullScreen:]");
+  is_fullscreen_transition = YES;
   [self windowWillExitFullScreen];
 }
 
@@ -7838,6 +7881,7 @@ - (void)windowDidExitFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowDidExitFullScreen:]");
   [self windowDidExitFullScreen];
+  is_fullscreen_transition = NO;
 }
 
 - (void)windowDidExitFullScreen /* provided for direct calls */
@@ -7867,6 +7911,21 @@ - (void)windowDidExitFullScreen /* provided for direct calls */
     [[self window] performZoom:self];
 }
 
+- (BOOL)isFullScreenTransition
+{
+  return is_fullscreen_transition;
+}
+
+- (void)waitFullScreenTransition
+{
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+  while ([self isFullScreenTransition]){
+    NSTRACE ("wait for fullscreen");
+    wait_reading_process_output (0, 300000000, 0, 1, Qnil, NULL, 0);
+  }
+#endif
+}
+
 - (BOOL)fsIsNative
 {
   return fs_is_native;
@@ -7904,10 +7963,17 @@ - (void)updateCollectionBehavior
     {
       NSWindow *win = [self window];
       NSWindowCollectionBehavior b = [win collectionBehavior];
-      if (ns_use_native_fullscreen)
-        b |= NSWindowCollectionBehaviorFullScreenPrimary;
-      else
+      if (ns_use_native_fullscreen) {
+          if ([win parentWindow]) {
+            b &= ~NSWindowCollectionBehaviorFullScreenPrimary;
+            b |= NSWindowCollectionBehaviorFullScreenAuxiliary;
+          } else {
+            b |= NSWindowCollectionBehaviorFullScreenPrimary;
+            b &= ~NSWindowCollectionBehaviorFullScreenAuxiliary;
+          }
+      } else {
         b &= ~NSWindowCollectionBehaviorFullScreenPrimary;
+      }
 
       [win setCollectionBehavior: b];
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
@@ -7932,9 +7998,14 @@ - (void)toggleFullScreen: (id)sender
     {
 #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
-      if ([[self window] respondsToSelector: @selector(toggleFullScreen:)])
+      if ([[self window] respondsToSelector: @selector(toggleFullScreen:)]){
 #endif
         [[self window] toggleFullScreen:sender];
+        // wait for fullscreen animation complete (bug#28496)
+        [self waitFullScreenTransition];
+#endif
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+      }
 #endif
       return;
     }

bug#36672: 27.0.50; NS build: Creating child frame leads to empty space

[Alan Third]

On Fri, Feb 14, 2020 at 10:23:22AM +0200, Andrii Kolomoiets wrote:
> Andrii Kolomoiets <andreyk.mad@gmail.com> writes:
> 
> >> On Jul 16, 2019, at 22:28, Alan Third <alan@idiocy.org> wrote:
> >> 
> >> The patch looks good to me, but lets see if we can find solutions to
> >> these other issues.
> >
> > I made some progress on this issue.
> >
> > The idea is to wait for fullscreen transition to complete:
> > - Set property isFullScreenTransition on windowWillEnterFullScreen and
> >   windowWillExitFullScreen events;
> > - Clear that property on windowDidEnterFullScreen and
> >   windowDidExitFullScreen events;
> > - Wait for property to be cleared when we need to.
> 
> Updated patch to work with latest master

Sorry for the delay, I’ll try and get a look this weekend.
-- 
Alan Third

bug#36672: 27.0.50; NS build: Creating child frame leads to empty space

[Alan Third]

On Fri, Feb 14, 2020 at 10:23:22AM +0200, Andrii Kolomoiets wrote:
> Andrii Kolomoiets <andreyk.mad@gmail.com> writes:
> 
> >> On Jul 16, 2019, at 22:28, Alan Third <alan@idiocy.org> wrote:
> >> 
> >> The patch looks good to me, but lets see if we can find solutions to
> >> these other issues.
> >
> > I made some progress on this issue.
> >
> > The idea is to wait for fullscreen transition to complete:
> > - Set property isFullScreenTransition on windowWillEnterFullScreen and
> >   windowWillExitFullScreen events;
> > - Clear that property on windowDidEnterFullScreen and
> >   windowDidExitFullScreen events;
> > - Wait for property to be cleared when we need to.
> 
> Updated patch to work with latest master

Thanks! This looks much better than what we had before. No crashes for
one!

A few nitpicks:

> +   BOOL is_fullscreen_transition;

I may be misunderstanding this, but would ‘in_fullscreen_transition’
be a better name?

> +#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
> +          // child frame must not be in fullscreen
> +          if ([view fsIsNative] && [view isFullscreen]){

Opening braces go on a new line.

> +- (void)waitFullScreenTransition
> +{
> +#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
> +  while ([self isFullScreenTransition]){

Opening brace, again.

>  - (BOOL)fsIsNative
>  {
>    return fs_is_native;
> @@ -7904,10 +7963,17 @@ - (void)updateCollectionBehavior
>      {
>        NSWindow *win = [self window];
>        NSWindowCollectionBehavior b = [win collectionBehavior];
> -      if (ns_use_native_fullscreen)
> -        b |= NSWindowCollectionBehaviorFullScreenPrimary;
> -      else
> +      if (ns_use_native_fullscreen) {
> +          if ([win parentWindow]) {
> +            b &= ~NSWindowCollectionBehaviorFullScreenPrimary;
> +            b |= NSWindowCollectionBehaviorFullScreenAuxiliary;
> +          } else {
> +            b |= NSWindowCollectionBehaviorFullScreenPrimary;
> +            b &= ~NSWindowCollectionBehaviorFullScreenAuxiliary;
> +          }
> +      } else {
>          b &= ~NSWindowCollectionBehaviorFullScreenPrimary;
> +      }

And again. That ‘} else {’ should also be over three lines.

>  #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
> @@ -7932,9 +7998,14 @@ - (void)toggleFullScreen: (id)sender
>      {
>  #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
>  #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
> -      if ([[self window] respondsToSelector: @selector(toggleFullScreen:)])
> +      if ([[self window] respondsToSelector: @selector(toggleFullScreen:)]){
>  #endif
>          [[self window] toggleFullScreen:sender];
> +        // wait for fullscreen animation complete (bug#28496)
> +        [self waitFullScreenTransition];
> +#endif
> +#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
> +      }
>  #endif
>        return;
>      }

Should that final #if #endif not be within the outer #if #endif? And
the opening brace again.
-- 
Alan Third

bug#36672: 27.0.50; NS build: Creating child frame leads to empty space

[Andrii Kolomoiets]

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

Alan Third <alan@idiocy.org> writes:

> Thanks! This looks much better than what we had before. No crashes for
> one!
>
> A few nitpicks:
>
>> +   BOOL is_fullscreen_transition;
>
> I may be misunderstanding this, but would ‘in_fullscreen_transition’
> be a better name?

OK, renamed. 

> Opening braces go on a new line.

Braces style is fixed to match Emacs code style.

> And again. That ‘} else {’ should also be over three lines.
>
>>  #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
>> @@ -7932,9 +7998,14 @@ - (void)toggleFullScreen: (id)sender
>>      {
>>  #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
>>  #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
>> -      if ([[self window] respondsToSelector: @selector(toggleFullScreen:)])
>> +      if ([[self window] respondsToSelector: @selector(toggleFullScreen:)]){
>>  #endif
>>          [[self window] toggleFullScreen:sender];
>> +        // wait for fullscreen animation complete (bug#28496)
>> +        [self waitFullScreenTransition];
>> +#endif
>> +#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
>> +      }
>>  #endif
>>        return;
>>      }
>
> Should that final #if #endif not be within the outer #if #endif?

Yes. Fixed.

Thanks!

Please see attached patch. Log message is also provided.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-NS-child-frame-in-native-fullscreen.patch --]
[-- Type: text/x-patch, Size: 10837 bytes --]

From a9705617188c9a5ef77a8f2d5cc796b86dda9fa6 Mon Sep 17 00:00:00 2001
From: Andrii Kolomoiets <andreyk.mad@gmail.com>
Date: Tue, 10 Mar 2020 10:14:59 +0200
Subject: [PATCH] NS child frame in native fullscreen

* lisp/frame.el (toggle-frame-fullscreen): Don't sleep on cocoa.
Fullscreen animation waiting is moved to src/nsterm.m.
* src/nsterm.h (EmacsView): Add in_fullscreen_transition,
inFullScreenTransition, waitFullScreenTransition.
(NSWindowCollectionBehaviorFullScreenAuxiliary): New define.
* src/nsterm.m (ns_make_frame_visible): Wait for fullscreen animation.
(ns_set_parent_frame): Set frame collection behavior; make child frames
non-fullscreen; make non-child frames fullscreen if parent was fullscreen.
([EmacsView initFrameFromEmacs]): Set in_fullscreen_transition; set frame
collection behavior according to parent frame.
([EmacsView windowDidMove]): Remove code by commenting with "fixme".
([EmacsView windowWillEnterFullScreen], [EmacsView windowDidEnterFullScreen])
([EmacsView windowWillExitFullScreen], [EmacsView windowDidExitFullScreen]):
Set in_fullscreen_transition.
([EmacsView inFullScreenTransition], [EmacsView waitFullScreenTransition]):
New methods.
([EmacsView updateCollectionBehavior]): Set collection behavior according to
parent frame.
([EmacsView toggleFullScreen]): Wait for fullscreen animation.
---
 lisp/frame.el |   6 +---
 src/nsterm.h  |   4 +++
 src/nsterm.m  | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 100 insertions(+), 17 deletions(-)

diff --git a/lisp/frame.el b/lisp/frame.el
index 16ee7580f8..7ed3e23f3c 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2676,11 +2676,7 @@ toggle-frame-fullscreen
 	      (set-frame-parameter frame 'fullscreen fullscreen-restore)
 	    (set-frame-parameter frame 'fullscreen nil)))
       (modify-frame-parameters
-       frame `((fullscreen . fullboth) (fullscreen-restore . ,fullscreen))))
-    ;; Manipulating a frame without waiting for the fullscreen
-    ;; animation to complete can cause a crash, or other unexpected
-    ;; behavior, on macOS (bug#28496).
-    (when (featurep 'cocoa) (sleep-for 0.5))))
+       frame `((fullscreen . fullboth) (fullscreen-restore . ,fullscreen))))))
 
 \f
 ;;;; Key bindings
diff --git a/src/nsterm.h b/src/nsterm.h
index db966e1581..8396a542f7 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -433,6 +433,7 @@ #define NS_DRAW_TO_BUFFER 1
    int maximized_width, maximized_height;
    NSWindow *nonfs_window;
    BOOL fs_is_native;
+   BOOL in_fullscreen_transition;
 #ifdef NS_DRAW_TO_BUFFER
    CGContextRef drawingBuffer;
 #endif
@@ -467,6 +468,8 @@ #define NS_DRAW_TO_BUFFER 1
 - (void) toggleFullScreen: (id) sender;
 - (BOOL) fsIsNative;
 - (BOOL) isFullscreen;
+- (BOOL) inFullScreenTransition;
+- (void) waitFullScreenTransition;
 #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
 - (void) updateCollectionBehavior;
 #endif
@@ -1286,6 +1289,7 @@ #define SCREENMAXBOUND(x) (IN_BOUND (-SCREENMAX, x, SCREENMAX))
 #if !defined (NS_IMPL_COCOA) || !defined (MAC_OS_X_VERSION_10_7)
 #define NSFullScreenWindowMask                      (1 << 14)
 #define NSWindowCollectionBehaviorFullScreenPrimary (1 << 7)
+#define NSWindowCollectionBehaviorFullScreenAuxiliary (1 << 8)
 #define NSApplicationPresentationFullScreen         (1 << 10)
 #define NSApplicationPresentationAutoHideToolbar    (1 << 11)
 #define NSAppKitVersionNumber10_7                   1138
diff --git a/src/nsterm.m b/src/nsterm.m
index 851a5617d7..96a7fdc018 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1571,9 +1571,12 @@ -(void)remove
 
       /* Making a new frame from a fullscreen frame will make the new frame
          fullscreen also.  So skip handleFS as this will print an error.  */
-      if ([view fsIsNative] && f->want_fullscreen == FULLSCREEN_BOTH
-          && [view isFullscreen])
-        return;
+      if ([view fsIsNative] && [view isFullscreen])
+        {
+          // maybe it is not necessary to wait
+          [view waitFullScreenTransition];
+          return;
+        }
 
       if (f->want_fullscreen != FULLSCREEN_NONE)
         {
@@ -1959,19 +1962,55 @@ so some key presses (TAB) are swallowed by the system.  */
       block_input ();
       child = [FRAME_NS_VIEW (f) window];
 
+#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+      EmacsView *view = (EmacsView *)FRAME_NS_VIEW (f);
+#endif
+
       if ([child parentWindow] != nil)
         {
+#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+          parent = [child parentWindow];
+#endif
+
           [[child parentWindow] removeChildWindow:child];
 #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 101000
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 101000
           if ([child respondsToSelector:@selector(setAccessibilitySubrole:)])
 #endif
               [child setAccessibilitySubrole:NSAccessibilityStandardWindowSubrole];
+#endif
+#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+          if (NILP (new_value))
+            {
+              NSTRACE ("child setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary");
+              [child setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary];
+              // if current parent in fullscreen and no new parent make child fullscreen
+              while (parent) {
+                if (([parent styleMask] & NSWindowStyleMaskFullScreen) != 0)
+                  {
+                    [view toggleFullScreen:child];
+                    break;
+                  }
+                // check all parents
+                parent = [parent parentWindow];
+              }
+            }
 #endif
         }
 
       if (!NILP (new_value))
         {
+#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+          // child frame must not be in fullscreen
+          if ([view fsIsNative] && [view isFullscreen])
+            {
+              // in case child is going fullscreen
+              [view waitFullScreenTransition];
+              [view toggleFullScreen:child];
+            }
+          NSTRACE ("child setCollectionBehavior:NSWindowCollectionBehaviorFullScreenAuxiliary");
+          [child setCollectionBehavior:NSWindowCollectionBehaviorFullScreenAuxiliary];
+#endif
           parent = [FRAME_NS_VIEW (p) window];
 
           [parent addChildWindow: child
@@ -7398,6 +7437,7 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
 #endif
     fs_is_native = ns_use_native_fullscreen;
 #endif
+  in_fullscreen_transition = NO;
 
   maximized_width = maximized_height = -1;
   nonfs_window = nil;
@@ -7431,7 +7471,10 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
   if (NSAppKitVersionNumber >= NSAppKitVersionNumber10_7)
 #endif
-    [win setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary];
+    if (FRAME_PARENT_FRAME (f))
+      [win setCollectionBehavior:NSWindowCollectionBehaviorFullScreenAuxiliary];
+    else
+      [win setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary];
 #endif
 
   wr = [win frame];
@@ -7554,11 +7597,12 @@ - (void)windowDidMove: sender
       emacsframe->top_pos =
         NS_PARENT_WINDOW_TOP_POS (emacsframe) - (r.origin.y + r.size.height);
 
-      if (emacs_event)
-        {
-          emacs_event->kind = MOVE_FRAME_EVENT;
-          EV_TRAILER ((id)nil);
-        }
+      // FIXME: after event part below didExitFullScreen is not received
+      // if (emacs_event)
+      //   {
+      //     emacs_event->kind = MOVE_FRAME_EVENT;
+      //     EV_TRAILER ((id)nil);
+      //   }
     }
 }
 
@@ -7758,6 +7802,7 @@ - (NSApplicationPresentationOptions)window:(NSWindow *)window
 - (void)windowWillEnterFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowWillEnterFullScreen:]");
+  in_fullscreen_transition = YES;
   [self windowWillEnterFullScreen];
 }
 - (void)windowWillEnterFullScreen /* provided for direct calls */
@@ -7770,6 +7815,7 @@ - (void)windowDidEnterFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowDidEnterFullScreen:]");
   [self windowDidEnterFullScreen];
+  in_fullscreen_transition = NO;
 }
 
 - (void)windowDidEnterFullScreen /* provided for direct calls */
@@ -7808,6 +7854,7 @@ - (void)windowDidEnterFullScreen /* provided for direct calls */
 - (void)windowWillExitFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowWillExitFullScreen:]");
+  in_fullscreen_transition = YES;
   [self windowWillExitFullScreen];
 }
 
@@ -7827,6 +7874,7 @@ - (void)windowDidExitFullScreen:(NSNotification *)notification
 {
   NSTRACE ("[EmacsView windowDidExitFullScreen:]");
   [self windowDidExitFullScreen];
+  in_fullscreen_transition = NO;
 }
 
 - (void)windowDidExitFullScreen /* provided for direct calls */
@@ -7856,6 +7904,22 @@ - (void)windowDidExitFullScreen /* provided for direct calls */
     [[self window] performZoom:self];
 }
 
+- (BOOL)inFullScreenTransition
+{
+  return in_fullscreen_transition;
+}
+
+- (void)waitFullScreenTransition
+{
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+  while ([self inFullScreenTransition])
+    {
+      NSTRACE ("wait for fullscreen");
+      wait_reading_process_output (0, 300000000, 0, 1, Qnil, NULL, 0);
+    }
+#endif
+}
+
 - (BOOL)fsIsNative
 {
   return fs_is_native;
@@ -7894,9 +7958,22 @@ - (void)updateCollectionBehavior
       NSWindow *win = [self window];
       NSWindowCollectionBehavior b = [win collectionBehavior];
       if (ns_use_native_fullscreen)
-        b |= NSWindowCollectionBehaviorFullScreenPrimary;
+        {
+          if ([win parentWindow])
+            {
+              b &= ~NSWindowCollectionBehaviorFullScreenPrimary;
+              b |= NSWindowCollectionBehaviorFullScreenAuxiliary;
+            }
+          else
+            {
+              b |= NSWindowCollectionBehaviorFullScreenPrimary;
+              b &= ~NSWindowCollectionBehaviorFullScreenAuxiliary;
+            }
+        }
       else
-        b &= ~NSWindowCollectionBehaviorFullScreenPrimary;
+        {
+          b &= ~NSWindowCollectionBehaviorFullScreenPrimary;
+        }
 
       [win setCollectionBehavior: b];
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
@@ -7922,8 +7999,14 @@ - (void)toggleFullScreen: (id)sender
 #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
       if ([[self window] respondsToSelector: @selector(toggleFullScreen:)])
+        {
+#endif
+          [[self window] toggleFullScreen:sender];
+          // wait for fullscreen animation complete (bug#28496)
+          [self waitFullScreenTransition];
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+        }
 #endif
-        [[self window] toggleFullScreen:sender];
 #endif
       return;
     }
-- 
2.15.1

bug#36672: 27.0.50; NS build: Creating child frame leads to empty space

[Alan Third]

On Tue, Mar 10, 2020 at 10:42:11AM +0200, Andrii Kolomoiets wrote:
> Please see attached patch. Log message is also provided.

This all looks good to me. Pushed to master as
bbc48b263485c26c6823eabdbbd7e9af62178e34.

Thank you!

-- 
Alan Third

bug#36672: 27.0.50; NS build: Creating child frame leads to empty space

[martin rudalics]

 > This all looks good to me. Pushed to master as
 > bbc48b263485c26c6823eabdbbd7e9af62178e34.

Alan, one child frame related question: When I specify a child frame in
a GNUstep build and move its parent frame around, the child frame does
_not_ move along with the parent but keeps its old position on the
display.  However, clicking into that child frame with the mouse
afterwards, moves it to its previous location within the parent frame.
Is this behavior observable with other NS builds?

Thanks, martin

bug#36672: 27.0.50; NS build: Creating child frame leads to empty space

[Alan Third]

On Fri, Mar 13, 2020 at 10:38:57AM +0100, martin rudalics wrote:
> > This all looks good to me. Pushed to master as
> > bbc48b263485c26c6823eabdbbd7e9af62178e34.
> 
> Alan, one child frame related question: When I specify a child frame in
> a GNUstep build and move its parent frame around, the child frame does
> _not_ move along with the parent but keeps its old position on the
> display.  However, clicking into that child frame with the mouse
> afterwards, moves it to its previous location within the parent frame.
> Is this behavior observable with other NS builds?

On macOS the child windows move with the parent. I thought GNUstep
worked that way too, but in my own experiments I can’t get them to
move at all, even with clicking in the windows like you’re doing.

I just tried an ancient build and it doesn’t work there, so I guess
this has probably never worked right.

I wonder if support is a WM thing?
-- 
Alan Third

bug#36672: 27.0.50; NS build: Creating child frame leads to empty space

[martin rudalics]

 > On macOS the child windows move with the parent.

Good.

 > I thought GNUstep
 > worked that way too, but in my own experiments I can’t get them to
 > move at all, even with clicking in the windows like you’re doing.

Indeed.  What I tried was to move the child frame by dragging its mode
line.  What happens when you call (set-frame-position child-frame x y)
for a child frame and some values of x and y?

 > I just tried an ancient build and it doesn’t work there, so I guess
 > this has probably never worked right.
 >
 > I wonder if support is a WM thing?

Indirectly, maybe.  I have to try with window maker later.

Thanks, martin

bug#36672: 27.0.50; NS build: Creating child frame leads to empty space

[martin rudalics]

 >  > I wonder if support is a WM thing?
 >
 > Indirectly, maybe.  I have to try with window maker later.

Window maker shows the same behavior.  That is, moving a parent frame
does not move the child frame and in addition the child frame does not
necessarily appear on top of the parent frame.  I doubt that we can do
much about this.

martin

bug#36672: 27.0.50; NS build: Creating child frame leads to empty space

[Alan Third]

On Fri, Mar 13, 2020 at 05:29:17PM +0100, martin rudalics wrote:
> > I thought GNUstep
> > worked that way too, but in my own experiments I can’t get them to
> > move at all, even with clicking in the windows like you’re doing.
> 
> Indeed.  What I tried was to move the child frame by dragging its mode
> line.  What happens when you call (set-frame-position child-frame x y)
> for a child frame and some values of x and y?

It does what I expect. So the Emacs side child frame stuff appears to
work, but the GNUstep stuff doesn’t. As far as I can see in the code
we are setting the child frame correctly, so I have to assume it just
doesn’t work.

-- 
Alan Third

bug#36672: 27.0.50; NS build: Creating child frame leads to empty space

[martin rudalics]

 > It does what I expect. So the Emacs side child frame stuff appears to
 > work, but the GNUstep stuff doesn’t. As far as I can see in the code
 > we are setting the child frame correctly, so I have to assume it just
 > doesn’t work.

OK.

Thanks for looking into it, martin