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 723636 - group: select for property notifications on the leader window
group: select for property notifications on the leader window
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2014-02-04 22:21 UTC by Giovanni Campagna
Modified: 2015-06-28 20:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
group: select for property notifications on the leader window (6.85 KB, patch)
2014-02-04 22:21 UTC, Giovanni Campagna
reviewed Details | Review

Description Giovanni Campagna 2014-02-04 22:21:55 UTC
If we don't select for PropertyChangeMask, we never notice changes
in _NET_STARTUP_ID, so meta_window_get_startup_id() returns a
stale value for the second window of a gtk app (because gtk
only changes the startup id on the leader), meaning that
meta_screen_apply_startup_properties() bails and the workspace
and timestamp are wrong (most importantly the workspace, because
the timestamp is also communicated to gtk through the ID and
already set properly)
Comment 1 Giovanni Campagna 2014-02-04 22:21:58 UTC
Created attachment 268106 [details] [review]
group: select for property notifications on the leader window
Comment 2 Giovanni Campagna 2014-03-05 17:07:49 UTC
Hey, any update on this?
Comment 3 Jasper St. Pierre (not reading bugmail) 2014-03-05 18:56:39 UTC
What exactly does this fix?
Comment 4 Giovanni Campagna 2014-03-05 19:00:33 UTC
The return value of meta_window_get_startup_id (), which is used to apply workspace and timestamp from startup notification, in all gtk+ apps
Comment 5 Giovanni Campagna 2014-03-24 09:34:33 UTC
Can we get this in for 3.12.1?
Comment 6 Jasper St. Pierre (not reading bugmail) 2014-03-24 15:46:05 UTC
Review of attachment 268106 [details] [review]:

::: src/core/group.c
@@ +175,3 @@
+              event_mask = ColormapChangeMask;
+              if (ancestor->override_redirect)
+                event_mask |= StructureNotifyMask;

I'd rather see something like this above in meta_group_new:

    gulong event_mask = 0;

    if (XGetWindowAttributes (xdisplay, group_leader, &attr))
      event_mask |= attr.your_event_mask;

    /* Make sure to listen for PropertyChange events, since
     * we need to detect changes in _NET_STARTUP_ID on the
     * group leader... */
    event_mask |= PropertyChangeMask;

::: src/core/screen.c
@@ +3320,2 @@
   sequence = NULL;
+  if (startup_id != NULL)

This seems completely unrelated to "group: select for property notifications on the leader window". If you aren't going to split it out, it needs explaining in the commit message.
Comment 7 Jasper St. Pierre (not reading bugmail) 2015-06-28 20:27:52 UTC
Pushed a simpler patch:

https://git.gnome.org/browse/mutter/commit/?id=aea71fbd0119d31a69787985beb77bfac8842e93

GTK+ is still incorrectly doing startup-notification though.