Skip to content

Fix popup height constraints#224

Merged
wmww merged 2 commits into
wmww:masterfrom
kelnos:fix-popup-height-constraint
Jun 14, 2026
Merged

Fix popup height constraints#224
wmww merged 2 commits into
wmww:masterfrom
kelnos:fix-popup-height-constraint

Conversation

@kelnos

@kelnos kelnos commented May 12, 2026

Copy link
Copy Markdown
Contributor

If a popup menu has enough items to be taller than the output/screen size, the compositor may send a configure event telling the client the max height of the menu. However, gtk_window_move()/gtk_window_resize() doesn't trigger a code path that actually honors the passed size in some cases.

Using gdk_window_move_resize() does trigger the right code path, and menus are constrained properly. We also need to add in the shadow widths/heights, as the GdkWindow considers them a part of its geometry, while from the GtkWindow's perspective, the passed geometry doesn't include shadows.

If the widget is not yet realized (and thus no GdkWindow is available), we fall back to gtk_window_move()/gtk_window_resize().

By opening this pull request, I agree for my modifications to be licensed under whatever licenses are indicated at the start of the files I modified

@kelnos kelnos force-pushed the fix-popup-height-constraint branch from c4e7d4e to e1292af Compare May 12, 2026 05:43
@kelnos

kelnos commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

Didn't check issues first, but this perhaps fixes #148

And I suppose more accurately this PR/commit should be titled fix size constraints, not height. Oh well.

@wmww

wmww commented May 14, 2026

Copy link
Copy Markdown
Owner

Hmm, this looks good at first glance, and I don't doubt it helps some use cases. However, as per the top of the readme this project is in maintenance mode. I'm committed to not breaking existing users, and releases are infrequent. I encourage apps to use GTK4 (I don't know if this works right on GTK4 Layer Shell either, but if it doesn't I'd be much more open to a fix). Is this fix vitally important for an existing app, or app that must be GTK3 for some reason? If not I'm inclined to leave the logic as-is.

@wmww

wmww commented May 14, 2026

Copy link
Copy Markdown
Owner

idk, this does look more correct. If this is a useful fix for legacy apps with real users and somebody cares enough to add tests I'll consider merging.

@kelnos

kelnos commented May 14, 2026

Copy link
Copy Markdown
Contributor Author

GTK3 may not be fully maintained, but it's still widely used. xfce4-panel & xfdesktop (layer shell windows) have an applications menu, and the menu (or one of its submenus) is often longer than the screen height. This bug makes the menu unusable in those situations. (We have zero plans to migrate to GTK4.)

I'm going to be traveling for the next couple weeks, but I'll look into adding tests when I get back, if that's a gate to merging.

If a popup menu has enough items to be taller than the output/screen
size, the compositor may send a configure event telling the client the
max height of the menu.  However, gtk_window_move()/gtk_window_resize()
doesn't trigger a code path that actually honors the passed size in some
cases.

Using gdk_window_move_resize() does trigger the right code path, and
menus are constrained properly.  We also need to add in the shadow
widths/heights, as the GdkWindow considers them a part of its geometry,
while from the GtkWindow's perspective, the passed geometry doesn't
include shadows.

If the widget is not yet realized (and thus no GdkWindow is available),
we fall back to gtk_window_move()/gtk_window_resize().
@kelnos kelnos force-pushed the fix-popup-height-constraint branch from e1292af to 9f29786 Compare May 24, 2026 06:03
@kelnos

kelnos commented May 24, 2026

Copy link
Copy Markdown
Contributor Author

Ok, I added a test, but it needs some work, as it hard-codes the (certainly theme-dependent) shadow width in the create_buffer expectation for the popup. Can you suggest what I should do there, since the tests don't have access to gtk-priv?

(FWIW, the test fails without the fixes in this PR, as create_buffer sends a height much larger than the screen height. With the fix, it's properly constrained by the compositor's configure.)

I also changed the code a bit; realized that the library shouldn't just blindly accept the width/height the compositor suggests, but should take its own needed size into account, and only shrink when the configure event says there isn't enough room. It seemed to work fine without that, but figured this would be a safer change. (Also the compositor is allowed to pass 0 for width/height, in which case we should stick with our own size.)

@wmww

wmww commented Jun 14, 2026

Copy link
Copy Markdown
Owner

.set_window_geometry works reliably and doesn't include shadow. PANEL_HEIGHT=50 was breaking on my system for some reason so I made it bigger.

@wmww wmww merged commit 427a621 into wmww:master Jun 14, 2026
2 checks passed
@wmww

wmww commented Jun 14, 2026

Copy link
Copy Markdown
Owner

Thanks! Sorry that took a while. Will be in next release, whenever that ends up being.

@kelnos kelnos deleted the fix-popup-height-constraint branch June 15, 2026 01:16
@kelnos

kelnos commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Great, thank you for taking care of it!

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.

2 participants