Skip to content

Conversation

@phantamanta44
Copy link
Contributor

This PR adds support for the EWMH client message _NET_WM_MOVERESIZE, which allows clients to delegate mouse-based movement and resizing to the window manager. It does this by:

  1. adding a handler for the _NET_WM_MOVERESIZE message that emits the request::mouse_move, request::mouse_resize, and request::mouse_cancel signals when a client makes a corresponding request.
  2. adding default handlers for those signals to awful.permissions that carry out the mouse movement/resizing operations using awful.mouse.resize.
  3. adding extra configuration options to the mouse resizing and placement library functions so that they behave the way clients would expect them to (default behaviour is unchanged).

Some implementation notes:

  • The "keyboard movement" and "keyboard resizing" operations defined in the _NET_WM_MOVERESIZE spec are ignored. It's unclear what they're supposed to mean and they are similarly ignored by i3 and bspwm.
  • The _NET_WM_MOVERESIZE spec mentions that a "race condition" is possible in the case where the user has already released the mouse button before the mouse movement message is processed. This is already dealt with by way of awful.mouse.resize cancelling the operation if the mouse button is no longer pressed.
  • It's a little ugly that the request::mouse_move and request::mouse_resize signals and the awful.mouse.client library functions move and resize apparently do the same thing independently of each other. I could not find a way to delegate one to the other without requiring breaking API or behavioural changes, and so I elected, for the time being, to just leave them independent.

I've tested this on my amd64 NixOS system on the following programs:

  • Firefox — fully functional.
  • Thunderbird — fully functional.
  • Steam — fully functional.
  • Visual Studio Code — functional when windowed, but un-maximizing a maximized window seems to put it in a broken state. This also seems to happen on awesome 4.3 without this PR, so it's probably unrelated...?

This should close #2140 and, by extension, #3136.

@codecov
Copy link

codecov bot commented Sep 30, 2023

Codecov Report

❌ Patch coverage is 45.05495% with 50 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.25%. Comparing base (41473c0) to head (e4b599b).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
lib/awful/permissions/init.lua 20.00% 28 Missing ⚠️
lib/awful/placement.lua 0.00% 10 Missing ⚠️
ewmh.c 82.92% 6 Missing and 1 partial ⚠️
lib/awful/mouse/resize.lua 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3859      +/-   ##
==========================================
- Coverage   54.45%   54.25%   -0.20%     
==========================================
  Files         262      261       -1     
  Lines       31196    31256      +60     
  Branches     1139     1141       +2     
==========================================
- Hits        16988    16959      -29     
- Misses      13702    13792      +90     
+ Partials      506      505       -1     
Files with missing lines Coverage Δ
objects/client.c 79.20% <ø> (+0.21%) ⬆️
lib/awful/mouse/resize.lua 34.65% <0.00%> (-1.07%) ⬇️
ewmh.c 64.90% <82.92%> (+1.79%) ⬆️
lib/awful/placement.lua 47.51% <0.00%> (-2.12%) ⬇️
lib/awful/permissions/init.lua 45.83% <20.00%> (-3.01%) ⬇️

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@actionless
Copy link
Member

it's adding some new api and changing several existing code branches - so would be nice to add some test

@phantamanta44 phantamanta44 force-pushed the ewmh-net-wm-moveresize branch from 231d334 to 6bb8d68 Compare October 3, 2023 13:31
@phantamanta44
Copy link
Contributor Author

  • Added documentation
  • Added integration tests for mouse movement/resizing
    • Augmented the test client with an option that equips it with a GTK 3 custom title bar for this purpose
    • Also fixed a bug I found while writing the tests where the mouse offset calculation wasn't accounting for the client border
  • Updated the default awesomerc so that the rule for enabling title bars doesn't apply to clients with requests_no_titlebar = true
    • I assume title bars were originally added to all clients because of lack of _NET_WM_MOVERESIZE support; therefore, this PR should obsolete that rule

@actionless
Copy link
Member

I assume title bars were originally added to all clients because of lack of _NET_WM_MOVERESIZE support; therefore, this PR should obsolete that rule

