it happens with alt+f5

*** Bug 493399 has been marked as a duplicate of this bug. ***

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.

You rock.  Thanks for tracking this down.  :-)

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.

Created attachment 98623 [details] [review]
revised patch to svn

You mean like this?

OK--first patch committed.

Yeah, like that.  :-)

Thanks!

OK--second patch committed!

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...

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...

(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).

Actually, might that be the cause of bug 449892 ?

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.

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.

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!

(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.

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.

*** Bug 485472 has been marked as a duplicate of this bug. ***

*** Bug 502273 has been marked as a duplicate of this bug. ***

*** Bug 507293 has been marked as a duplicate of this bug. ***

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 .

Seems to be OK in metacity-2.22.0-3.fc9.x86_64.

...but you're right, it's back in metacity-2.23.610-1.fc10.i386!  Also current snv :(

Seems to be a regression...

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

Created attachment 119204 [details] [review]
patch against current svn

More conservative patch.

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.

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.

(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.

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 :)

Bleh, I had this happening with 2.23.x, but now it doesn't happen with 2.24.0.

Created attachment 124827 [details]
testcase

This test program still self-maximizes with metacity-2.24.0-2.fc10.i386.

*** Bug 560142 has been marked as a duplicate of this bug. ***

Ping? 

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 ?

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.

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!

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.

*** Bug 588679 has been marked as a duplicate of this bug. ***

Ping? Any chance of getting the fix in for 2.28.0 or .1 ?

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.

Committed with thanks; sorry for the delay.  The testcase was very helpful.

*** Bug 554104 has been marked as a duplicate of this bug. ***