GNOME Bugzilla – Bug 630363
No dimming with Xchat quit confirmation dialog
Last modified: 2010-09-30 21:09:08 UTC
Open Xchat and connect to at least one IRC network. Hit ctrl+q to quit, that will open a modal dialog asking for confirmation. The modal dialog is like « attached to the title bar », which is expected, but the main window, behind, is not dimmed at all. Also, with other applications, modal dialogs appear with a subtle animation, like they are « getting out of the title bar ». This doesn't happen with the exit confirmation in Xchat.
By the way, this is with a current jhbuild build of gnome-shell, and xchat-2.8.8-2.fc13.x86_64
Created attachment 170900 [details] [review] sync window_type between MutterWindow and MetaWindow
Created attachment 170901 [details] [review] [windowDimmer] handle changing of window_type dim/undim parent window when window_type changed to/from MODAL_DIALOG
Review of attachment 170900 [details] [review]: What I'd rather see is just removing window_type from MutterWindow. There's no reason for the duplication. Adel has a patch to do that written, so rejecting this patch.
Review of attachment 170901 [details] [review]: Will need rebasing once MutterWindow.window_type is removed.
Created attachment 170971 [details] [review] mutter-window: Don't track the window type twice Currently mutter-window has its own type field, even though the same information is already present in meta_window.
Created attachment 170972 [details] [review] Don't use mutter_window_get_window_type It has been removed so we have to use meta_window_get_window_type() instead.
Created attachment 170980 [details] [review] Don't use mutter_window_get_window_type It has been removed so we have to use meta_window_get_window_type() instead. *) Remove useless blank line
Review of attachment 170980 [details] [review]: In my opinion, It is better to use meta_window property. e.g. actor.meta_window.get_window_type()
(In reply to comment #9) > Review of attachment 170980 [details] [review]: > > In my opinion, It is better to use meta_window property. > e.g. actor.meta_window.get_window_type() Makes sense.
Created attachment 171011 [details] [review] Don't use mutter_window_get_window_type It has been removed so we have to use meta_window_get_window_type() instead.
Created attachment 171042 [details] [review] [windowDimmer] handle changing of window_type (depend on 2 patches from this bug)
Review of attachment 170971 [details] [review]: ::: src/compositor/mutter-window.c @@ +556,3 @@ { MutterWindowPrivate * priv = self->priv; + MetaCompWindowType window_type = meta_window_get_window_type (priv->window); We should get rid of MetaCompWindowType and just use MetaWindowType, right?
Review of attachment 171011 [details] [review]: Needs adjustment for MetaCompWindowType removal as well.
Review of attachment 171042 [details] [review]: I'm OK with the approach of actor._windowType and actor._notifyWindowTypeSignalId, but in your notification function for window-type you are duplicating significant amounts of code from elsewhere in the file - needs refactoring to avoid that. ::: js/ui/windowManager.js @@ +282,3 @@ + if (this._shouldAnimate()) + this._dimParentWindow(actor, true); + } else if (actor._windowType == Meta.CompWindowType.MODAL_DIALOG) { Will need updating to Meta.WindowType
Created attachment 171362 [details] [review] mutter-window: Don't track the window type twice Currently mutter-window has its own type field, even though the same information is already present in meta_window. And while at it get rid of MetaCompWindowType, it is equally redundant.
Created attachment 171363 [details] [review] Don't use mutter_window_get_window_type / MetaCompWindowType They have been removed so we have to use meta_window_get_window_type() and MetaWindowType instead.
Created attachment 171381 [details] [review] [windowDimmer] handle changing of window_type
Review of attachment 171363 [details] [review]: ::: js/ui/windowManager.js @@ +226,3 @@ var count = 0; parent.foreach_transient(function(win) { + if (win.meta_window.get_window_type() == Meta.WindowType.MODAL_DIALOG && win != self) win is MetaWindow.
Created attachment 171408 [details] [review] Don't use mutter_window_get_window_type / MetaCompWindowType They have been removed so we have to use meta_window_get_window_type() and MetaWindowType instead.
Review of attachment 171362 [details] [review]: One minor cleanup needed, one extra space ,otherwise OK to commit. ::: src/compositor/compositor.c @@ -153,2 @@ else if (event->atom == meta_display_get_atom (display, META_ATOM__NET_WM_WINDOW_TYPE)) This whole if should be removed ::: src/compositor/plugins/default.c @@ +585,3 @@ gint end_x, gint end_y, gint end_width, gint end_height) { + MetaWindow *meta_window = mutter_window_get_meta_window (mc_window); extra space in 'MetaWindow *'
Review of attachment 171408 [details] [review]: Looks fine
Review of attachment 171381 [details] [review]: ::: js/ui/windowManager.js @@ +233,3 @@ }, + _markParentWindowAsDimAble: function(actor, animate) { Dimmable not DimAble Other than that, looks reasonable... definitely better than the last version. I'm not sure it handles every change on the fly - e.g., it would be possible for the parent window to change on the fly - but I don't think it's worth a lot of effort to try to catch corner cases for that until/unless we hit problems.
Comment on attachment 171362 [details] [review] mutter-window: Don't track the window type twice Attachment 171362 [details] pushed as f2ccf70 - mutter-window: Don't track the window type twice
Comment on attachment 171408 [details] [review] Don't use mutter_window_get_window_type / MetaCompWindowType Attachment 171408 [details] pushed as 61c9068 - Don't use mutter_window_get_window_type / MetaCompWindowType