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 461927 - windows sometimes self-maximise
windows sometimes self-maximise
Product: metacity
Classification: Other
Component: general
Other Linux
: Normal normal
: ---
Assigned To: Metacity maintainers list
Metacity maintainers list
: 485472 493399 502273 554104 560142 588679 (view as bug list)
Depends on:
Reported: 2007-07-30 19:46 UTC by Christian Persch
Modified: 2010-01-20 20:43 UTC
See Also:
GNOME target: ---
GNOME version: ---

proposed patch to svn (532 bytes, patch)
2007-11-06 00:13 UTC, Peter Bloomfield
accepted-commit_now Details | Review
revised patch to svn (816 bytes, patch)
2007-11-06 05:13 UTC, Peter Bloomfield
committed Details | Review
proposed patch to current svn (2.32 KB, patch)
2007-11-10 01:03 UTC, Peter Bloomfield
committed Details | Review
patch against current svn (1.66 KB, patch)
2008-09-23 02:43 UTC, Peter Bloomfield
none Details | Review
Revised patch (3.20 KB, patch)
2008-09-23 10:37 UTC, Peter Bloomfield
none Details | Review
revised patch (4.68 KB, patch)
2008-09-25 13:47 UTC, Peter Bloomfield
none Details | Review
testcase (2.12 KB, text/plain)
2008-12-16 21:13 UTC, Peter Bloomfield
extended patch (5.66 KB, patch)
2009-04-16 22:59 UTC, Peter Bloomfield
none Details | Review
fullscreen testcase (2.89 KB, text/plain)
2009-04-17 11:54 UTC, Peter Bloomfield

Description Christian Persch 2007-07-30 19:46:48 UTC
Steps to repro:
0) Start evince with some document
1) Alt-F10 to maximise
2) Close evince
3) Start evince with same doc again (the window is now maximised)
4) Alt-F5 to demaximise
5) Ctrl-F to show the find bar

Window maximises itself (although it isn't in maximised state, it just occupies the whole screen).

Alternative steps:
0) Start "epiphany -p" (private instance). The windows it shows is maximised.
1) Alt-F5
2) Ctrl-L, type URL, enter

Window self-maximises.

This is with metacity 2.19.34.
Comment 1 Jan Niklas Hasse (Account disabled) 2007-09-22 12:47:25 UTC
it happens with alt+f5
Comment 2 Peter Bloomfield 2007-11-05 11:46:25 UTC
*** Bug 493399 has been marked as a duplicate of this bug. ***
Comment 3 Peter Bloomfield 2007-11-06 00:13:56 UTC
Created attachment 98593 [details] [review]
proposed patch to svn

A quick look at the (well-documented!) Metacity code suggests that window->user_rect shouldn't be updated if the window is currently maximized, and I verified that in fact it is.  The attached patch fixed the problem for the test program at Bug 493399.
Comment 4 Elijah Newren 2007-11-06 02:18:40 UTC
You rock.  Thanks for tracking this down.  :-)
Comment 5 Elijah Newren 2007-11-06 04:38:10 UTC
Hmm...we should probably tweak it a bit to handle unidirectional (vertical or horizontal) maximization, only recording the bit of the window position corresponding to the unmaximized direction(s).  Go ahead and commit, but keep this bug open.
Comment 6 Peter Bloomfield 2007-11-06 05:13:37 UTC
Created attachment 98623 [details] [review]
revised patch to svn

You mean like this?
Comment 7 Peter Bloomfield 2007-11-06 13:26:36 UTC
OK--first patch committed.
Comment 8 Elijah Newren 2007-11-07 03:56:43 UTC
Yeah, like that.  :-)

Comment 9 Peter Bloomfield 2007-11-07 04:29:43 UTC
OK--second patch committed!
Comment 10 Elijah Newren 2007-11-08 01:46:48 UTC
The appropriate status is not 'none' (which means unreviewed), rather it is 'committed'.  ;-)

And I just had another thought...will we have a similar bug with the window being fullscreened (i.e. do we need to avoid setting user_rect if the window is in fullscreen mode)?  And do we have other calls to setting user_rect that should have the same kind of changes?  Maybe we should make a function somewhere and have all the relevant places call it...
Comment 11 Peter Bloomfield 2007-11-08 16:39:47 UTC
Sorry about the status--I reloaded an old page, and I didn't notice that status didn't get updated--must file a bug against Epiphany about that :)

Re fullscreen: the only reports of the current bug are for a program that starts out maximized, presumably because it saved its previous state.  In fact,

if (is_user_action || (!window->placed && !META_WINDOW_MAXIMIZED (window)))
     meta_window_get_client_root_coords (window, &window->user_rect);

would probably have been enough to fix it, as reported.  I can't imagine why an app would be intially fullscreen--in fact, I'd say that would be downright user-unfriendly!  Going to fullscreen and back after starting the app does /not/ trigger the bug for me.

