GNOME Bugzilla – Bug 630842
Summary item titles for tray items should be clickable
Last modified: 2011-06-21 23:27:47 UTC
Make the titles of summary items that represent tray icons clickable. Right or left clicking them should result in the same action or show the same menu as right or left clicking the icon itself. This is a short-term solution, as we want to get rid of many of the tray icons and represent all the information and options in summary notifications when the icon should in fact be present in the message tray.
Created attachment 178252 [details] [review] messageTray: forward clicks on trayicon SummaryItems to the icon (WIP) If the user clicks on the title of a trayicon's SummaryItem, forward that click to the trayicon. Not fully functional; if the clicked-on window grabs the pointer to pop up a menu, then the summary will close (while leaving the menu popped up). Using XWarpPointer instead of fake Enter/Leave events has the same problems (except even if the icon *doesn't* do a grab), and not doing any pointer fakery at all makes the trayicon reject the event, because gdk filters it out since the pointer isn't actually in the window that the event claims was clicked.
Created attachment 178738 [details] [review] shell-global: try to resync the pointer state after grabs (WIP) If the pointer moves on or off the stage while another process has a grab, we will lose track of it. One example of this is that if you use a popup menu from a message tray trayicon, the tray will stay up after the menu goes away, because the shell never saw the pointer leave it. Add a new method shell_global_sync_pointer() that causes clutter to recheck what actor is under the pointer and generate leave/enter events if appropriate. Of course, we can't actually tell for sure when another process has a grab, so we need a heuristic of when to call this. Currently we call it from Chrome._windowsRestacked(), which is not really the right thing at all, but does fix the menu-from-trayicon case...
Review of attachment 178738 [details] [review]: This looks like a reasonable hack, but it doesn't really point toward a solution for the forwarded event case, does it?
conversation from irc from after the first patch: <owen> danw: figured out why the LeaveEvent isn't delivered <owen> danw: when you right click on the empathy status icon, you get an implicit grab on the pointer, which affects the subsequent crossing events <owen> danw: including the ones from NotifyGrab <danw> owen: hm... so if I can manage to grab the pointer to the icon window without *that* triggering a LeaveNotify then that should fix it <owen> I can't see how grabbing the pointer to the icon window is going to work - if the pointer is grabbed by mutter, empathy (or whatever) isn't going to be able to pop up a menu <owen> and "work" here means "tray potentially stuck up" <danw> oh... was thinking that grabbing a foreign window would dtrt somehow <owen> grabbing is always about the client that grabs <owen> there's no way to "inject" a grab into a different client <danw> so we'd need to filter out the NotifyGrab in this case then <owen> I think my best idea here is to use our position as wm and our SubstructureNotify on the root to keep track of a "menu up" state and trust toolkits to put properties on menus that allow us to identify menus and distinguish them from tooltips <owen> We'll get the MapNotify for the menu *before* we get the GrabNotify, so the idea would be that when a menu is up we freeze the state of the notification tray, and when we leave the "menu up" state, we do an explicit query for pointer location to see if we should dismiss the tray <danw> you need a per-window menu-up state. if the tray slides up and then you open a menu in the main empathy window, you don't want that to hold the tray open <owen> danw: I'd consider that acceptable collateral damage <owen> I mean, why would the tray be open when you were opening a menu in the main empathy window? <danw> because you received a notification <owen> I think people aren't going to really notice or care if the tray doesn't close until they've finished menu navigation <owen> on #fedora-desktop halfline suggested we could make the case where we aren't forwarding the event the same as the case where we do forward the event by using XGrabButton. And then since in both cases we know when the menu popping up starts we could use some sort of polling mode.... not too crazy about that, and it wouldn't give us the behavior we want where the tray stays up until the menu is dismissed
*** Bug 641420 has been marked as a duplicate of this bug. ***
Created attachment 180518 [details] [review] messageTray: forward clicks on trayicon SummaryItems to the icon (WIP) minor updates, including gcc-4.6 happiness
For some reason, this does not work with gpk-update-icon in rawhide. (I don't remember if I originally wrote this patch before or after updating to rawhide.) It does still work with some other (non-gtk3-based) trayicons. stracing gpk-update-icon shows that it's not even receiving the faked events from the X server. adding an "or mode == NotifyGrab and detail == NotifyNonlinear" to the existing hack in gnome_shell_plugin_xevent_filter() fixes the clutter-thinks-we-left-when-the-icon-gets-a-grab problem, but I'm not sure if it would spoil anything else. We could add shell_global_ignore_nonlinear_grab_within_the_next_half_second() or something. Was trying to think if there might be other ways to make this work by disconnecting the X Window from its ClutterActor in various ways. Eg, make the window rectangular but the actor square. Or move the Window around so it's always directly underneath the pointer if the pointer is over the label :-)
(In reply to comment #7) > For some reason, this does not work with gpk-update-icon in rawhide. (I don't > remember if I originally wrote this patch before or after updating to rawhide.) > It does still work with some other (non-gtk3-based) trayicons. stracing > gpk-update-icon shows that it's not even receiving the faked events from the X > server. Maybe XI2 related?
(In reply to comment #8) > Maybe XI2 related? Ah, yes, calling gdk_disable_multidevice() makes it work. So... I guess it needs to send both core and XI2 enter/press/release/leave events. (Or will something synthesize core events from the XI2 events if needed? I guess I can experiment.)
*** Bug 644191 has been marked as a duplicate of this bug. ***
This has been pointed by the marketing team as something very important for GNOME 3.0, I am marking it as a blocker so we can track it.
OK, it is not currently possible to XSendEvent() an XI2 event. Carlos Garnacho sent a patch for this to xorg-devel in November (http://lists.x.org/archives/xorg-devel/2010-November/016078.html), but it had a few problems and hasn't been committed yet (and presumably even if it got committed this week, we could not count on having the new libX11 available for GNOME 3.0). So... I don't think there are any good possibilities for fixing this at this point. One thing that might mitigate it a little bit would be to modify bug 644361 so that trayicon items have an "insensitive" style, that doesn't prelight, and has a grayed out title, to try to suggest that you can't click on the title...
Created attachment 183452 [details] [review] messageTray: forward clicks on trayicon SummaryItems to the icon If the user clicks on the title of a trayicon's SummaryItem, forward that click to the trayicon. Also adjust gnome_shell_plugin_xevent_filter() so that if the trayicon takes a grab as a result of this, we don't hide the message tray. === The part of the earlier gnome_shell_plugin_xevent_filter() comment about clutter not generating an enter event for the trayicon was sort of confused; clutter wouldn't generate an enter event for it anyway, because the trayicon window gets the pointer events when the pointer is over it, so clutter sees nothing and so correspondingly generates no events. So I just removed that part. Requires bug 644847 to work with gtk3 trayicons. (You can use tests/teststatusicon in the gtk tree as a test.)
Created attachment 183453 [details] [review] shell-global: try to resync the pointer state after grabs If the pointer moves on or off the stage while another process has a grab, we will lose track of it. One example of this is that if you use a popup menu from a message tray trayicon, the tray will stay up after the menu goes away, because the shell never saw the pointer leave it. Add a new method shell_global_sync_pointer() that causes clutter to recheck what actor is under the pointer and generate leave/enter events if appropriate. Of course, we can't actually tell for sure when another process has a grab, so we need a heuristic of when to call this. Currently we call it from Chrome._windowsRestacked(), which is not really the right thing at all, but does fix the menu-from-trayicon case... ====== still not sure this is the bestest fix. It occurs to me that we can probably use shell_global_sync_pointer() to replace our other hover- syncing hacks elsewhere though...
*** Bug 644524 has been marked as a duplicate of this bug. ***
Review of attachment 183452 [details] [review]: generally looks good and seems to work.b ::: js/ui/messageTray.js @@ +1177,3 @@ + summaryItem.actor.connect('clicked', Lang.bind(this, + function (actor, button) { + this._onSummaryItemClicked(summaryItem, button); By changing this to 'clicked', you've moved the right click popup menu for normal summary items from press to release. Generally I'd say that is wrong, though if I guess here it could be seen as intentional to get consistency with the legacy status icons. And from another perspective, the right click menu is pretty much identical to the left click bubble and they should interact the same with with the mouse. Up to you whether you think it's worth doing something here or not. But actually, I don't get the right click menu at all, see below. @@ +1395,3 @@ + this._unsetClickedSummaryItem(); + else if (button != 1) + return; from discussion on irc, this should be removed. ::: src/gnome-shell-plugin.c @@ +334,3 @@ + /* If the pointer is grabbed by a window it is not currently in, + * filter that out as well; this will leave Clutter out of sync, + * but that's standard behavior for grabs. In particular, if Not really - what's standard is that you have to choose between considering grab notifies to be actual leaves or not being able to track. @@ +336,3 @@ + * but that's standard behavior for grabs. In particular, if + * a trayicon grabs the pointer after a click on its label, + * we don't want to hide the message tray. Might be good to mention what we do to get back into sync, even if that's a separate follow-on patch.
Review of attachment 183453 [details] [review]: Fine except for comment issues. ::: js/ui/chrome.js @@ +337,3 @@ } + + global.sync_pointer(); I'd like to see a comment in the code here and not require git blame to figure out what this is about and what it relates to. The fact that this has a relationship to message tray behavior and ignoring leave events in gnome-shell-plugin.c would be impossible to figure out without git archeology. ::: src/shell-global.c @@ +1489,3 @@ + ClutterMotionEvent event; + + gdk_display_get_pointer (gdk_display_get_default (), NULL, &x, &y, &mods); One thing here that should be commented here is that we always consider the pointer to be on the stage - while normally when we aren't over the input region we are *off* the stage. It might be possible to use XQueryPointer() directly and look at the child argument to see if it's the COW. I'm a little concerned that that might cause problems if this function gets called during a grab, though I can't think of any specific problems. Probably better to just comment the situation until and unless we get problems for not getting stage leaves. @@ +1504,3 @@ + /* Leaving event.source NULL will force clutter to look it up, which + * will generate enter/leave events as a side effect, if they are + * needed. This strikes me as vastly unsafe against future clutter changes. Can you file a bug asking for a supported way of doing this and put the link here?
*** Bug 644363 has been marked as a duplicate of this bug. ***
This patch seems to be triggering warnings from GTK+ 2 applications like GNote: (gnote:14943): Gdk-CRITICAL **: IA__gdk_window_get_root_coords: assertion `GDK_IS_WINDOW (window)' failed
filed http://bugzilla.clutter-project.org/show_bug.cgi?id=2615 "need a way to forcibly resync input device state"
(In reply to comment #19) > This patch seems to be triggering warnings from GTK+ 2 applications like GNote: > > (gnote:14943): Gdk-CRITICAL **: IA__gdk_window_get_root_coords: assertion > `GDK_IS_WINDOW (window)' failed Nope, gtk2 GtkTooltip bug; it happens under gnome-panel in GNOME 2.32 too.
Attachment 183452 [details] pushed as 7f17fcf - messageTray: forward clicks on trayicon SummaryItems to the icon Attachment 183453 [details] pushed as 2782011 - shell-global: try to resync the pointer state after grabs
*** Bug 638998 has been marked as a duplicate of this bug. ***
*** Bug 643332 has been marked as a duplicate of this bug. ***
*** Bug 651945 has been marked as a duplicate of this bug. ***
*** Bug 653141 has been marked as a duplicate of this bug. ***