search

xmonad / xmonad-contrib

Contributed modules for xmonad
https://xmonad.org
BSD 3-Clause "New" or "Revised" License
584 stars 274 forks source link

EwmhDesktops: Set _NET_WM_ALLOWED_ACTIONS #781

Closed ilya-bobyr closed 1 year ago

ilya-bobyr commented 1 year ago

Description

According to the "Extended Window Manager Hints" spec, _NET_WM_ALLOWED_ACTIONS is supposed to be set by the Window Manager to indicate to the clients, which operations are supported for a given window.

EwmhDesktops was only declaring supported operations on the root window via _NET_SUPPORTED, giving a superset of everything it supports. But individual windows may have different operations.

For now, only _NET_WM_ACTION_CLOSE is added to _NET_WM_ALLOWED_ACTIONS. Assuming that other operations, such as movement, resizing, etc. are handled by the layouts. But it is not fully correct. For example, it seems perfectly fine to allow floating windows to resize themselves.

ewmhFullscreen adds _NET_WM_ACTION_FULLSCREEN to _NET_WM_ALLOWED_ACTIONS for all managed windows, indicating that windows could request a full screen mode.

TOOD: Ideally, XMonad should remove the _NET_WM_ALLOWED_ACTIONS property from all windows when it is restarting or shutting down. I could not figure out how to do it.

This change also fixes Firefox full screen, as suggested in this bug report: https://bugzilla.mozilla.org/show_bug.cgi?id=1273746#c7

Checklist

geekosaur commented 1 year ago

This looks incomplete to me. For example, minimize is allowed and works; to manipulate it from xmonad you need a module, but you can do it from e.g. a panel applet just fine. We should probably review all the states to see which ones should be marked as supported. (For example, maximize should probably not be marked as supported even though you could use e.g. MultiToggle or X.L.Maximize, since neither one allows a window to request it.)

ilya-bobyr commented 1 year ago

I can add more operations to the initial value of _NET_WM_ALLOWED_ACTIONS if you think they should be marked as supported as well.

One can look at this PR as a first step. It does not prevent more operations to be added later. So, I probably would not call it “incomplete”, if you just mean that more can be done. At the moment, no operations are marked as supported at all.

ilya-bobyr commented 1 year ago

One part that is actually missing is the removal of the _NET_WM_ALLOWED_ACTIONS property when XMonad exits. It should not be a big problem, but, ideally, it would be nice to clean up. Layouts receive ReleaseResources LayoutMessage, but there seems to be no hook for non-layout extensions. Would it make sense to add a shutdownHook into XConfig, or did I miss something?

TheMC47 commented 1 year ago

Would it make sense to add a shutdownHook into XConfig, or did I miss something?

There's no shutdown hook currently. I agree it would be useful; not particularly in this case though as I don't see the problem of keeping the properties set when xmonad exits/restarts.

ilya-bobyr commented 1 year ago

I figured, if there is no active window manager, then it would seem right for _NET_WM_ALLOWED_ACTIONS to be absent. If the user switches to a new window manager, this new WM will be responsible for setting the _NET_WM_ALLOWED_ACTIONS, if it supports the spec. If it is just XMonad restarting, then _NET_WM_ALLOWED_ACTIONS will be set again if ewmh is in the config.

It is an edge case, I agree. Most users will never be in this situation. For XMonad users, it might only matter when the user removes ewmh from their config and does not reboot the machine. In any case, I think, advertising operations that are not really supported by the WM is unlikely to cause big UX issues.

ilya-bobyr commented 1 year ago

Firefox full screen did break for me. And after I've made this change it started to work. But apparently the reason was something else.

Tomáš Janoušek suggested I close the PR, as the issue is gone.