[vim/vim] fix(gtk): use GTK_DIALOG_MODAL over type hints (PR #20727)
dezza
Closes: #17760
It is unclear whether #172 was properly fixed by
#16100
Remaining issues on wayland:
-
Centering dialog
(unlikely to exist a proper fix as compositor is supposed to center the dialog, not gtk3 - so it will become an advanced hack) -
Find/replace dialog starts maximized , occupying entire
gui.mainwin -
Other dialogs needs tested
@CS-cwhite can you clarify if the issue was fixed for you with your PR or it reappeared? Either way 5049daf
GDK_WINDOW_TYPE_HINT_POPUP_MENU is the wrong fix, since it causes new issues (unfocus on wayland, #17760)
@aswild welcome to test this PR
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/20727
Commit Summary
- 81c7c2c fix(gtk): use GTK_DIALOG_MODAL over type hints
File Changes
(1 file)
- M src/gui_gtk.c (5)
Patch Links:
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Allen Wild
aswild left a comment (vim/vim#20727)Hi @dezza, thanks for the patch! It's working alright for me on my WSL setup. Seems like the same behavior as setting GDK_WINDOW_TYPE_HINT_DIALOG.
There's one quirk/bug, in that it's possible to click the main window and send the dialog to the background. When the dialog is "hidden", moving the mouse around or clicking outside the window usually gets it back. This seems to happen on wayland or x11 mode.
Arch Linux, gtk3 3.24.52, WSL2.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
dezza
dezza left a comment (vim/vim#20727)Hi @dezza, thanks for the patch! It's working alright for me on my WSL setup. Seems like the same behavior as setting GDK_WINDOW_TYPE_HINT_DIALOG.
There's one quirk/bug, in that it's possible to click the main window and send the dialog to the background. When the dialog is "hidden", moving the mouse around or clicking outside the window usually gets it back. This seems to happen on wayland or x11 mode.
Arch Linux, gtk3 3.24.52, WSL2.
thanks, I couldn't reproduce, but I found another issue dialog when not maximized can be outside window. I'll see what can be done.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Allen Wild
aswild left a comment (vim/vim#20727)Retested on 6325d11
Overall I'd say this seems basically fine. The dialog getting hidden when clicking around on the window isn't really something that would happen "normally" and is the sort of thing I'm very willing to accept as general Linux GUI jankyness. Keyboard focus and mouse input on the dialog work as expected and that's the most important thing.
Thanks for your work here and in the dialog overhaul feature - I know how much of a pain it can be dealing with cross-platform GUI stuff and all of the weird Linux compositors and implementations and backends and such.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
dezza
dezza left a comment (vim/vim#20727)The dialog getting hidden when clicking around on the window isn't really something that would happen "normally"
I have tried to press the buffer edit area behind dialog in many places and title of the window as well as outside the window using wayland. I don't think I can reproduce on kwin with wayland. Tried xwayland from within kwin wayland as well.
Thanks for your work here and in the dialog overhaul feature - I know how much of a pain it can be dealing with cross-platform GUI stuff and all of the weird Linux compositors and implementations and backends and such.
Thanks! I see maintaining gtk2,gtk3, legacy compat as a responsibility now I have digged so much in the gui code. I know how much legacy compat means to the project and has been a core value, so I prioritize it before adding new stuff and see it as a required chore.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
dezza
dezza left a comment (vim/vim#20727)@aswild when you have time feel free to test
git clone -b fix-gtk-focus-dialog-refocus-parent g...@github.com:dezza/vim.git
dezza@86e8332
https://github.com/dezza/vim/tree/fix-gtk-focus-dialog-refocus-parent
I can't test it, but I hope it works.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Allen Wild
aswild left a comment (vim/vim#20727)e558003 doesn't seem to have any new or different behavior with this dialog as far as I can tell
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
dezza
dezza left a comment (vim/vim#20727)e558003 doesn't seem to have any new or different behavior with this dialog as far as I can tell
This one is the change - just had to fix preproc after, I can't reproduce so I had to ask.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
nitro322
nitro322 left a comment (vim/vim#20727)hi, @dezza. Apologies for the delay, just got a chance to test. Yes! It seems to work well. I have a pretty simple use case - I just want to be able to navigate the buttons with tab and select no, so I don't have to use the mouse to close the window. It seems to work great for that.
In the process I did discover an unrelated issue. When building with --enable-gui=gtk4, it seems support for "set lines" in ~/.gvimrc is completely broken. I have set lines=80 in my .gvimrc, but the newly built gvim always and seemingly only opens with lines set to 22. When running :verbose set lines? I can even see it referencing the correct line in my .gvimrc:
lines=22
Last set from ~/.gvimrc line 21
Line 21 is set lines 80, yet clearly it shows it's set to only 22. Really weird. It seems like this issue, but that was supposedly fixed:
#17286
I don't have this problem when rebuilding with gtk3, and your patch also works with gtk3, so I'm very happy with that.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
dezza
dezza left a comment (vim/vim#20727)@nitro322 thank you! I think these changes should be isolated from gtk4, is it related to this change? Appreciate the feedback regardless - I don't have much knowledge on the gtk4 impl yet.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
nitro322
nitro322 left a comment (vim/vim#20727)no, I don't think your change would've caused the issue I reported with gtk4. Just something weird and very unexpected I observed that really threw me off.
I also built from current master, which doesn't seem to have this pull request, right? I get the same broken lines behavior on gtk4 there, but switching to gtk3 results in the un-navigable confirmation dialog from before you fixed it. So that should pretty thoroughly rule out this patch as having any adverse impact there.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
dezza
dezza left a comment (vim/vim#20727)Try the new dialogs. #20738
git clone -b feat-gtk3-overlay-dialog g...@github.com:dezza/vim.git
These can't get any better, but this is the best fix (revert) to old behaviour.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
dezza
dezza left a comment (vim/vim#20727)@aswild do the bugs you discovered with this also happen on master/packaged gvim? If yes, then I think I can ready this for review - its unlikely this gets much better since shortcoming on gtk3-wayland with positioning.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Allen Wild
aswild left a comment (vim/vim#20727)@aswild do the bugs you discovered with this also happen on master/packaged gvim?
Yeah, looks like the same thing happens on master (as well as the gvim version 9.2.0804-1 package in Arch Linux under WSL2). This PR is a definite improvement (positions the dialog better) so I agree it should be fine to merge.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
nitro322
nitro322 left a comment (vim/vim#20727)I tested with the latest version, @dezza. I see the fancy new icon. It works for my needs, but personally I prefer the previous version because the title bar was a header, and it was a movable window if I tried to drag it (not that I have a real reason to, but that's what I'd consider expected behavior).
image.png (view on web)New version is all black, no way to distinguish title bar, and completely immovable:
image.png (view on web)However, keyboard (tab) navigation still works. So honestly, I'm happy either way, and since gtk3 is supposedly on the way out I'm not greatly bothered. But, just wanted to provide feedback.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
dezza
dezza left a comment (vim/vim#20727)I tested with the latest version, @dezza. I see the fancy new icon. It works for my needs, but personally I prefer the previous version because the title bar was a header, and it was a movable window if I tried to drag it (not that I have a real reason to, but that's what I'd consider expected behavior).
Yeah the problem with the old dialogs is the bad support for wayland. The old dialogs will still be available, but I'd say they are mostly usable on X11 and wayland, at least should default to the new GtkOverlay ones.
I believe there are pros and cons to both, however its predictable that they spawn inside the main window instead of potentially concealed on multiple screens or behind layering.
This exact PR solves the old dialogs not spawning with keyboard focus.
The other PR solves compatability on mainly wayland and normalizing behaviour between both backends of x11+wl.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Christopher White
CS-cwhite left a comment (vim/vim#20727)Apologies for not responding --- I've been buried at work. Sorry about that!
clarify if the issue was fixed for you with your PR or it reappeared?
Yep, every time Ubuntu pushes a new vim 8, the issue returns. I re-apply my patch from #16100 every time and the issue is fixed. (currently Ubuntu 22.04 x86_64, Wayland, GNOME 42.9, Ubuntu vim==2:8.2.3995-1ubuntu2.34)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
dezza
dezza left a comment (vim/vim#20727)Yep, every time Ubuntu pushes a new vim 8, the issue returns. I re-apply my patch from #16100 every time and the issue is fixed. (currently Ubuntu 22.04 x86_64, Wayland, GNOME 42.9, Ubuntu
vim==2:8.2.3995-1ubuntu2.34)
I just fixed a similar issue with focus for the new dialogs. Can you please try if adding _after like here to the focus-out or both focus-out and focus-in would solve it without the POPUP hint?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
dezza
dezza left a comment (vim/vim#20727)Popup hint as introduced by #16100 is a worse default because:
- Popup hint defaults to unfocused when it appears, forcing you to use the mouse to focus it.
- Popup has more cons, side-effects than defaulting to the modal.
.. if someone is opposed or have bugs they need to come with reproducable steps, but the previous default was the better one, because most people will be opposed to losing the default keyboard focus anyways.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#20727)thanks. It seems people have more issues with what #16100 was supposed to fix. So let me include it.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.