Ubuntu bug on https://bugs.launchpad.net/metacity/+bug/133541

"Using keyboard shortcuts, I have set a key (F8 in my case) to make the window manager expand the current window to full *height*, leaving the width unchanged.

It recently stopped working nicely: now it expands windows so that the bottom of them is invisible below my lower Gnome panel.
The expand to full height and width function still works fine. The expand to full width function works fine, even if I have a Gnome panel on the side -- ie it expands so as not to overlap the panel. The expand to full height (the one that is broken) does not make an error concerning the top panel, only a bottom panel.

Correct behaviour would be for "Expand vertically" to expand to the full height minus the top and bottom panels."

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

I see this from time to time too. Restarting either gnome-panel or metacity resolves the problem temporary.

Anyone have any hints about how to duplicate?  Or was this fixed as a side-effect of fixing bug 461927?  I can't seem to duplicate in a few minutes of randomly using vertical & horizontal maximization keyboard shortcuts...

I'm finding it hard to duplicate since I last worked around it, which I did by moving the single panel I have on the screen from the top to the bottom and then back to the top.  After I did that, the problem went away.

Note that there is still another issue which is easy to duplicate:

* Open a new window
* Press the shortcut for Maximize Window (Vertically or Horizontally) twice
* Now press the shortcut for Maximize Window (Horizontally or Vertically, the opposite of whatever was chosen in the last step) once or twice to observe incorrect behaviour

In either case, the window will maximize in both directions when the shortcut to maximize the window's opposite dimension (i.e. horizontal instead of vertical) is pressed

I can duplicate the behavior in comment 5, but only with resize-increment windows (e.g. emacs, gnome-terminal).

If you can grab a fedora 8 live CD then you'll be able to easily duplicate this. It happens with all kinds of windows. FWIW, metacity's version is 2.20.0-3.fc8

It's reproduceable (sometimes) with epiphany as well here, dunno if this is a 'resize-increment' window. Hiding/showing again the panel using the (optional) masking buttons is enough to get the windows to get the maximization right.

The original bug (window expanding vertically below the bottom panel) isn't something I have been able to reproduce easily. It usually just start happening after I have been logged in for a couple of hours.

However, I have been running svn trunk (r3381) for close to 24 hours now, and haven't seen the bug, so it looks like it's gone.

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

I can confirm, that with current svn (r3421 here) this bug does not happen.

@Developers: Why isn't this very easily reproducible bug (see e.g. #496070 for the steps to reproduce it ) still not fixed in in a stable point release?

I mean this is a *simple* 100% reproducible regression bug. I understand that fixing it could take up to a week but >3 months? I am sorry but this really raises the question whether metacity still maintained...

Sven claimed that it was fixed, or at least not reproducible, so it rather dropped below the radar. We all have busy lives and a lot of software to maintain; if you'd like to produce a patch, it would be very welcome.

I think bugreports@nn7.de is refering to the behaviour in bug 496070, I was talking about the expanding-over-bottom-panel bug, not sure if they are the same?

Anyway, it doesn't seem to be gone. I'm using the iains-blingtastic-bucket-o-bling branch, which should match trunk 3425, and I get the bug again. Could of course also be something related to the compositor?

