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 755606 - Setting a parent/transient_for should affect the stack immediately
Setting a parent/transient_for should affect the stack immediately
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2015-09-25 00:41 UTC by Jonas Ådahl
Modified: 2015-12-23 07:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
window: Update the stack after setting the transient_for field (1.45 KB, patch)
2015-09-25 00:41 UTC, Jonas Ådahl
committed Details | Review
tests: Add test for testing that setting a parent affects the stack (3.49 KB, patch)
2015-09-25 00:41 UTC, Jonas Ådahl
committed Details | Review

Description Jonas Ådahl 2015-09-25 00:41:38 UTC
Making a MetaWindow a parent of another MetaWindow by setting the 'transient_for' state only affects the stacking order at an arbitrary later point after the state was set (whenever the stacking constraints were processed after the connection was made). These patches fixes it and adds a test to tests/stacking/.
Comment 1 Jonas Ådahl 2015-09-25 00:41:42 UTC
Created attachment 312099 [details] [review]
window: Update the stack after setting the transient_for field

Don't update the stack until after setting the window->transient_for
field. Updating before will cause the stack transient-for constraint to
be missing until the next time constraints are applied.
Comment 2 Jonas Ådahl 2015-09-25 00:41:47 UTC
Created attachment 312100 [details] [review]
tests: Add test for testing that setting a parent affects the stack

A new test is added that tests that xdg_surface.set_parent (referred to
as "transient for" in X11 terminology) affects the stack immediately.
Comment 3 Jonas Ådahl 2015-12-03 08:18:11 UTC
Ping.
Comment 4 Olivier Fourdan 2015-12-08 15:42:17 UTC
We might need this fix for bug 759161 as well.
Comment 5 Rui Matos 2015-12-17 17:24:37 UTC
Review of attachment 312099 [details] [review]:

Seems obviously correct. But I'm curious, did you find a case where this was a problem in practice? What was it?
Comment 6 Rui Matos 2015-12-17 17:31:43 UTC
Review of attachment 312100 [details] [review]:

looks fine

::: src/tests/test-client.c
@@ +141,3 @@
+      if (argc != 3)
+        {
+          g_print ("usage: menu <window-id> <parent-id>");

this should probably be "set_parent <client-id>/<window-id> <parent-id>"
Comment 7 Jonas Ådahl 2015-12-22 10:45:03 UTC
(In reply to Rui Matos from comment #5)
> Review of attachment 312099 [details] [review] [review]:
> 
> Seems obviously correct. But I'm curious, did you find a case where this was
> a problem in practice? What was it?

If you have a existing window that is made a child window of some other window it wouldn't take effect until some other window stacking operation was done. The client I used where I hit this was my xdg-app portal-on-wayland proof of concept (https://github.com/jadahl/gtkforeign/ ).
Comment 8 Olivier Fourdan 2015-12-22 11:49:51 UTC
... and gtk itself as well as this is required for bug 759161
Comment 9 Jonas Ådahl 2015-12-23 06:57:02 UTC
(In reply to Rui Matos from comment #6)
> Review of attachment 312100 [details] [review] [review]:
> 
> looks fine
> 
> ::: src/tests/test-client.c
> @@ +141,3 @@
> +      if (argc != 3)
> +        {
> +          g_print ("usage: menu <window-id> <parent-id>");
> 
> this should probably be "set_parent <client-id>/<window-id> <parent-id>"

This is the usage of the test-client, which does not have the concept of "client id" since it is the client.
Comment 10 Jonas Ådahl 2015-12-23 07:32:38 UTC
Attachment 312099 [details] pushed as 213f0fa - window: Update the stack after setting the transient_for field
Attachment 312100 [details] pushed as 75b992c - tests: Add test for testing that setting a parent affects the stack