GNOME Bugzilla – Bug 643606
Maximizing windows not working if window is too wide
Last modified: 2012-03-19 07:19:22 UTC
(I'm using the latest gnome-shell from Fedora 15 repository) When starting Evoluton with LC_ALL=de_DE the window is too wide to fit on 1024x768. This didn't happen under GNOME 2, now with gnome-shell the fonts and everything else are larger than before, and there seems no obvious way to change is. But the main bug is: if I move the Evolution window to the upper border in order to maximize it, the shell displays the blueish overlay indicating that the window is maximized if I drop it, but then it isn't maximized (though it receives the maximized property, i.e. I cannot resize it afterwards). This works fine for windows that aren't too wide (e.g. LC_ALL=C evolution), but not if the window is too wide. I have to manually un-maximize it and then manually change the size to maximum.
Created attachment 188603 [details] screenshot Can confirm this.
FYI. To reproduce this: * Change resolution to say 1024x768 using Display Settings * Run LC_ALL=de_DE evolution * Right click on titlebar and select Maximize
Also confirmed with LC_ALL=de_DE palimpsest
Created attachment 209284 [details] [review] window: Disallow maximization for windows that can't be maximized
Talked to Jon a bit. if the window can't fit in the screen size, there's no point in pretending it can be maximized. This is really the only solution we can do.
Review of attachment 209284 [details] [review]: Approach makes sense to me. Commit message lacks a body. ::: src/core/window.c @@ +3673,3 @@ +static void +get_possible_tile_area (MetaWindow *window, Don't like that name ... maybe get maximum or available tile area? @@ +3679,3 @@ + + monitor = meta_screen_get_current_monitor (window->screen); + meta_window_get_work_area_for_monitor (window, monitor->number, tile_area); I'd just do window->monitor->number ... you don't need "monitor" for anything else anyway. @@ +3713,3 @@ + /* Don't call can_maximize here -- we end up doing mostly + * the same calculations, so just do the only unique check + * here. */ "Only unique check" sounds weird ... common instead of unique?
Review of attachment 209284 [details] [review]: ::: src/core/window.c @@ +3673,3 @@ +static void +get_possible_tile_area (MetaWindow *window, Everything about the name is terrible - it's not just related to the tile area, either. It's just the monitor rect minus visible borders... ... Wait, why are we removing the visible borders? I think this code is incorrect and we should be removing the invisible borders... @@ +3679,3 @@ + + monitor = meta_screen_get_current_monitor (window->screen); + meta_window_get_work_area_for_monitor (window, monitor->number, tile_area); I copied the code from meta_window_can_tile_side_by_side. I'm not sure of the case where the window's monitor and "the current" monitor are different in practice, but I want to make sure I'm not breaking anything. @@ +3713,3 @@ + /* Don't call can_maximize here -- we end up doing mostly + * the same calculations, so just do the only unique check + * here. */ It's not common, though. It's the one check that's in can_maximize, but not can_tile_side_by_side. "the only check unique to can_maximize" maybe?
(In reply to comment #7) > Review of attachment 209284 [details] [review]: (Argh. yet another case of "doing a review does not add me to CC" -.- ) > ::: src/core/window.c > @@ +3673,3 @@ > > +static void > +get_possible_tile_area (MetaWindow *window, > > Everything about the name is terrible - it's not just related to the tile area, > either. It's just the monitor rect minus visible borders... *You* named it ... ;) > ... Wait, why are we removing the visible borders? I think this code is > incorrect and we should be removing the invisible borders... Err yeah. > @@ +3679,3 @@ > + > + monitor = meta_screen_get_current_monitor (window->screen); > + meta_window_get_work_area_for_monitor (window, monitor->number, tile_area); > > I copied the code from meta_window_can_tile_side_by_side. I'm not sure of the > case where the window's monitor and "the current" monitor are different in > practice, but I want to make sure I'm not breaking anything. We do use window->monitor->number in a lot of places so I doubt this will break anything. > @@ +3713,3 @@ > + /* Don't call can_maximize here -- we end up doing mostly > + * the same calculations, so just do the only unique check > + * here. */ > > It's not common, though. It's the one check that's in can_maximize, but not > can_tile_side_by_side. > > "the only check unique to can_maximize" maybe? I meant "common" as in "both have it".
(In reply to comment #8) > > I copied the code from meta_window_can_tile_side_by_side. I'm not sure of the > > case where the window's monitor and "the current" monitor are different in > > practice, but I want to make sure I'm not breaking anything. > > We do use window->monitor->number in a lot of places so I doubt this will break > anything. The two may indeed be different: window->monitor->number is the monitor which contains the largest part of window; meta_screen_get_current_monitor() is the monitor which contains the pointer. In most cases the two are identical (because the function is only called while dragging the window around iirc), but they may be different near the screen edges (which happen to be the areas we are most interested in for tiling). (For deciding whether we show the tile previews, we check whether the pointer is near a monitor edge; it would be pretty weird if entering the "hot area" triggered a tile preview on another monitor, so we really want to base the "tile monitor" on the pointer position)
Created attachment 209545 [details] [review] window: Disallow maximization for windows that can't be maximized Windows that have minimum widths larger than the screen can't be maximized, even though we put them in a maximized state and allow users to do so: the window just won't change size and position. Fix this by simply not giving the option to maximize, like what happens for non-resizable windows. I still can't find a good name for that function. As Florian points out, I think meta_screen_get_current_monitor is the appropriate thing to do here (though a lot of this will break when we start handling touch and multiple pointers, but that's not my bug :)
i think you need to make sure _NET_WM_ALLOWED_ACTIONS is up to date, too, otherwise libwnck will be out of sync from mutter (not that that would practically matter in most cases). Still "git grep has_maximize_func" shows a lot of places where has_maximize_func is checked to mean "can maximize", and it shows that variable getting toggled off in some cases where maximization isn't allowed. So I think we already have a can_maximize() kind of thing in the code, it's "window->has_maximize_func". Given that, maybe it makes sense to toggle it directly? It looks like recalc_window_features would be an appropriate place.
Created attachment 209557 [details] [review] Simplify the frame testing logic in callers to grab borders A lot of code did something similar to: MetaFrameBorders borders; if (window->frame) meta_frame_calc_borders (window->frame, &borders); else meta_frame_borders_clear (&borders); Sometimes, the else part was omitted and we were unknowingly using uninitalized values for OR windows. Clean this up by just testing for a NULL frame in meta_frame_calc_borders and clearing for the caller if so.
Created attachment 209559 [details] [review] window: Fix meta_window_get_workspaces when a window isn't on a workspace Since we're going to be evaluating the work area at startup now, we need to make sure that we don't iterate over workspaces before they're assigned. The easiest way to do this is to make sure that meta_window_get_workspaces doesn't crash.
Created attachment 209560 [details] [review] window: Disallow maximization for windows that can't be maximized Windows that have minimum widths larger than the screen can't be maximized, even though we put them in a maximized state and allow users to do so: the window just won't change size and position. Fix this by simply not giving the option to maximize, like what happens for non-resizable windows.
Review of attachment 209560 [details] [review]: Makes sense, fine to commit with the comment fixed. ::: src/core/window.c @@ +4506,3 @@ + + /* If we're changing monitors, we need to update the has_maximize_func flag, + * as the working area has changed. */ "work area"
Review of attachment 209559 [details] [review]: Looks good.
This still depends on the first "frame borders" commit -- while not necessary, it's a nice cleanup IMO.
Comment on attachment 209559 [details] [review] window: Fix meta_window_get_workspaces when a window isn't on a workspace Attachment 209559 [details] pushed as 578b1c0 - window: Fix meta_window_get_workspaces when a window isn't on a workspace
Created attachment 209973 [details] [review] window: Disallow maximization for windows that can't be maximized Windows that have minimum widths larger than the screen can't be maximized, even though we put them in a maximized state and allow users to do so: the window just won't change size and position. Fix this by simply not giving the option to maximize, like what happens for non-resizable windows. And if you don't want to review the frame testing logic patch because we're close to hard code freeze, here's an alternate version that doesn't depend on it.
Review of attachment 209557 [details] [review]: Looks good.
Review of attachment 209973 [details] [review]: We don't need this one.
Attachment 209557 [details] pushed as 8c1b2d5 - Simplify the frame testing logic in callers to grab borders Attachment 209560 [details] pushed as 7f64d6b - window: Disallow maximization for windows that can't be maximized
*** Bug 672378 has been marked as a duplicate of this bug. ***