(In reply to comment #11)
> I mean this is a *simple* 100% reproducible regression bug. I understand that
> fixing it could take up to a week but >3 months? I am sorry but this really
> raises the question whether metacity still maintained...
 
Yeah, I wish I would have had time a few years ago for adding a status field to bugzilla for all products in order to be able to mark each as unmaintained/undermaintained/actively maintained, etc, with the values being determined by various stats with allowed short-term overrides.  Anyway, Metacity has been undermaintained at best for probably a year and a half and unmaintained altogether for a stretch of a month or two at a time.  Thomas and I get a spurt of time here and there, but we're buried in backlog.  I think things are looking up for both of us going forward.  We'll see...  (/me goes back to catching up on email)

Saying me too.  

Windows, when vertically maximised were always going below my bottom panel.  After dragging the panel to the side and back, window maximisation respects the panel's presence now.  

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

Created attachment 101530 [details]
me too

Sometimes it just works, sometimes it doesn't. Moving the bottom panel to the right and then to the bottom makes metacity aware of it and solves the problem.

I don't know if it makes it easier to track down, but I have been trying older metacity releases, and the expanding-over-bottom-panel bug seems to be present as far back as in 2.19.1.

If it's any help, I can try individual commits and see if I can track it down further.

Another data point about this bug:

when a window is in this vertical-maximized-state-under-the-panel if one clicks on the window border (left, right or bottom) with the left mouse button, instead of entering resize mode, the window menu is displayed.

FWIW this is happening consistently on fedora 8 with metacity-2.20.1-1.fc8.x86_64 either with terminal windows or normal windows (for instace epiphany).

http://bugs.debian.org/452139 looks like a similar issue.

I have done some more backtracking, and it looks like revision 3147 might be the culprit.

As the bug isn't easily reproducible I can't be entirely sure of course.

( http://svn.gnome.org/viewvc/metacity/trunk/src/?view=log&pathrev=3147 to save you all fighting the web interface )

My mistake, I have reproduced it with 3146 now. I will try to do more careful testing in the future.

My new guess is 3144.
http://svn.gnome.org/viewvc/metacity?view=revision&revision=3144

I have been running metacity with the first part of r3144 (http://bugzilla.gnome.org/attachment.cgi?id=84627&action=diff) for a while now with no problems with vertical maximization. 

I'm guessing the bug is somewhere in the second half, "Make the strut lists (stored in workspaces) record both the rectangle and the side that the strut is on. Lots of code cleanups relating to struts.", but it doesn't seem easy to track  down further.

Created attachment 110014 [details]
Screenshot of broken vertical maximize with r3696

I can still reproduce this with r3696. Sometimes moving the bottom panel to the side and back fixes it (temporarily), but sometimes it doesn't work.

I see this in the log when I maximize. I take it it's normal for the max size to be INT_MAX ?

WINDOW_OPS: Maximizing 0x12176ce (~/metacity) vertically
WINDOW_OPS: Window 0x12176ce (~/metacity) fullscreen = 0 not resizable, maximizable = 1 fullscreenable = 1 min size 65x42 max size 2147483647x2147483647
WINDOW_OPS: Window 0x12176ce (~/metacity) decorated = 1 border_only = 0 has_close = 1 has_minimize = 1 has_maximize = 1 has_move = 1 has_shade = 1 skip_taskbar = 0 skip_pager = 0

Related to comment #5 I notice that if you unmaximise a window,
subsequently doing a vertical maximise, also does a horizontal maximise.

I've spent a few days building a testing various versions.

I can reproduce the bug quite easily, all it takes is switching virtual desktops once or twice and vertically maxmising a terminal.

With revision 3144 I can reliably reproduce the bug.

With revision 3143 I can NOT reproduce the bug, I've been running for several hours and haven't hit it yet.

So I'd wager something in 3144 is the culprit. However given some of the other comments on the bug perhaps 3144 just made the bug easier to hit.

Me too, I'm bothered by this issue as well. On ubuntu hardy, metacity version 1:2.22.0-0ubuntu4.

This is fixed for me in ubuntu, or at least I almost never get it anymore... and I've been working with this a lot these last weeks.

I suspect theme issue? I'm using fedora's nodoka (in ubuntu) right now. Didn't happen to me with raleigh either.

Have the same issue here on Ubuntu Hardy. Tweaking the the panel (turning Autohide on and off for example) fixes the problem for a while, until it comes back again. Wasn't able to find how to reproduce it intentionally.

I've been running with 3143 all week and haven't hit the bug once, whereas with 3144 I can hit it in seconds. So AFAIC the bug is in 3144 - unfortunately it's not a small patch.

I just installed Nodoka, and can reproduce the bug with Ubuntu's metacity in under a minute. So perhaps there's some theme issue involved, but it certainly doesn't make the problem go away.

Anecdotic info: 
 - my top and bottom panels are 25px.
 - my window title font is dejavu sans bold (10pt)
 - screen dpi is 96

I guess that's all silly facts, still maybe a clue.

Created attachment 111378 [details] [review]
Patch to workaround bug in vertical maximisation

I've been running for several days with this patch applied and can no longer reproduce the bug. If I revert this patch I can reproduce within seconds.

The patch essentially reverts the first hunk of 3144 (http://svn.gnome.org/viewvc/metacity?view=revision&revision=3144) - the change to src/constraints.c (now src/core/constraints.c)

I was able to reproduce this bug yesterday, don't know how, but now it's gone again. Really weird. So discard my last comment.

I've been running  metacity with your patch applied for about 8 hours now, and so far so good. Without it, I was hit by the bug in less than 1 hour, and then it constantly reoccurred. 

Cool, sounds like we're on the right track then. If anyone's bored I guess the next thing is to try and understand the code affected by that patch.

Was going to post the same bug, and found a patch instead :-)

The patch above works for me. I had big problems with vertical maximizing, for some reason when doing a vertical maximixation of any window it got the height of half of the screen.

The patch above fixes this and reverts the behaviour to how it was, say, a year ago. However, there was still a small problem with it: vertically resizing any window which resizes in big steps (for example, gnome-terminal or the xfte editor) will forcedly set a unaligned vertical size (e.g. not divisible by vertical resizing steps), which usually produces a partial line filled with black in best case (gnome-terminal) and with garbage in worst case (xfte).

Same on OpenBSD-current 2.20.3.

Patch works. Sent to ports@openbsd.org for inclusion until fixed upstream.

Thanks for the data point.  I would include the patch straight off, but Michael says he doesn't know why it works, and I like to know why things work before I include them.  (I've just started a new job, but I'm hoping when my schedule shakes out a bit that I'll have more time for patch review; this is currently one of my top two bugs to work on.)

(I have to say, by the way, that everyone has been very helpful supplying information about revision numbers that broke and writing patches.  Thank you all!)

Could you also please look into aligning the vertical size according to window preferences as well? This bugs annoys me several years, and if it'll be fixed, this will be clearly a progress, not just a bug fix :-)

Andrew: I don't understand comment 44 very well, but this is a bug for fixing this one particular problem; it's bad practice to mix multiple issues in the same bug.  If you want your alignment issue added, please raise it as a separate bug.

ok, I've filled bug #542373

I confirm this bug on gentoo. It was introduced around 2.14, I can't remember, and as with all gnome "features" it gets worse with each version.

Do:
Switch to a new desktop.
Open an xterm.
Press F8, which I have bound to maximize-vertically.

Result:
Window maximized to full screen size, shifted down by height of top panel, goes under bottom panel and off bottom of screen.

Workaround:
Drag top panel to bottom of screen, then back to top. Window now resizes itself correctly. Also, next xterm behaves correctly, for a while.

Pertinent package versions:

dev-libs/glib-2.16.3-r1
x11-libs/gtk+-2.12.9-r2
gnome-base/gnome-2.20.3
gnome-base/gnome-common-2.20.0
gnome-base/gnome-desktop-2.20.3
gnome-base/gnome-menus-2.20.3
gnome-base/gnome-panel-2.20.3
gnome-base/gnome-session-2.20.3
gnome-base/gnome-vfs-2.20.1-r1
gnome-base/libbonobo-2.20.3
gnome-base/libbonoboui-2.20.0
gnome-base/libgnome-2.20.1.1
gnome-base/libgnomeui-2.20.1.1
gnome-base/nautilus-2.20.0-r1

Note, alt-drag a window (vertically?) sometimes causes it to make itself bigger, even when it was initially correctly maximized.

Unfortunately I can't reproduce this anymore, not sure why.

From the svn log of the commit that caused the breakage (I'm fairly sure):

2007-04-02  Elijah Newren  <newren gmail com>

	Patch from Carlo Wood to fix handling of unidirectional
	maximization and partial struts.  #358311.

	* src/constraints.c (constrain_maximization):
	determine target size for unidirectionally maximized windows by
	determining how far they can be maximized without hitting
	orthogonal struts.  Avoids weird "empty spaces".


It's that part that I reverted to get it working, but I don't really understand why. Anyone seen Carlo Wood? :)

From time to time I can still reproduce this bug. The windows takes the height of the whole screen, without taking into account the size of the panels. Still, most of the time it works fine, and to trigger the bug I really have to play a lot with the various maximisations and moving the window all around (or I have to wait until I want _for real_ the window to be vertically maximized ;-) ).

So, a summary.  We have an intermittent issue; I've never seen it myself, but many have, and nobody can produce a reliable testcase: a nuisance.  Michael theorises that this problem was introduced by the committed patch 85747, which is huge, of bug 358311, which was that when windows maximised they behaved as though partial struts were full struts.  This was a complicated fix and was said at the time, using a figure of speech I believe to be called meiosis, to be "a bit difficult to follow".

Patch 111378, which is under consideration, has had entirely good reviews.  It has two effects:

 a) It renames a variable called "target_size" to "work_area".  Perhaps this is a better name, but I think the patch would have been a bit clearer if this had been done in a separate patch.

 b) It removes a great chunk of the code added by patch 85747 which deals with checking whether the maximised window overlaps struts.  What is left of constrain_maximize() is code which constrains a maximised window to be the size of the field "work_area_xinerama".  But cunningly, that field has earlier been set to be a rectangle as large as possible while avoiding struts.  So I think there's no reason to assume this doesn't work.

