After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 305882 - Let users know when an app tries to draw attention to itself
Let users know when an app tries to draw attention to itself
Status: RESOLVED FIXED
Product: metacity
Classification: Other
Component: general
trunk
Other Linux
: Low enhancement
: ---
Assigned To: Thomas Thurman
Metacity maintainers list
Depends on:
Blocks:
 
 
Reported: 2005-05-30 00:53 UTC by Elijah Newren
Modified: 2006-03-30 19:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Set demands attention hint when a window is denied from being raised to the top (1.11 KB, patch)
2005-05-30 00:54 UTC, Elijah Newren
committed Details | Review
Add meta_window_(un|)set_demands_attention functions (3.61 KB, patch)
2006-03-04 01:51 UTC, Thomas Thurman
none Details | Review
Add meta_window_(un|)set_demands_attention functions (5.14 KB, patch)
2006-03-14 01:35 UTC, Thomas Thurman
needs-work Details | Review
Create a window, then try to raise it every five seconds (454 bytes, text/x-csrc)
2006-03-14 03:31 UTC, Thomas Thurman
  Details
Add meta_window_(un|)set_demands_attention functions (5.26 KB, patch)
2006-03-26 21:22 UTC, Thomas Thurman
committed Details | Review

Description Elijah Newren 2005-05-30 00:53:20 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?)
Comment 1 Elijah Newren 2005-05-30 00:54:07 UTC
Created attachment 47015 [details] [review]
Set demands attention hint when a window is denied from being raised to the top
Comment 2 Elijah Newren 2005-05-30 20:07:03 UTC
(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 3 Havoc Pennington 2005-06-01 15:02:14 UTC
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.  ?
Comment 4 Elijah Newren 2005-06-02 15:50:53 UTC
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?
Comment 5 Elijah Newren 2005-07-12 17:10:53 UTC
Dropping priority/severity as the main issue was committed and it's only about a
possible coding quality enhancement now.
Comment 6 Thomas Thurman 2006-03-04 01:51:23 UTC
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.
Comment 7 Elijah Newren 2006-03-11 20:47:01 UTC
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.
Comment 8 Thomas Thurman 2006-03-14 01:35:12 UTC
Created attachment 61210 [details] [review]
Add meta_window_(un|)set_demands_attention functions

Here's a version which checks the stack, then.
Comment 9 Thomas Thurman 2006-03-14 03:31:19 UTC
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?
Comment 10 Elijah Newren 2006-03-25 04:27:29 UTC
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?
Comment 11 Thomas Thurman 2006-03-26 21:22:17 UTC
Created attachment 62069 [details] [review]
Add meta_window_(un|)set_demands_attention functions

Thanks! Here's a version which fixes those problems.
Comment 12 Elijah Newren 2006-03-29 17:39:52 UTC
Looks good to me, thanks.  :)
Comment 13 Thomas Thurman 2006-03-29 18:47:17 UTC
Fixed in the development version. The fix will be available in the next major release. Thank you for your bug report.

yay.
Comment 14 Havoc Pennington 2006-03-30 06:12:36 UTC
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.
Comment 15 Elijah Newren 2006-03-30 19:40:21 UTC
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.  :)