GNOME Bugzilla – Bug 758186
Wine fullscreen-windows crash on minimize - patch included
Last modified: 2015-12-14 19:35:34 UTC
Created attachment 315691 [details] diff-file for mutter/src/x11/window-x11.c Wine does minimize windows on alt-tab now as Windows does. Mutter and other Metacity based WMs suffer from a bug that let them crash as they do not support XIconifyWindow without MWM_FUNC_MINIMIZE. This is a patch from MATE that makes it work again which is of great benefit for wine user/gamer. Tested on Fedora 23, wine-staging 1.7.53, Starcraft 2 Related: https://github.com/mate-desktop/marco/issues/166 https://github.com/mate-desktop/marco/pull/167 https://bugs.winehq.org/show_bug.cgi?id=37765 alternative patch: https://github.com/mate-desktop/marco/pull/169 Hoping to attract interest and best regards, Robert
Muffin also has this code version.
Er, so we just flat ignore the minimize field from the window? I'm not a big fan of that. Why do some of these windows say they don't support minimize?
Stefan Dösinger describes it here: https://github.com/mate-desktop/marco/issues/166
I think the idea is that a window shouldn't be able to minimize itself if there's no way for the user to unminimize it, or it just doesn't make sense. The origin of the check is: 2001-10-07 Havoc Pennington <hp@pobox.com> * src/window.c (meta_window_client_message): don't allow shade/maximize/minimize for windows that don't support those operations. (minimizing the panel = bad) But this doesn't actually correspond exactly to has_minimize_func, so changing it a bit to look instead at whether the window is !skip_taskbar would seem fine to me - skip_taskbar should tell us exactly whether the user has a way to get back to the window if minimized. It could potentially break an assumption that tasks lists have that minimized windows only are those with NET_WM_ACTION_MINIMIZE, but - I suppose MATE tested this with their panel, and we don't care about external process task lists for Mutter. - The official task list shell extension in GNOME actually allows minimizing windows that are !has_minimize_func - these windows that have minimize grayed out in their window menu, but enabled in the task list popup menu. In summary, I think changing the check to !skip_taskbar is fine, removing it is probably a bad idea.
(In reply to Owen Taylor from comment #4) > - I suppose MATE tested this with their panel, and we don't care about > external process task lists for Mutter. This isn't necessarily true -- the dock in the Elementary DE is external. I think this is still fine though.
Works fine for me with !window->skip_taskbar. Tested with starcraft 2, diablo 3 and warcraft 3.
Created attachment 315746 [details] [review] patch file as discussed below
Any chance that this gets included during gnome 3.18 livecycle? It has been adopted in other window managers and seems to work fine. It would greatly improve out-of-the-box wine-gaming experience for many users with little risk, eliminating the need to install other DEs beside gnome. Regards
Update: This also affects xwayland. So the patch solves this issue for wayland-sessions, too.
Created attachment 316812 [details] [review] window: Allow minimizing windows which don't advertise support for it Wine removes the minimize func from its Motif hints on full-screen windows, because, as the Win32 API literally says, the minimize button is indeed not visible on full-screen windows. Given that this code was added to prevent minimizing a panel by accident, I don't necessarily think that it's relevant anymore.
Review of attachment 316812 [details] [review]: ::: src/x11/window-x11.c @@ +2442,3 @@ meta_verbose ("WM_CHANGE_STATE client message, state: %ld\n", event->xclient.data.l[0]); + if (event->xclient.data.l[0] == IconicState) Doesn't this mean that a skip_taskbar window is allowed to disappear here?
(In reply to Rui Matos from comment #11) > Doesn't this mean that a skip_taskbar window is allowed to disappear here? Sure, but since it itself sent the _NET_WM_STATE message, I think that's OK. Anywhere else that might show it (window menu, panel) has disabled it because of the has_minimize_func.
Comment on attachment 316812 [details] [review] window: Allow minimizing windows which don't advertise support for it (In reply to Jasper St. Pierre from comment #12) > (In reply to Rui Matos from comment #11) > > Doesn't this mean that a skip_taskbar window is allowed to disappear here? > > Sure, but since it itself sent the _NET_WM_STATE message, I think that's OK. > Anywhere else that might show it (window menu, panel) has disabled it > because of the has_minimize_func. Ok, I don't have a strong opinion, just wanted to make sure that's really what you intended here. The window should still be accessible from the overview in any case.
skip_taskbar windows aren't in the overview, though. Or did I get that wrong?
(In reply to Jasper St. Pierre from comment #14) > skip_taskbar windows aren't in the overview, though. Or did I get that wrong? Aha, you're right, they aren't. In that case, I don't like this as much. Why do you want to allow applications to do this? It's orthogonal to the bug report anyway.
Read the original WINE report. The issue is that XIconifyWindow (which sends the WM_CHANGE_STATE message) doesn't work when windows don't have the minimize reported. Since MWM_DECOR_MINIMIZE does not have any definition or specification, and no other window manager special cases WM_CHANGE_STATE for MWM_DECOR_MINIMIZE, I'm matching the behavior of other WMs. So we're allowing windows to minimize themselves even if they set MWM_DECOR_MINIMIZE. That's the window-x11.c part of the patch. The special case was added to prevent users from hiding skip-taskbar windows through (namely, gnome-panel). To prevent users from minimizing windows like that in the future, we automatically remove the minimize button from windows that are marked as skip-taskbar to make sure that users can't accidentally minimize it with no way to get it back.
Right, I understood all that. And I think window-x11.c should be changed so that !window->skip_taskbar is a requirement to minimize a window. Not a strong opinion though, so feel free to push as you prefer.
Attachment 316812 [details] pushed as 8cc345f - window: Allow minimizing windows which don't advertise support for it I obviously disagree, so pushing now.