If it ain't broke...
Comment 12 Christian Persch 2007-11-08 17:25:47 UTC
(In reply to comment #11)
> I can't imagine why an
> app would be intially fullscreen

It would do that because it saved the state on exit and restores it on startup (e.g. session restore).
Comment 13 Christian Persch 2007-11-08 17:27:54 UTC
Actually, might that be the cause of bug 449892 ?
Comment 14 Peter Bloomfield 2007-11-10 01:03:28 UTC
Created attachment 98853 [details] [review]
proposed patch to current svn

An app that starts out fullscreen does indeed trigger this bug, if unfullscreened and then redrawn--and Christian makes a persuasive case that it matters!  This proposed patch fixes it, and as Elijah suggests (Comment #10) fixes the one other place where user_rect is updated, with a new function (meta_window_save_user_rect).

It's not clear that the other place where user_rect is updated needs to be protected against fullscreen and maximize, but it seems like good defensive coding.

The new function meta_window_save_user_rect has very similar logic to the existing meta_window_save_rect, but I don't see how to eliminate the redundancy.
Comment 15 Peter Bloomfield 2007-11-10 01:07:42 UTC
BTW: the patch doesn't fix the problems in Bug #449892--neither the excessive window-state events reported by Bastien, nor the bad configure event reported by Christian.  Showing the window before fullscreening it eliminates both, but it's a poor work-around.
Comment 16 Peter Bloomfield 2007-11-12 22:35:21 UTC
Re Comment #13: part of the reason for the bad configure event is that in meta_window_new_with_attrs(), Metacity calls meta_window_move_resize_internal, which in turn calls XConfigureWindow with the dimensions, before set_net_wm_state, which in turn calls XChangeProperty with details such as fullscreen status.  In consequence, a GdkEvent with the size is called before the one with the status.

It wouldn't be a problem if the app already knew that the window was fullscreen, but the documented sequence of GdkEvents shows that the app has received a bogus unfullscreen event, hence the misinterpretation of the size.  So the root cause of the problem is the bogus unfullscreen event.

However, arguably Metacity shouldn't depend on the app knowing the status of the window, and should make sure to advertize the status to X before advertizing the size.  A quick test suggests that moving the calls to set_wm_state and set_net_wm_state before the call to meta_window_move_resize_internal fixes the problem without immediately breaking anything else.  I have no idea whether it causes any non-immediate breakage!
Comment 17 Thomas Thurman 2007-11-13 04:45:40 UTC
(In reply to comment #14)
> The new function meta_window_save_user_rect has very similar logic to the
> existing meta_window_save_rect, but I don't see how to eliminate the
> redundancy.

To be honest, I think you'd tie yourself in knots doing so-- and it's not like the alleged duplication is something you've added. The patch all seems like a sane idea and very much in order, and Metacity works fine with it applied, although I haven't tested anything in full screen mode with and without the patch, since I'm not aware of a good way to test the problem.
Comment 18 Peter Bloomfield 2007-11-13 13:37:02 UTC
OK--committed almost as is: the comment about what meta_window_save_user_rect does is now in the function instead of where it is called.
Comment 19 Elijah Newren 2007-11-24 15:50:33 UTC
*** Bug 485472 has been marked as a duplicate of this bug. ***
Comment 20 Christian Persch 2007-12-07 11:44:15 UTC
*** Bug 502273 has been marked as a duplicate of this bug. ***
Comment 21 Andreas Røsdal 2008-01-06 20:46:05 UTC
*** Bug 507293 has been marked as a duplicate of this bug. ***
Comment 22 Christian Persch 2008-09-21 12:03:42 UTC
Is this bug supposed to have been fixed by the committed patch? I can still repro with the steps from comment 0 in version 2.23.610 .
Comment 23 Peter Bloomfield 2008-09-21 17:02:04 UTC
Seems to be OK in metacity-2.22.0-3.fc9.x86_64.
Comment 24 Peter Bloomfield 2008-09-21 21:48:51 UTC
...but you're right, it's back in metacity-2.23.610-1.fc10.i386!  Also current snv :(

Seems to be a regression...
Comment 25 Peter Bloomfield 2008-09-22 22:19:10 UTC
This hack allows the test program at Bug 493399 to work--I have not verified with evince, as apparently it no longer remembers the maximized state.  Windows that are initially fullscreen were also a problem in the earlier instances of this bug--I have not explored those at all.

--- src/core/window.c	(revision 3914)
+++ src/core/window.c	(working copy)
@@ -2520,6 +2520,12 @@
   window->maximized_vertically = 
     window->maximized_vertically   || maximize_vertically;
+  /* Hack: latest fix for #461927: if the window is initially maximized,
+   * set window->placed, so that later we do not
+   * force_save_user_window_placement (window); */
+  if (maximize_horizontally || maximize_vertically)
+    window->placed = TRUE;
   /* Fix for #336850: If the frame shape isn't reapplied, it is
    * possible that the frame will retains its rounded corners. That
    * happens if the client's size when maximized equals the unmaximized
Comment 26 Peter Bloomfield 2008-09-23 02:43:54 UTC
Created attachment 119204 [details] [review]
patch against current svn

More conservative patch.
Comment 27 Peter Bloomfield 2008-09-23 10:37:15 UTC
Created attachment 119214 [details] [review]
Revised patch

That patch was not quite right: the unmaximized window did not resize to fill the screen, but it jumped to the top left corner.  This one does better.
Comment 28 Peter Bloomfield 2008-09-25 13:47:38 UTC
Created attachment 119369 [details] [review]
revised patch

The issue with a window that's initially fullscreen has also been reintroduced in 2.23, and in current svn.  The attached revised patch fixes it: after un-fullscreening, the window no longer self-maximizes.

One piece of nastiness remains: after un-fullscreening, the window is placed at the top left of the screen, instead of where it would have been placed if not initially fullscreen.  But that was also true back in the days of 2.22, before the bug was reintroduced, so I guess it's acceptable.
Comment 29 Peter Bloomfield 2008-09-27 15:56:52 UTC
(In reply to comment #28)
> One piece of nastiness remains: after un-fullscreening, the window is placed at
> the top left of the screen, instead of where it would have been placed if not
> initially fullscreen.  But that was also true back in the days of 2.22, before
> the bug was reintroduced, so I guess it's acceptable.

This seems to be a feature;)  place_window_if_needed has a section documented as

          /* define a sane saved_rect so that the user can unmaximize to
           * something reasonable.

and it works: after unmaximizing, the window is placed like any new window of its size.  But nothing seems to be done to permit un-fullscreening to something reasonable, hence the nastiness: the un-fullscreened window overlaps an existing window even though enough screen is empty to have placed it there.
Comment 30 Federico Mena Quintero 2008-12-16 17:47:56 UTC
This is really bad when you have multiple monitors:

1. Start with two monitors, laid out left-to-right.

2. Start an Epiphany window in the rightmost monitor and maximize it.

3. Hit C-n to open a new Epiphany window; it should appear maximized over the first one.

4. Type a URL in the location bar and hit Enter.

5. The window from (3) moves to the *leftmost* monitor.

This is pretty annoying; windows should not change monitors out of the blue :)
Comment 31 Federico Mena Quintero 2008-12-16 18:25:52 UTC
Bleh, I had this happening with 2.23.x, but now it doesn't happen with 2.24.0.
Comment 32 Peter Bloomfield 2008-12-16 21:13:46 UTC
Created attachment 124827 [details]

This test program still self-maximizes with metacity-2.24.0-2.fc10.i386.
Comment 33 Christian Persch 2009-01-15 21:21:17 UTC
*** Bug 560142 has been marked as a duplicate of this bug. ***
Comment 34 Christian Persch 2009-04-15 20:19:44 UTC

This is still reproducible in metacity trunk, with the testcase above (click on the left button after de-maximising). The patch attached above fixes the issue. Any chance of getting the patch committed for trunk and 2.26 ?
Comment 35 Peter Bloomfield 2009-04-16 22:59:22 UTC
Created attachment 132798 [details] [review]
extended patch

Hi Christian:

Yes, this keeps nagging at me, too.  On the off chance that a patch gets considered, here's one that will also give a sane placement for a window that's initially full-screen and is then un-full-screened.
Comment 36 Peter Bloomfield 2009-04-16 23:07:58 UTC
But that last patch is a little more intrusive, and might be frowned upon as a result--I'd be glad to see either committed!
Comment 37 Peter Bloomfield 2009-04-17 11:54:45 UTC
Created attachment 132820 [details]
fullscreen testcase

This testcase illustrates the full-screen issue.  Start it with no argument, and it is initially full-screen.  Un-full-screen it, and it is placed at the top left of the screen, obscuring any window in its way.  Click the "short label" button, and it self-full-screens.

For comparison, start it with any argument, and it is initially un-full-screen, and placed sanely.  You can full-screen it, un-full-screen it, click buttons to add labels, and it all works the way you'd expect.  With the most recent patch, the same is true if it is started full-screen.
Comment 38 Christian Persch 2009-07-19 07:52:34 UTC
*** Bug 588679 has been marked as a duplicate of this bug. ***
Comment 39 Christian Persch 2009-09-12 22:09:56 UTC
Ping? Any chance of getting the fix in for 2.28.0 or .1 ?
Comment 40 Peter Bloomfield 2009-09-17 00:53:50 UTC
Maximize and full-screen testcases both still exhibit the issue with metacity-2.26.0-1.fc11.i586, and most recent patch applies successfully to current git master and fixes both issues.
Comment 41 Thomas Thurman 2010-01-20 16:00:10 UTC
Committed with thanks; sorry for the delay.  The testcase was very helpful.
Comment 42 Peter Bloomfield 2010-01-20 20:43:42 UTC
*** Bug 554104 has been marked as a duplicate of this bug. ***