GNOME Bugzilla – Bug 305882
Let users know when an app tries to draw attention to itself
Last modified: 2006-03-30 19:40:21 UTC
Currently, there are a number of apps that try to draw attention to themselves by raising themselves (apparently GAIM does so when the user receives additional IMs when they already have a conversation when a window open, Mozilla/Firefox do so in response to Javascript commands--e.g. in gmail when the user receives another email, and there are probably others). We decided (bug 166395) that apps shouldn't be able to rudely steal toplevel from other apps, just as we don't let apps steal focus. However, unlike attempts to steal focus, we didn't provide any way to notify the user that the app thinks it needs attention when we deny the app from being raised. I think we should do so. I will attach a patch in a minute that sets the demands attention hint on a window whenever it attempts to raise itself above other windows and is denied. This should help solve on of the issues in the GAIM complaints--and may help with other things as well. :-) (How's that for a long description for a two-line (excluding comments) patch?)
Created attachment 47015 [details] [review] Set demands attention hint when a window is denied from being raised to the top
(Side note: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=157271 is related; I think the original comment (especially the "existing windows" part) and points 1 and 2 of https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=157271#c16 are probably the quickest summary)
Comment on attachment 47015 [details] [review] Set demands attention hint when a window is denied from being raised to the top I can't tell from the context of hte patch and am too lazy to go looking, but be sure this only affects raise and not lower, or restack I think the change makes sense in principle though. Maybe we should write a function for setting demands_attention similar to meta_window_maximize(), meta_window_focus(), etc. ?
Okay, I fixed it up to only affect raise and committed. I guess writing a meta_window_set_demands_attention/meta_window_unset_demands_attention makes sense, even if it's only currently two lines--should I leave this bug open for that?
Dropping priority/severity as the main issue was committed and it's only about a possible coding quality enhancement now.
Created attachment 60616 [details] [review] Add meta_window_(un|)set_demands_attention functions Here's a patch to do that. The problem I see with this is that in meta_window_show we now call meta_window_set_demands_attention if necessary, which calls set_net_wm_state, and then go on to call set_net_wm_state ourselves for the other flags-- so that's a bit inefficient. I made it a call to meta_window_set_demands_attention rather than simply setting the flag as before in case one day we want to do something different whenever a window is marked as demanding attention.
I started a similar patch elsewhere having forgotten about this bug, but with the goal in mind of doing something additional when trying to mark a window as demands attention. I didn't finish that patch because of distractions, but maybe I can get you to do so... (if not, I'll still just dump my thoughts here) Basically, the reason for setting demands attention is because we used to place the window on top to let the user notice them (e.g. when showing a new window or the window tried to raise itself); we don't always do that now but we do want to make sure the user still notices them so we do the blinking thing. But, if the window happens to be fully visible anyway, then there's no point in doing the blinking. Thus, I think the meta_window_set_demands_attention() call should first check if the window is obscured by some other window and just return early if it isn't. (This can be achieved using the screen's stack of windows and the meta_rectangle_overlap() function, being careful to account for frames) Note that in the inefficient case you point out with your patch, calling meta_window_set_demands_attention() would become even more ineffecient because there's already code to check for overlapping-ness which can be optimized for that particular case (new windows are either on top or just below the focus window). So, it might be best just to comment that case and set the flag directly, or provide some parameters to meta_window_set_demands_attention() which can avoid some of the extra work in that case. Wow, that took longer to type up than I expected. Oh, well. :) Anyway, the patch looks good and the above is just a bunch of extra stuff we might want to handle. If you don't want to worry about it, you can feel free to commit your patch to HEAD and then I'll handle the other stuff later.
Created attachment 61210 [details] [review] Add meta_window_(un|)set_demands_attention functions Here's a version which checks the stack, then.
Created attachment 61211 [details] Create a window, then try to raise it every five seconds Here's the program I used to test the patch. It's pretty simple, but it was useful. Do we have somewhere in CVS we can keep tools like this one?
Haven't tested the patch, but it looks pretty good overall. Just a couple comments: + if (other_window->workspace == window->workspace) This will fail if either other_window->on_all_workspaces or window->on_all_workspaces is set. + meta_warning (/*META_DEBUG_WINDOW_OPS,*/ From reading this, it looks like you meant to change it back to meta_topic before it went in, which would be good. Definitely good for debugging though (I do this an awful lot...). ;-) (In reply to comment #9) > Here's the program I used to test the patch. It's pretty simple, but it was > useful. Do we have somewhere in CVS we can keep tools like this one? Cool; these little test programs are nice to have. It'd probably make sense to make a place to collect tools like this. I have a testcases directory with just under a dozen similar simple programs; most collected as testcases that others attached to metacity bugs. Havoc: Any suggestions?
Created attachment 62069 [details] [review] Add meta_window_(un|)set_demands_attention functions Thanks! Here's a version which fixes those problems.
Looks good to me, thanks. :)
Fixed in the development version. The fix will be available in the next major release. Thank you for your bug report. yay.
I thought we already had some test apps in the source tree? Anyway I think it'd make sense to keep these in the source tree, in a separate dir that isn't installed, obviously.
Oh, right, we do. They seem to be more about testing certain classes of functionality (gravity, resizing, size-hints, boxes) rather than specific bugs, but a new directory for ones about specific bugs may make sense. I'll try to put a README together so that such testcases can have some consistency to them; for know, I just filed it as bug 336660 so that I'll remember this. :)