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 684741 - Can't unminimize iconified window on first attempt
Can't unminimize iconified window on first attempt
Status: RESOLVED FIXED
Product: metacity
Classification: Other
Component: general
trunk
Other Linux
: Normal normal
: ---
Assigned To: Metacity maintainers list
Metacity maintainers list
Depends on:
Blocks:
 
 
Reported: 2012-09-24 21:30 UTC by Daniel Drake
Modified: 2012-10-12 15:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (1.38 KB, patch)
2012-09-24 21:30 UTC, Daniel Drake
needs-work Details | Review
updated patch (1.51 KB, patch)
2012-10-07 21:36 UTC, Daniel Drake
committed Details | Review

Description Daniel Drake 2012-09-24 21:30:26 UTC
Created attachment 225108 [details] [review]
patch

Recent GTK versions have started using _NET_WM_STATE_HIDDEN:

http://git.gnome.org/browse/gtk+/commit/?h=gtk-2-24&id=3f6592f60fd15fb353fc84600caefba3054dc892

(same thing was also applied to GTK3)

This has triggered a metacity bug w.r.t. _NET_WM_STATE_HIDDEN which can be seen by running this simple test program: http://dev.laptop.org/~dsd/20120920/iconify.py

The test creates a window that is iconified at the start. It appears in the gnome-panel window list, but if you click on it there, the window doesn't unmaximize. You have to click again, and then finally it appears.

I reported this to the GTK+ list, but it was pointed out that openbox behaves OK here, and I have confirmed this. https://mail.gnome.org/archives/gtk-devel-list/2012-September/msg00042.html

This is causing problems with Sugar's Journal: the user has to click on the icon twice for it to appear on-screen.

Looking at the metacity code:

The _NET_ACTIVE_WINDOW request causes the window to be unmimized then shown. However, immediately after, we end up in meta_window_constrain() which calls place_window_if_needed() which has this code:

      if (window->minimize_after_placement)
        {
          meta_window_minimize (window);
          window->minimize_after_placement = FALSE;
        }

This flag only ever gets set in 1 place: reload_net_wm_state()

      else if (value->v.atom_list.atoms[i] == window->display->atom__NET_WM_STATE_HIDDEN)
        window->minimize_after_placement = TRUE;

so now we have the link back to why _NET_WM_STATE_HIDDEN has triggered this bug.

I guess the code that sets and deals with minimize_after_placement is still true in other situations (it was introduced in commit 07e4cacf14874746453c1c4818122e899bdc4159) but in this case its clearly not behaving quite right.

I think the solution is to simply unset this flag when we're asked to unminimize a window. Patch attached. I can commit if this passes review.
Comment 1 Milan Bouchet-Valat 2012-09-25 08:06:31 UTC
If this also applies to Mutter (which is likely), you'd better open a bug there too. As it's more actively maintained, you will get a review quicker...
Comment 2 Daniel Drake 2012-09-25 14:29:19 UTC
Can't reproduce this under mutter/gnome-shell.
Comment 3 Florian Müllner 2012-10-06 16:18:07 UTC
Review of attachment 225108 [details] [review]:

I think the patch is a hack. The problem at hand is caused by this:

 (1) minimize_after_placement is reset after placing the window
 (2) if a window is minimized, placement is deferred

Resetting minimize_after_placement unconditionally in place_window_if_needed() should work as well, and makes more sense IMHO:

diff --git a/src/core/constraints.c b/src/core/constraints.c
index c28fc0d..ecf05d3 100644
--- a/src/core/constraints.c
+++ b/src/core/constraints.c
@@ -570,11 +570,10 @@ place_window_if_needed(MetaWindow     *window,
           window->maximize_vertically_after_placement = FALSE;
         }
       if (window->minimize_after_placement)
-        {
-          meta_window_minimize (window);
-          window->minimize_after_placement = FALSE;
-        }
+        meta_window_minimize (window);
     }
+
+  window->minimize_after_placement = FALSE;
 }
Comment 4 Florian Müllner 2012-10-06 16:22:27 UTC
(In reply to comment #2)
> Can't reproduce this under mutter/gnome-shell.

Conceptually the issue applies to mutter as well, but it is not a problem in practice as placement of minimized windows is not deferred (which is necessary because minimized windows are shown in the shell's overview).

It's probably still good to apply the change anyway.
Comment 5 Daniel Drake 2012-10-07 21:36:32 UTC
Created attachment 226007 [details] [review]
updated patch

Thanks for your input - I agree.

I've tested your approach, and it is working. Here it is as a patch.
Comment 6 Florian Müllner 2012-10-12 07:58:57 UTC
Let's consider your comment as review then and push this ...

Next week when rolling tarballs for GNOME 3.6.1, I'll do a metacity release as well to get the fix out.
Comment 7 Daniel Drake 2012-10-12 15:10:55 UTC
Thanks!