GNOME Bugzilla – Bug 647712
meta_window_propagate_focus_appearance() in multi-generational families
Last modified: 2011-07-07 21:48:59 UTC
In various places - meta_window_unmanage(), reload_transient_for() we do something like: if (window->has_focus && window->xtransient_for != None) meta_window_propagate_focus_appearance (window, FALSE); But this isn't right if we have parent/child/grand-child, it really should be: meta_window_appears_focused() && window->transient_for != None I think.
Created attachment 185960 [details] [review] window: fix appears-focused propagation with attached grandchildren When detaching/attaching a dialog, we were only updating appears-focused on the parent if the child itself was focused, but in fact, we need to do it if the child has an attached child which is focused too. To simplify the case of detaching a focused subtree from its parent, we change meta_window_propagate_focus_appearance() to use @window->display->focus_window as the window to add/remove as the attached_focus_window, and @window only as the starting point to propagate from. That way we can propagate focus-removal up to @window's (soon-to-be-ex-)ancestors without having to remove it from its descendants as well.
poke
Created attachment 191493 [details] [review] reload_transient_for: avoid xtransient_for loops Don't set a window's xtransient_for if it would create a loop. Since this is the only place we ever set xtransient_for, we can therefore assume everywhere else that it does not loop.
Created attachment 191494 [details] [review] window: fix appears-focused propagation with attached grandchildren When detaching/attaching a dialog, we were only updating appears-focused on the parent if the child itself was focused, but in fact, we need to do it if the child has an attached child which is focused too. To simplify the case of detaching a focused subtree from its parent, we change meta_window_propagate_focus_appearance() to use @window->display->focus_window as the window to add/remove as the attached_focus_window, and @window only as the starting point to propagate from. That way we can propagate focus-removal up to @window's (soon-to-be-ex-)ancestors without having to remove it from its descendants as well.
Review of attachment 191493 [details] [review]: I can't find a loophole here - it's not completely obvious, because the MetaWindow pointed to be window->xtransient_for can change arbitrarily - we can have one window be removed, and another show up (which makes the "Invalid WM_TRANSIENT_FOR window 0x%lx specified" check rather dubious - we have to be able that case because it could become true at any time even if it wasn't initially.) But the only time window->xtransient_for changes to a different MetaWindow is when a new MetaWindow is mapped, and at that point, we'll reload the new MetaWindow's xtransient_for, and break any loops there.
Review of attachment 191494 [details] [review]: Looks good to me
Attachment 191493 [details] pushed as 6dc79ce - reload_transient_for: avoid xtransient_for loops Attachment 191494 [details] pushed as 48cabd1 - window: fix appears-focused propagation with attached grandchildren