[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#3303: delete-frame raises old (invisible) frame
From: |
Stefan Monnier |
Subject: |
bug#3303: delete-frame raises old (invisible) frame |
Date: |
Mon, 25 May 2009 11:17:17 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.0.93 (gnu/linux) |
> OK, I think I have a set of fixes now for this bug.
Good, thanks.
> 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.
Agreed, this is a separate problem.
I'm not familiar enough with the NS code to judge if the patch is OK,
but here are some comments:
> frame.c:
> Fraise_frame: do not make invisible frames visible (Stefan Monnier).
This is not OK for 23.1. It might be good to try it for 23.2.
Also I think your other changes should make it unnecessary for the
problem we're trying to fix.
> nsterm.m:
> ns_raise_frame(): only raise frame if visible.
Since you say that "invisible" is implemented by lowering the window
below the background, this seems like a plain bug-fix, yes.
> x_make_frame_visible(): move frame to front rather than calling
> ns_raise_frame().
Sounds right.
> keyDown: do not swallow events that aren't re-sent if frame isn't
> key window.
If you say so.
> drawRect: do not set visibility/iconified flags because drawRect may be
> called by NSView even if the frame is hidden.
Do you happen to know why/when NSView might be called even for a frame
that's not visible?
> nsfns.m:
> Fx_create_frame(): follow other ports in determining visibility; default to
> t. Ensure async_visible is set.
Sounds good.
> + f->async_visible=1;
[...]
> + f->async_visible=0;
Please put spaces around infix operators (like `=' above).
> @@ -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;
> + }
> }
Shouldn't we BLOCK_INPUT around the whole thing, just in case
async_visible gets changed at the wrong time? It probably
doesn't matter.
> {
> 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;
> }
I think I'd prefer:
if (!FRAME_VISIBLE_P (f))
+ {
+ f->async_visible = 1;
ns_raise_frame (f);
+ }
> @@ -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;
> }
In this kind of change, it's often good to keep the old code commented
out, with a comment explaining why it was removed (and ideally why it
was there before as well).
Stefan
bug#3303: delete-frame raises old (invisible) frame, Chong Yidong, 2009/05/16
bug#3303: delete-frame raises old (invisible) frame, David Reitter, 2009/05/22
- bug#3303: delete-frame raises old (invisible) frame,
Stefan Monnier <=
bug#3303: delete-frame raises old (invisible) frame, Adrian Robert, 2009/05/27
bug#3303: delete-frame raises old (invisible) frame, Stefan Monnier, 2009/05/27
bug#3303: delete-frame raises old (invisible) frame, David Reitter, 2009/05/27