You should note that this patch removes the only call to meta_rectangle_expand_to_avoiding_struts(), so that function should also be removed.  Note that neither the previous sentence nor point a) above is a request in itself for a new patch.

What I would really like is to test that this works properly with PARTIAL struts, because it's supposed to be a fix for a problem introduced by a fix to our handling of partial struts, and I don't want to introduce a regression.

HOWEVER, the original reason for bug 358311 was about unidirectional maximisation, and all this is to deal with constrain_maximise(), which has nothing to do with partial maximisation.

ALSO, if it really is possible to do just what we currently do, for an operation as common as the maximisation constraint, in so much less code, then that's a positive reason to want to commit.

SO, especially since we just branched, I think we can tentatively go for it.

Anyone want to comment before I commit?

Delete the HOWEVER point above, since it's completely untrue.  I was called away in the middle of writeup and unwittingly lost all my mental registers about the problem.

...hence I am less happy about checking this in, since it destroys my neat sorites that the conclusion was built on.

If you have a strut taking up, let's say, 0,0 to 100,100, then the top edge of work_area_xinerama can't be any closer to the top of the screen than 100.  Suppose you have a window whose left-hand side is at 400, and vertically maximise it.  Then if you rely on work_area_xinerama, its top also cannot be closer to the top of the screen than 100.  Then your window won't reach the top of the screen and so you have caused a regression of bug 368311.  I think I have to reject this unless someone can show me I got this wrong.

