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 770324 - window: Don't unmanage transient_for when attached
window: Don't unmanage transient_for when attached
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: wayland
unspecified
Other All
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2016-08-24 10:05 UTC by Jonas Ådahl
Modified: 2016-08-26 02:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
window: Don't unmanage transient_for when attached (2.64 KB, patch)
2016-08-24 10:05 UTC, Jonas Ådahl
committed Details | Review

Description Jonas Ådahl 2016-08-24 10:05:38 UTC
Had a strange issue where a GTK+ client would create a dialog, but it never showed up. The client appeared frozen, except that it still responded to ping and changed the pointer cursor depending on what the pointer cursor was over.

The issue was that the client first set dialog as modal, and then made it a
transient for the parent. This caused mutter to trigger some path that depends
on side effects I could not understand. It would be good to get rid of that
side-effect dependency (the comment doesn't help much, and I did some quick git
history research but didn't see what would trigger the apparent window
recreation). Either way, said side effect has no chance of working on Wayland,
thus the whole thing recreating the window is simply made X11-only.
Comment 1 Jonas Ådahl 2016-08-24 10:05:43 UTC
Created attachment 334067 [details] [review]
window: Don't unmanage transient_for when attached

For some reason, when a modal dialog was made an attaching
transient-for, if the window wasn't "constructing", it would be
unmanaged and rely on some side effect to be recreated. This side
effect is not triggered for Wayland clients, thus if one happen to set
a surface as "modal" via gtk_surface.set_modal before
xdg_toplevel.set_parent, it'd be unmanaged and never show up.

Instead, simply just set the tranciency anyway for Wayland clients.
This makes GTK+ clients that set_modal() before set_transient_for()
work.
Comment 2 Rui Matos 2016-08-24 15:30:12 UTC
(In reply to Jonas Ådahl from comment #0)
> The issue was that the client first set dialog as modal, and then made it a
> transient for the parent. This caused mutter to trigger some path that
> depends
> on side effects I could not understand. It would be good to get rid of that
> side-effect dependency (the comment doesn't help much, and I did some quick
> git
> history research but didn't see what would trigger the apparent window
> recreation).

This is what triggers it:

https://git.gnome.org/browse/mutter/tree/src/x11/window-x11.c#n605

but yeah, I'm not sure *why* we do this, have you tried just removing that code and see if it works under X too ?
Comment 3 Jonas Ådahl 2016-08-25 04:52:05 UTC
(In reply to Rui Matos from comment #2)
> (In reply to Jonas Ådahl from comment #0)
> > The issue was that the client first set dialog as modal, and then made it a
> > transient for the parent. This caused mutter to trigger some path that
> > depends
> > on side effects I could not understand. It would be good to get rid of that
> > side-effect dependency (the comment doesn't help much, and I did some quick
> > git
> > history research but didn't see what would trigger the apparent window
> > recreation).
> 
> This is what triggers it:
> 
> https://git.gnome.org/browse/mutter/tree/src/x11/window-x11.c#n605

I see.

> 
> but yeah, I'm not sure *why* we do this, have you tried just removing that
> code and see if it works under X too ?

I haven't because I don't have a test case for it. The case where it triggered on Wayland didn't trigger when using the X11 backend, since at the time this call happened it was still "constructing".
Comment 4 Rui Matos 2016-08-25 14:18:39 UTC
Review of attachment 334067 [details] [review]:

well, let's go with it
Comment 5 Jonas Ådahl 2016-08-26 02:25:34 UTC
Attachment 334067 [details] pushed as 658d97d - window: Don't unmanage transient_for when attached