GNOME Bugzilla – Bug 154598
Need to alter the meaning of expected_focus_window
Last modified: 2004-12-23 06:45:20 UTC
Currently, display->expected_focus_window is set to match the following: A) If a window is about to receive a FocusIn due to calling meta_window_focus, this is the expected_focus_window B) If that window receives a FocusIn, the expected_focus_window is NULL. So, the expected_focus_window is NULL most of the time. That seems a little odd from it's name. Here is what I thought it would mean, and what I propose changing it to: A) If we expect a certain window to have focus, it is the expected_focus_window. B) If we expect the "dummy" no_focus_window to have focus, the expected_focus_window is NULL. (Though it's probably obvious, a small clarification: "Expect a window to have focus" means we have called XSetInputFocus on that window; we don't wait for it to receive a FocusIn) I've searched through all occurrences of expected_focus_window in the code (there aren't that many) in addition to testing it, and making this change presents no problems whatsoever. It also makes it possible to fix a remaining focus consistency bug (LeaveNotify events use window->has_focus which is problmatic; note that there's a comment for EnterNotify events that window->has_focus has to be ignored due to possible race conditions). There's another way of thinking about this change. What it does is make "window->has_focus" and "window==window->display->expected_focus_window" have a similar meaning (which I had previously assumed they already had). With the change, the two expressions will have the same truth value most of the time. The difference is that the former expression will tend to lag the latter one, because it is set when FocusIn/FocusOut events are received whereas the latter is set when calls to XSetInputFocus are made. I will attach a patch momentarily.
Created attachment 32257 [details] [review] Fix the expected focus window There are some things I'm wondering about with this patch, that may make sense to do: Calling XSetInputFocus won't always result in the new window getting focus--it depends on the timestamp. Perhaps we should have a wrapper function that calls XSetInputFocus and keeps track of the last focus timestamp so that it can know whether to update the expected_focus_window. "window->has_focus || window==window->display->expected_focus_window" appears in a couple places in stack.c (in determining whether to put a window into META_LAYER_FULLSCREEN). We could now drop the window->has_focus half, unless we really do want this to continue to be true for a little while after we've called XSetInputFocus for a different window.
This change would also allow us to use if (window == window->display->expected_focus_window) return; inside meta_window_focus, which may be the fix we need for bug 140977.
Comment on attachment 32257 [details] [review] Fix the expected focus window This looks plausible to me. Your suggestions to wrap SetInputFocus etc. also sound like they'd be worth trying out.
Comment on attachment 32257 [details] [review] Fix the expected focus window Committed, now the patch in bug 154601 is valid and can be reviewed.
Created attachment 35150 [details] [review] Wrap XSetInputFocus This patch: - introduces meta_display_set_input_focus_window, along with a last_focus_time - fixes the XSERVER_TIME macro to reflect the gtk+ version (gdkwindow-x11.c; I believe the current metacity version is problematic on 64-bit systems) - moves the autoraise callback removal from meta_window_focus to meta_display_set_input_focus_window, which seemed to be slightly more natural (I also considered making meta_display_remove_autoraise_callback static...). I think that makes expected_focus_window almost everything we could ask for. Of course, it's not quite as perfect as I'd hoped (see below), so it's possible we might not want to depend on it heavily. In particular, my comments about stack.c (see comment 1) and a fix for bug 140977 (see comment 2) might not work. Gory details on caveat: We can't make it perfect, because applications can call XSetInputFocus directly (and are even supposed to, it appears, in the case of WM_TAKE_FOCUS--though I still don't understand WM_TAKE_FOCUS much so I'm not completely sure how that works). We, of course, can't detect that. I tried adding some code that would also update the expected_focus_window when we receive FocusIn or FocusOut events, but that failed--I can easily get multiple windows queued up for FocusIn/FocusOut events and then when they occur the expected_focus_window is unset from the correct value to something else (although it eventually gets the right value with the last FocusIn/FocusOut--but that defeats the whole point of having an expected_focus_window, and it ends up being not much different than just a second copy of the focus_window).
Comment on attachment 35150 [details] [review] Wrap XSetInputFocus Looks good to me, thanks.
Committed.