GNOME Bugzilla – Bug 144126
metacity maximizes windows to wrong size for large STRUTS values
Last modified: 2005-11-01 18:31:50 UTC
Run an app like GOK, or better yet, gnome-mag's "magnifier" binary with the "-v" option (from CVS HEAD, with the patch for bug 124690 applied). Large WM_STRUT values are set (on the order of the screen width). Although Metacity verifies that the strut values are set correctly (by not allowing windows to be dragged all the way under the magnifier window with the mouse), if you maximize a window it's always maximized "too big", so that the right hand side is cut off. Similar problems seem to affect large top strut values. This means that the magnifier's STRUT behavior doesn't really help the user avoid having stuff placed under the magnifier's window.
Havoc, please check and see if I'm correct that this is a metacity bug (and not something weird I'm doing); the fact that the drag bounds are matching the magnifier window correctly makes me think I'm setting STRUTS correctly.
I wrote the struts code for metacity. If I remember correctly there's a sanity check that prevents you from setting ridiculously big strut sizes, though I don't recall exactly how that worked. I think something like enforcing that there's at least a box in the middle of at least a certain minimum size. I'm not sure exactly what's wrong here; I don't have a HEAD build convenient to test on. Could you provide a screenshot or perhaps explain more precisely what happens?
Rob: maybe the sanity check needs to be revisited. Especially if it's something like "dont allow the window minus borders to get maximized to less than half the screen width". That's kind of what it looks like... I can attach a screenshot of mozilla "maximized" alongside a magnifier which takes up exactly half the screen width (RHS)... the mozilla window is wider than half the screen, so the right hand 15% or so of the mozilla window is obscured by the strut owner (magnifier window).
Created attachment 28588 [details] screenshot illustrating "over-maximized" window
Rob: if you point me to the code I might be able to find the problem.
Created attachment 28591 [details] corrected screenshot showing not-quite-correctly-maximized Mozilla this screenshot was made after applying the STRUTS patch to gnome-mag; please ignore the previous patch.
Created attachment 28592 [details] screenshot showing window with left border against the STRUT this attachment suggests that the STRUT value is correctly set, as there is a window visible near the center of the screen which has been dragged to the maximum allowed rightwards extent.
actually, come to think of it: it's screen width/2-15 pixels is the maximum strut size. This was done to avoid a possibility of the constraint iteration failing to find a fixed point. This needs to be changed so that there's at least a 30 pixel "window" somewhere on the screen that's not covered by a strut.
Something like this might fix: gap = screen_width - left_strut - right_strut; gap -= 15; /* require a gap of at least 15 */ if (gap < 0) { left_strut += gap / 2; right_strut += gap / 2; }
Rob: can you prepare a new patch for this? Or are you waiting for me? (I'll be pretty much away till post-GUADEC, but I'd really like to get this resolved soon after. Many thanks!) Note that the gnome-mag patch is now in CVS, so gnome-mag from HEAD can be used to test. "magnifier -v -m" should highlight the problem.
Can we get a fix for this in GNOME 2.6.0 please? It's making some GOK and gnome-mag stuff pretty b0rked. Or, Havoc, if you are waiting for me, just say so and I will create the patch with your code snippet above.
Sorry I'd forgotten about this and I'm a bit swamped right now. If you could create a patch that'd be great. Shouldn't be too hard I hope. -Rob
Someone writing (and more importantly testing) a patch would be helpful, yes.
Created attachment 30518 [details] [review] proposed patch implementing Havoc's suggestion in Comment #9.
This seems to solve most of the problems with gnome-mag and GOK regarding struts.
Comment on attachment 30518 [details] [review] proposed patch implementing Havoc's suggestion in Comment #9. Looks like you use screen->width and the left/right struts to compute gap for the vertical MIN( should be MIN ( looks good, thanks.
actually the gap between left and right struts does need to be at least 75, or the constraints code handling the struts would run into problems, since the left and right constraints would be mutually exclusive and there is no feasible solution to the constraint satisfaction problem. So change MIN_EMPTY back to 75. Also, spaces, not tabs.
and also since you're doing gap/2 you'd need to set it to 76 so that the gap is actually at least 75 and not 74, since 75/2 + 75/2 = 74. Or maybe not; I can't remember if (-75)/2 is -37 or -38. Either way changing it to 76 would be better I think.
Created attachment 30555 [details] [review] Final patch I've gone ahead and made those corrections to the patch and committed this final version. I'm going to go ahead and make a release now with this fix.
Comment on attachment 30555 [details] [review] Final patch It looks like this patch still uses the width struts to compute the gap for the height calculation: + gap = window->screen->width - struts[0] - struts[1]; + gap -= MIN_EMPTY; + new_top.height = (int)struts[2] + MIN (0, gap/2);
Actually now that I think about it this patch doesn't remotely do what I was initially thinking that it did. All it does is look at a single set of struts from a single window. To ensure a feasible solution can be found by the iteration, it would have to look at all the struts, which is a problem just as difficult as the original constraint satisfaction problem in the first place. One solution is to just keep the patch as it is (changing width to height, 0 to 2, and 1 to 3 as appropriate, and just allow the constraint iteration to get confused and hit the paranoia limit if they have really insanely huge struts. Or just have the magnifier not take up fully half the screen.
Note that in practice it would never be a problem if the constraint code gets confused. The result would be mildy unpredictable but consistent; i.e. it would basically choose a constraint to violate and then allow that constraint to be violated from then on. But only if there was one set of struts on the left overlapping with another on the right (or top/bottom). The result in the user interface I think would be really no worse than enforcing a struts gap, since either way the window is underneath a panel-type window possibly irretreivably (barring using the window list to choose Move and moving the window back). So perhaps this patch is OK actually.
Created attachment 30560 [details] [review] Fix to final patch above This fixes the width/height problem. I think I'm reasonably happy with this now.
Rob, you said: "Or just have the magnifier not take up fully half the screen." Not really an option, as the magnifier may need more than half the screen, etc. But as you said, I suspect this final patch fixes the majority of cases.
I take it from the bug status and the attachment status that these last two patches have not been applied?
no they have been, and they're in the 2.8.5 release made yesterday. I left the bug open in case someone has a nice way to do this.
*** Bug 149848 has been marked as a duplicate of this bug. ***
Apologies for spam-- ensuring Sun a11y team are cc'ed on all current a11y bugs. Filter on "SUN A11Y SPAM" to ignore.
The patches have been committed so I'm marking them as such to get them off the list of unreviewed patches (http://bugzilla.gnome.org/reports/patch-report.cgi?product=metacity, which now includes links to direct patches instead of just links to bugs)
Is there justification for keeping this bug open, insofar as the patches have been in CVS for well over a year? I've removed the 'accessibility' keyword for now, to prevent this bug from continuing to show up in the open a11y bugs list, but if you close this bug I'd appreciate it if you reinstated that keyword. Thanks.
probably not.
Actually, due to comment 21 and 22 there was a potential reason for leaving it open. However, I fixed this last issue on the constraints_experiments branch anyway.