Subject: Re: bug#16097: [PATCH] Treat windows on other spaces as not visible in Cocoa

From: Alan Third <[email-masked]>
Date: Sat, 29 Jan 2022 00:16:24 +0000

On Fri, Jan 28, 2022 at 09:17:24AM +0100, Robert Pluim wrote:
> +- (void)updateVisibility: (NSNotification *)notification
> +{
> +  NSTRACE (updateVisibility);
> +
>    if (!emacsframe->output_data.ns)
>      return;
>  
> -  SET_FRAME_ICONIFIED (emacsframe, 1);
> -  SET_FRAME_VISIBLE (emacsframe, 0);
> +  NSWindow *win = [self window];
> +  BOOL on_active_space = YES;
> +  if ([win respondsToSelector: @selector (isOnActiveSpace)])
> +    on_active_space = [win isOnActiveSpace];
> +  if (on_active_space && [win isVisible])
> +    {
> +      if (FRAME_VISIBLE_P (emacsframe) && !FRAME_ICONIFIED_P (emacsframe))
> +        return;
>  
> -  if (emacs_event)
> +      SET_FRAME_ICONIFIED (emacsframe, 0);
> +      SET_FRAME_VISIBLE (emacsframe, 1);
> +      windows_or_buffers_changed = 63;
> +
> +      if (emacs_event)
> +        {
> +          emacs_event->kind = DEICONIFY_EVENT;
> +          EV_TRAILER ((id)nil);
> +        }
> +    }
> +  else
>      {
> -      emacs_event->kind = ICONIFY_EVENT;
> -      EV_TRAILER ((id)nil);
> +      if (!FRAME_VISIBLE_P (emacsframe) && FRAME_ICONIFIED_P (emacsframe))
> +        return;
> +
> +      SET_FRAME_ICONIFIED (emacsframe, 1);
> +      SET_FRAME_VISIBLE (emacsframe, 0);
> +
> +      if (emacs_event)
> +        {
> +          emacs_event->kind = ICONIFY_EVENT;
> +          EV_TRAILER ((id)nil);
> +        }
>      }
>  }

Why are we marking all frames not on the current space as iconified?
We're also, as far as I can tell, marking all invisible frames on the
current space as iconified too. We'll be sending these iconification
events to Emacs every time the space is switched, whether their status
has changed or not.

I'm unsure what happens to actually minimised frames here.

I'm still not sure this is the best approach versus, say modifying
candidate_frame in frame.c to check if the frame is on the active
space, similar to how it currently checks if the frames use the same
keyboard (which is a check that's not even relevant on macOS, afaict).

Even if it is the best approach it looks to me like it should really
be more streamlined. I don't think we want to call the same function
for (de)minimising and changing space. They strike me as distinctly
different operations and I think the conflation of the two is making
this code's logic... questionable.
-- 
Alan Third