unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#3303: delete-frame raises old (invisible) frame
@ 2009-05-16  1:09 David Reitter
  2009-05-17 19:06 ` Stefan Monnier
  0 siblings, 1 reply; 33+ messages in thread
From: David Reitter @ 2009-05-16  1:09 UTC (permalink / raw)
  To: emacs-pretest-bug

Emacs -Q under NS, then

(progn
   (make-frame-invisible nil t)
   (make-frame)
   (delete-frame (selected-frame) t))

will unexpectedly leave one frame visible and raised.  It should  
actually hide all frames.

The code that does it come in via the NS port (75f88b1c by arobert on  
2008-07-15), in do_switch_frame():

#ifdef NS_IMPL_COCOA
   /* term gets no other notification of this */
   if (for_deletion)
     Fraise_frame(Qnil);
#endif

	(do_switch_frame): When for_deletion under Cocoa, add
	Fraise_frame(Qnil).


This seems needed in order to raise another (visible) frame -  
otherwise no frame gets raised (or visibly selected).

The change below would address this - but is Fselect_frame() intended  
to make frames visible (on other platforms)?



diff --git a/src/frame.c b/src/frame.c
index de857af..fbef938 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -868,7 +868,7 @@ do_switch_frame (frame, track, for_deletion,  
norecord)

  #ifdef NS_IMPL_COCOA
    /* term gets no other notification of this */
-  if (for_deletion)
+  if (for_deletion && FRAME_VISIBLE_P (XFRAME (frame)))
      Fraise_frame(Qnil);
  #endif









^ permalink raw reply related	[flat|nested] 33+ messages in thread
* bug#3303: delete-frame raises old (invisible) frame
@ 2009-05-16 19:28 Chong Yidong
  2009-05-17  2:55 ` David Reitter
  0 siblings, 1 reply; 33+ messages in thread
From: Chong Yidong @ 2009-05-16 19:28 UTC (permalink / raw)
  To: David Reitter; +Cc: 3303

> Emacs -Q under NS, then
>
> (progn
>   (make-frame-invisible nil t)
>   (make-frame)
>   (delete-frame (selected-frame) t))
>
> will unexpectedly leave one frame visible and raised.  It should  
> actually hide all frames.

Emacs does this on GNU/Linux as well, at least under the Metacity window
manager.  If we "fix" it---which I'm not sure we should---then the fix
should occur in the platform-independent code.






^ permalink raw reply	[flat|nested] 33+ messages in thread
* bug#3303: delete-frame raises old (invisible) frame
@ 2009-05-22  3:57 David Reitter
  2009-05-25 15:17 ` Stefan Monnier
  0 siblings, 1 reply; 33+ messages in thread
From: David Reitter @ 2009-05-22  3:57 UTC (permalink / raw)
  To: 3303; +Cc: Chong Yidong, Adrian Robert

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

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;
  }




[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2438 bytes --]

^ permalink raw reply related	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2009-06-01  9:37 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-05-16  1:09 bug#3303: delete-frame raises old (invisible) frame David Reitter
2009-05-17 19:06 ` Stefan Monnier
2009-05-17 19:12   ` David Reitter
2009-05-17 20:43     ` Stefan Monnier
2009-05-17 22:27       ` Lennart Borgman
2009-05-18  3:26         ` Stefan Monnier
2009-05-18  1:16       ` Adrian Robert
2009-05-18  3:33         ` Stefan Monnier
2009-05-18  8:05           ` Adrian Robert
2009-05-18 15:08             ` David Reitter
2009-05-18 20:12               ` Stefan Monnier
2009-05-18 23:00                 ` David Reitter
2009-05-19  2:46                   ` Stefan Monnier
2009-05-19  2:56                     ` David Reitter
2009-05-19  3:09                       ` Stefan Monnier
2009-05-19  3:15                         ` David Reitter
2009-05-19  8:20                       ` YAMAMOTO Mitsuharu
2009-05-19 14:30                         ` Stefan Monnier
2009-05-20  2:07                       ` David Reitter
2009-05-19  0:58               ` YAMAMOTO Mitsuharu
2009-05-18  8:19           ` YAMAMOTO Mitsuharu
  -- strict thread matches above, loose matches on Subject: below --
2009-05-16 19:28 Chong Yidong
2009-05-17  2:55 ` David Reitter
2009-05-22  3:57 David Reitter
2009-05-25 15:17 ` Stefan Monnier
2009-05-26 18:20   ` David Reitter
2009-05-26 19:37     ` Stefan Monnier
2009-05-26 20:15       ` David Reitter
2009-05-26 21:30         ` Stefan Monnier
2009-05-27  4:51     ` Adrian Robert
2009-05-27 14:36       ` Stefan Monnier
2009-06-01  9:37         ` Adrian Robert
2009-05-27 15:28       ` David Reitter

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).