bug#3303: delete-frame raises old (invisible) frame

[David Reitter]

OK, I think I have a set of fixes now for this bug.
The problems included swallowed events, setting async_visible and  
async_iconified from drawRect under the false assumption that this  
proves that the frame view is visible, and attendant failure to set  
visibility correctly when creating the frame (as the other ports,  
including Appkit, do).  Also, we separate visibility from frame order  
in the NS layer.

Description, test cases and patch below.
Can someone look this over to see if there is anything obviously  
wrong?  I'll check it in pending feedback.

- David

PS.: I agree with the proposed change to after-make-frame-functions  
(not selecting the frame immediately), but this doesn't relate to the  
bug at hand.  Also, I don't know why the modeline isn't updated.



Desc:

frame.c:
Fraise_frame: do not make invisible frames visible (Stefan Monnier).

nsterm.m:
ns_raise_frame(): only raise frame if visible.
x_make_frame_visible(): move frame to front rather than calling  
ns_raise_frame().
keyDown: do not swallow events that aren't re-sent if frame isn't key  
window.
drawRect: do not set visibility/iconified flags because drawRect may  
be called by NSView even if the frame is hidden.

nsfns.m:
Fx_create_frame(): follow other ports in determining visibility;  
default to t. Ensure async_visible is set.

Test cases:


;; test case
;; second time around aaa is t
(let ((f (selected-frame)))
  (make-frame-invisible f t)
  (setq aa2v (frame-visible-p f))
  (redisplay)
  (setq aa3v (frame-visible-p f))
)
;; frame should be gone
(list aa2v aa3v)
;; should be nil nil

;; test case
(progn
 (make-frame-invisible (selected-frame) t)
 (select-frame (make-frame))
 (delete-frame (selected-frame) t)
 (select-frame (make-frame))
 (sit-for 0)
 (delete-frame (selected-frame) t))
;; there should be no frame



Patch:


diff --git a/src/frame.c b/src/frame.c
index afcc96c..fbb00ba 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -2024,7 +2024,7 @@ doesn't support multiple overlapping frames,  
this function selects FRAME.  */)
   if (FRAME_TERMCAP_P (f))
     /* On a text-only terminal select FRAME.  */
     Fselect_frame (frame, Qnil);
-  else
+  else if (FRAME_ICONIFIED_P (f))
     /* Do like the documentation says. */
     Fmake_frame_visible (frame);

diff --git a/src/nsfns.m b/src/nsfns.m
index 01ffcf1..25d9b30 100644
--- a/src/nsfns.m
+++ b/src/nsfns.m
@@ -1317,13 +1317,20 @@ be shared by the new frame.  */)

   if (! f->output_data.ns->explicit_parent)
     {
-        tem = x_get_arg (dpyinfo, parms, Qvisibility, 0, 0,  
RES_TYPE_BOOLEAN);
-        if (EQ (tem, Qunbound))
-            tem = Qnil;
-
-        x_set_visibility (f, tem, Qnil);
-        if (EQ (tem, Qt))
-            [[FRAME_NS_VIEW (f) window] makeKeyWindow];
+      tem = x_get_arg (dpyinfo, parms, Qvisibility, 0, 0,  
RES_TYPE_SYMBOL);
+      if (EQ (tem, Qunbound))
+       tem = Qt;
+      x_set_visibility (f, tem, Qnil);
+      if (EQ (tem, Qicon))
+       x_iconify_frame (f);
+      else if (! NILP (tem))
+       {
+         x_make_frame_visible (f);
+         f->async_visible=1;
+         [[FRAME_NS_VIEW (f) window] makeKeyWindow];
+       }
+      else
+         f->async_visible=0;
     }

   if (FRAME_HAS_MINIBUF_P (f)
diff --git a/src/nsterm.m b/src/nsterm.m
index 9ca74e8..d7a8f81 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -895,9 +895,14 @@ ns_raise_frame (struct frame *f)
 {
   NSView *view = FRAME_NS_VIEW (f);
   check_ns ();
-  BLOCK_INPUT;
-  [[view window] makeKeyAndOrderFront: NSApp];
-  UNBLOCK_INPUT;
+
+  FRAME_SAMPLE_VISIBILITY (f);
+  if (FRAME_VISIBLE_P (f))
+    {
+      BLOCK_INPUT;
+      [[view window] makeKeyAndOrderFront: NSApp];
+      UNBLOCK_INPUT;
+    }
 }


@@ -979,11 +984,17 @@ x_make_frame_visible (struct frame *f)
     
-------------------------------------------------------------------------- */
 {
   NSTRACE (x_make_frame_visible);
+  NSView *view = FRAME_NS_VIEW (f);
   /* XXX: at some points in past this was not needed, as the only  
place that
      called this (frame.c:Fraise_frame ()) also called raise_lower;
      if this ends up the case again, comment this out again. */
   if (!FRAME_VISIBLE_P (f))
-    ns_raise_frame (f);
+    {
+      BLOCK_INPUT;
+      [[view window] makeKeyAndOrderFront: NSApp];
+      UNBLOCK_INPUT;
+    }
+  f->async_visible = 1;
 }


@@ -4461,7 +4472,8 @@ extern void update_window_cursor (struct window  
*w, int on);
   if (!emacs_event)
     return;

- if (![[self window] isKeyWindow])
+ if (![[self window] isKeyWindow]
+     && [[theEvent window] isKindOfClass: [EmacsWindow class]])
    {
      /* XXX: There is an occasional condition in which, when Emacs  
display
          updates a different frame from the current one, and  
temporarily
@@ -4469,8 +4481,7 @@ extern void update_window_cursor (struct window  
*w, int on);
          (dispnew.c:3878), OS will send the event to the correct  
NSWindow, but
          for some reason that window has its first responder set to  
the NSView
          most recently updated (I guess), which is not the correct  
one. */
-     if ([[theEvent window] isKindOfClass: [EmacsWindow class]])
-         [(EmacsView *)[[theEvent window] delegate] keyDown: theEvent];
+     [(EmacsView *)[[theEvent window] delegate] keyDown: theEvent];
      return;
    }

@@ -5466,8 +5477,6 @@ extern void update_window_cursor (struct window  
*w, int on);

   ns_clear_frame_area (emacsframe, x, y, width, height);
   expose_frame (emacsframe, x, y, width, height);
-  emacsframe->async_visible = 1;
-  emacsframe->async_iconified = 0;
 }

smime.p7s
Description: S/MIME cryptographic signature