i think it was no specific reason for doing that, it just wasn't implemented that specific case, but thanks for looking into it 👍

@actionless
Copy link
Member

also you need to do this:

$ rg 'unpack.*='
tests/test-awful-rules.lua
5:local unpack = unpack or table.unpack -- luacheck: globals unpack (compatibility with Lua 5.1)

i think we should eventually move that to gears.table but it is what it is now 🥲

@phantamanta44
Copy link
Contributor Author

bleh fixed

@phantamanta44
Copy link
Contributor Author

seems like the failed checks are just formatting issues. should i just fix them then squash the last few commits?

@actionless
Copy link
Member

yup, exactly 👍

@phantamanta44 phantamanta44 force-pushed the ewmh-net-wm-moveresize branch from 71cfc9e to 4d6d7a3 Compare October 3, 2023 17:02
actionless
actionless previously approved these changes Oct 3, 2023
Copy link
Member

@Aire-One Aire-One left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this work!

Aire-One
Aire-One previously approved these changes Nov 17, 2023
Copy link
Member

@Aire-One Aire-One left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@phantamanta44
Copy link
Contributor Author

phantamanta44 commented Nov 17, 2023

agh, awful.mouse.client passes corner as part of the args for awful.mouse.resize, but the same args table then gets passed down to the placement function used for the resizing, which conflicts with my definition of corner. that's a bit awkward, and i would guess that's why i originally introduced corner_lock

if anything, i would argue that this new behaviour of locking the resizing axes is more "correct" than the existing behaviour, where moving the mouse from a side of the window towards a corner will change from a single-axis resize to a two-axis (i.e. corner) resize. i'm unsure whether this warrants a breaking behavioural change, however

@Elv13
Copy link
Member

Elv13 commented Dec 31, 2023

(finally...) have time to look at this. First of all, wow, this is very well implemented.

Is this ready to be merged, the tests are still failing.

@phantamanta44
Copy link
Contributor Author

phantamanta44 commented Jan 1, 2024

there are a couple things holding this back:

  • the tests are failing because of my comment above about corner locking in mouse resizing operations. probably the most sensible way to deal with it here is to apply a "bandage" fix like the corner_lock solution from earlier and to leave breaking behavioural changes to a separate PR, regardless of whether this new behaviour is "more correct" or not

  • i've been running on this patch for a while and i've noticed an issue where some windows ignore screen borders (e.g. a task bar) when they're initially laid out on opening. i'm not sure why it happens yet, but as far as i'm aware, it doesn't happen on the main branch. notable apps include intellij idea, okular, and strawberry music player

i'm a bit busy right now so i haven't had time to deal with this stuff, but i'll get back to it soon™

@phantamanta44
Copy link
Contributor Author

  • the tests are failing because of my comment above about corner locking in mouse resizing operations. probably the most sensible way to deal with it here is to apply a "bandage" fix like the corner_lock solution from earlier and to leave breaking behavioural changes to a separate PR, regardless of whether this new behaviour is "more correct" or not

  • i've been running on this patch for a while and i've noticed an issue where some windows ignore screen borders (e.g. a task bar) when they're initially laid out on opening. i'm not sure why it happens yet, but as far as i'm aware, it doesn't happen on the main branch. notable apps include intellij idea, okular, and strawberry music player

what I've just pushed reintroduces the corner_lock thing to deal with the first issue here. after more experimentation, I believe the second issue is actually just #3778, and so it's out of the scope of this PR. at any rate, the tests should pass now

@phantamanta44
Copy link
Contributor Author

Rebased onto master. I've also fixed an issue I was having with Steam duplicating a mouse movement request and thus causing mousegrabber to raise an "already running" error. In particular, I believe xcb_grab_pointer ignores any such duplicate requests if the pointer is already grabbed, a behaviour which I've replicated by guarding against mousegrabber.isrunning() in the request::mouse_* handlers. I am still running this patch on my own machine with no problems, aside from this duplicate request thing.

@actionless actionless force-pushed the ewmh-net-wm-moveresize branch from ff91364 to e4b599b Compare November 17, 2025 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support _NET_WM_MOVERESIZE

4 participants