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 629560 - Destroy effect of modal windows is odd when the parent has gone away
Destroy effect of modal windows is odd when the parent has gone away
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Florian Müllner
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2010-09-13 18:56 UTC by Florian Müllner
Modified: 2010-09-15 11:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
cancel Destroy effect of modal windows when the parent has gone away (3.33 KB, patch)
2010-09-14 08:56 UTC, Maxim Ermilov
reviewed Details | Review
cancel Destroy effect of modal windows when the parent has gone away (3.07 KB, patch)
2010-09-15 07:32 UTC, Maxim Ermilov
committed Details | Review

Description Florian Müllner 2010-09-13 18:56:46 UTC
The shrink animation of (attached) modal dialogs looks odd if the parent window has been closed - the window now "rolls up" into empty space.

To reproduce:

1. open gedit
2. type something
3. close gedit
4. select "Close without saving" in the modal popup

I guess we just want to cancel the effect when the parent is destroyed.
Comment 1 Maxim Ermilov 2010-09-14 08:56:03 UTC
Created attachment 170225 [details] [review]
cancel Destroy effect of modal windows when the parent has gone away
Comment 2 Florian Müllner 2010-09-14 21:17:27 UTC
Review of attachment 170225 [details] [review]:

Works like advertised, but some style comments below:

::: js/ui/windowManager.js
@@ +365,3 @@
+            let signalId = parent.connect('unmanaged', Lang.bind(this, function () {
+                Tweener.removeTweens(actor);
+                this._destroyWindowDone(shellwm, actor, signalId);

I think it is weird to pass the signal id around like this - can't you monkey patch the actor?

Something along the lines of
actor._parentDestroyId = parent.connect(...)

@@ +385,3 @@
 
+    _destroyWindowDone : function(shellwm, actor, signalId) {
+        if (actor && actor.get_window_type() == Meta.CompWindowType.MODAL_DIALOG)

You removed the test for parent here? I would have expected something like

let parent = actor.get_meta_window().get_transient_for();
if (parent)
    parent.disconnect(signalId);
if (actor && actor.get_window_type() == Meta.CompWindowType.MODAL_DIALOG && parent) {
    // what you had before
}
Comment 3 Maxim Ermilov 2010-09-15 07:32:14 UTC
Created attachment 170317 [details] [review]
cancel Destroy effect of modal windows when the parent has gone away

> I think it is weird to pass the signal id around like this - can't you monkey
> patch the actor?
done

> You removed the test for parent here?
I remove this test at all (if effect was started need complete it)
Also I remove _destroyWindowOverwrite (it doing same thing _destroyWindowDone).
Comment 4 Florian Müllner 2010-09-15 11:27:36 UTC
Review of attachment 170317 [details] [review]:

Code looks good, some nitpicking about the commit message:

- cancel => Cancel (not sure about Destroy => destroy)
- missing body, e.g.
  "Modal dialogs slide back into the titlebar of the parent window when destroyed.
   This looks weird if the parent window itself has been destroyed, so cancel the 
   effect in this case."

Good to commit with this change.