GNOME Bugzilla – Bug 136581
Fix window minimization vs. activation for mouse focus
Last modified: 2004-12-22 21:47:04 UTC
In both sloppy and click-to-focus modes, when I click on the window in the tasklist corresponding to the most recently activated window, the window is minimized. If I click on any other window, the window is raised. In mouse focus, even if the window I click on was the most recently activated window it can be activated instead of minimized. This seems counter-intuitive to me. The reason for this is that in mouse focus the window will be deactivated by leaving the window, and libwnck uses was-currently-active logic instead of was-most-recently-activated logic. [Two sidenotes: (1) was-currently-active is slightly different than is-currently-active, since the panel becomes activated when clicking on it. (2) was-most-recently-activated is different than Metacity's "Most Recently Used" concept--as it should be--since the most recently activated window may no longer exist] I will attach a patch that fixes this. I've tested with click-to-focus, sloppy focus, and mouse focus. For click-to-focus and sloppy focus, there's no change in behavior, even for the corner cases that occurred to me (think "switching workspaces in order to make no window focused in sloppy focus"). For mouse focus, it fixes the problem quite nicely. Comments?
Created attachment 25356 [details] [review] Fix activation vs. minimization for mouse focus
We're in hard code freeze, so I'm adding the BLOCKED_BY_FREEZE keyword. It looks like I also forgot to add the PATCH keyword before.
Needs to be looked at for 2.7.x.
Created attachment 30286 [details] [review] Use -p in the patch to make it easier to read Same patch, updated to CVS HEAD as of today (changes the line numbers slightly). I've also used the -p flag this time around to make the patch easier to read.
Elijah [thinking to himself]: Let's see...I need an excuse to cc metacity-maint because Havoc doesn't read libwnck email, but does read stuff sent to metacity-maint. And this bug still bothers me unless I take the time to manually apply this patch. Elijah: Uh, this bug affects metacity-libwnck interaction, so I'm cc'ing metacity-maint. Um, yeah. Something like that. :-)
haha, but my procmail rule to go to the libwnck folder is before the one to go to the metacity folder, so if you copy both I still won't read it ;-) For whatever reason I'm finding the explanation and the patch here really confusing ;-) Seem to be using different terminology in the bug and the patch? Maybe I need to go refamiliarize with whatever existing hacks we have to understand this layered hack.
Darn, foiled again! :-) Okay, let me try explaining better. Let's say you have a single window on your desktop and it happens to be focused. Let's say you want to minimize it. With click or sloppy focus, you move the mouse to the tasklist, click on the icon for the window, and it minimizes. (because that window was active immediately prior to clicking on the panel tasklist applet) With mouse focus, you move the mouse to the tasklist, click on the icon for the window, and it activates the window. Annoying. The problem? The window was defocused/deactivated when you moved the mouse outside of it. My solution? Keep track of the most-recently-activated window.
Er.... let me change that last sentence to "Keep track of the information necessary to determine the most-recently-activated-window." In particular, I keep track of the previously_active_window in addition to the active_window. The idea is if (active_window != NULL) most_recently_activated_window = active_window; else most_recently_activated_window = previously_active_window and then use most_recently_activated_window instead of active_window to determine whether to minimize. Doesn't change a thing for sloppy or click focus methods (because active_window won't be NULL), but fixes the issue seen when using mouse focus.
Comment on attachment 30286 [details] [review] Use -p in the patch to make it easier to read maybe is_most_recently_activated() is a better name? would be good to set previously_active_window = NULL if the window is freed, just to avoid a dangerous dangling pointer (hopefully we do that with active_window already) The docs for was_most_recently_activated() could be clearer - suggest using the explanation you just added to the bug comments ;-) looks good otherwise, thanks.
Created attachment 30505 [details] [review] Incorporate Havoc's feedback > maybe is_most_recently_activated() is a better name? yeah, that'd be more consistent. I fixed that. > would be good to set previously_active_window = NULL if the window is > freed, just to avoid a dangerous dangling pointer (hopefully we do > that with active_window already) Yeah, was already done with active_window. This patch now does it for previously_active_window too. > The docs for was_most_recently_activated() could be clearer - suggest > using the explanation you just added to the bug comments ;-) I totally reworked the comments and docs using my previous explanation.
Comment on attachment 30505 [details] [review] Incorporate Havoc's feedback looks good, thanks.
committed.