So, two possible ways forward:
 1) It does look like the code that was removed contained the problem, somewhere.  Someone could search around it very carefully looking for an ill-defined problem

 2) Someone could come up with a repeatable testcase.

I had a more in-depth look at this bug. It definitely comes from meta_rectangle_expand_to_avoiding_struts(), which doesn't detect all the struts.

However, it doesn't seem an algorithm problem. Here is the position and size of the strut that it gets (after instrumenting the code) with a panel at the top and one at the bottom:
Visiting strut from (0,0) + (1280,27)
Visiting strut from (1634362692,1394636118) + (7564897,0)

Or after changing workspace:
Visiting strut from (27759056,0) + (27888256,0)
Visiting strut from (0,0) + (1280,27)

Again later:
Visiting strut from (30394624,0) + (27942352,0)
Visiting strut from (27942944,0) + (0,0)

So, sometimes there is corruption in the strut data. Not sure yet why/when this happens. As a data point, I'm on x86_64. Has any one seen this bug recently on 32bits?

x86_64 frequently has problems reading X properties that are integers, it's easy to get wrong because Xlib does not work in the expected way. (IIRC, 32-bit properties are always converted to "long".) That's the first thing I would check is that the strut hint is read properly.

You could also use xprop to check that windows are setting it properly.

The read of the properties seems correct. I've instrumented ensure_work_areas_validated(), and all_struts is always good. Such as:
Found strut from (0,0) + (1280,27)
Found strut from (0,736) + (492,64)

However, when arriving in constrain_maximization(), which can be very soon after reading the properties when moving a panel, the struts are already corrupted. The GSList itself is always fine, it's only the data of each node which is corrupted (both rect and side).

I'm seeing this bug on Ubuntu 8.04. Happy to help if you need anything tested.

I was seeing this problem every time with version 2.22.0 (from Ubuntu 8.04 x86_64). I've compiled the latest version (2.23.55) and the problem seems to have been fixed.

Eric, I see this on my Core 2 Duo laptop, where both X and metacity are compiled as 32-bit. I also see it on my desktop which is running 64-bit.

Created attachment 116799 [details] [review]
Duplicate the struts of the workspace so they are not freed too early

Ok, I've finally found the exact problem: it was not a 64bit thingy but a "too early free" bug.
ensure_work_areas_validated() creates a list of all the interesting struts out of those of the windows, just pointing to them. However, meta_window_update_struts() updates the strut and free the old one (even if the info is identical), freeing the data still used for the workspaces if done for the window of a panel.

The fix is to simply duplicate the info (it's 5 ints per panel, so not very big). Alternatively, it might be possible to more cleverly free the strut in meta_window_update_struts(), but that would require to be very clever on which strut in all_struts to free in meta_workspace_invalidate_work_area(), so I decided to keep the things simple and just duplicate the values.

The patch is against svn trunk r3816. I guess it's also worth applying to 2.24 branch :-)

Wow, great work in tracking this down. I applied that patch to the metacity 2.22 build I'm using here (and removed the earlier patch from that bug I had applied). I'll let you know if the bug still occurs with your patch applied.

Interesting, such bugs should be easily caught by valgrind. Does anybody debug metacity with it ever?

Yes, metacity gets run under valgrind from time to time-- although not by me recently.  However, mostly bugs in the WM show up when it's being used for everyday work; it's hard to get effective coverage when it's only being run for a test.  It's impractical to run the WM under valgrind for everyday work, so often valgrind problems don't show up unless they're pretty obvious.

This is another reason I need to finish the test suite so we have a decent amount of coverage for things like this.

Awesome! Great work Eric.

In terms of catching similar bugs in future, if it's not practical running under valgrind there are other options for catching use after free bugs - although I haven't used any for a while because valgrind is so good :)

Checked into trunk.

http://svn.gnome.org/viewvc/metacity?rev=3817&view=rev

We had a network outage so I don't want to do the 2.23 release tonight, but I will tomorrow, and I'll put this into it.

Marking FIXED, assuming this problem was the actual problem.  Lovely, thank you!

(Later this went on to cause bug 549952)

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