Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gtk: make sure that window-decoration is honored on all paths #4130

Merged
merged 2 commits into from
Dec 31, 2024

Conversation

jcollie
Copy link
Collaborator

@jcollie jcollie commented Dec 30, 2024

Fix a regression from #4110 .

@mitchellh mitchellh added this to the 1.0.1 milestone Dec 30, 2024
@adriandelgado
Copy link

I confirm that this fixes the regression.

@jcollie jcollie marked this pull request as ready for review December 30, 2024 22:45
@artist-artix
Copy link

artist-artix commented Dec 31, 2024

With this config the problem of the absent window decoration still reproduces:

keybind = ctrl+z=toggle_window_decorations
window-decoration = true
gtk-titlebar = false

The GTK titlebar is not shown: correct
The native WM decoration with its titlebar is not shown: incorrect

Pressing ctrl-z shows some minor change to the window border, but also then the WM decoration with its titlebar is not displayed.
And pressing ctrl-t to create a new tab crashes ghostty.

@jcollie
Copy link
Collaborator Author

jcollie commented Dec 31, 2024

With this config the problem of the absent window decoration still reproduces:

keybind = ctrl+z=toggle_window_decorations
window-decoration = true
gtk-titlebar = false

The GTK titlebar is not shown: correct The native WM decoration with its titlebar is not shown: incorrect

Pressing ctrl-z shows some minor change to the window border, but also then the WM decoration with its titlebar is not displayed. And pressing ctrl-t to create a new tab crashes ghostty.

I don't think that this is directly a result of this PR. I'm getting the following logs when I try and toggle window decorations:

debug(surface): key event binding flags=input.Binding.Flags{ .consumed = true, .all = false, .global = false } action=toggle_window_decorations
warning(gtk): unimplemented action=apprt.action.Action.Key.key_sequence

@artist-artix
Copy link

Indeed this PR is not the cause, but it is related. The problem is originally from commit a7c93cd I think, as there are multiple commits modifying Window.zig it's not easy to determine, but if this PR is to "make sure that window-decoration is honored on all paths" the mentioned issue also needs to be solved.
Thank you.

@jcollie
Copy link
Collaborator Author

jcollie commented Dec 31, 2024

With this config the problem of the absent window decoration still reproduces:

keybind = ctrl+z=toggle_window_decorations
window-decoration = true
gtk-titlebar = false

Ah, I think I understand. When gtk-titlebar=false the titlebar will never be shown, even when toggling window decorations. I'm not sure if that's a regression from previous behavior or not, those are not options that I use.

The decorations are toggling but it's very hard to see as it's basically just a shadow.

@artist-artix
Copy link

artist-artix commented Dec 31, 2024

I'm not sure how to address this, but without a fix for the regression ghostty is no longer usable in case gtk-titlebar=false.
There is no titlebar with the min/max/close buttons, but also creating a new tab crashes ghostty.

@jcollie
Copy link
Collaborator Author

jcollie commented Dec 31, 2024

Indeed this PR is not the cause, but it is related. The problem is originally from commit a7c93cd I think, as there are multiple commits modifying Window.zig it's not easy to determine, but if this PR is to "make sure that window-decoration is honored on all paths" the mentioned issue also needs to be solved. Thank you.

Hmm... The user has explicitly told Ghostty that they do not want a titlebar via --gtk-titlebar=false and that's not what we are toggling. In any case, I don't think that this is going to be a easy/quick fix as we don't even create a titlebar if --gtk-titlebar=false.

@artist-artix
Copy link

artist-artix commented Dec 31, 2024

Indeed gtk-titlebar is false, but window-decoration is true so the native window title bar should be shown just like it was before today's commits.

This is from the documentation

window-decoration
Valid values:
    true
    false - windows won't have native decorations, i.e. titlebar and borders.

There is no word of gtk-titlebar=false overruling window-decoration=true

@jcollie
Copy link
Collaborator Author

jcollie commented Dec 31, 2024

I checked out 1.0.0 and tested:

gtk-titlebar: false
window-decoration: true

Titlebar was shown on launch but could be hidden with toggle_window_decorations

gtk-titlebar: true
window-decoration: true

Worked as expected.

gtk-titlebar: true
window-decoration: false

Worked as expected.

gtk-titlebar: false
window-decoration: false

toggle_window_decorations had no effect, titlebar and decorations never appeared

So I don't think that this was ever working 100% as one would expect.

@mitchellh mitchellh merged commit fa4d4a3 into ghostty-org:main Dec 31, 2024
21 checks passed
@jcollie
Copy link
Collaborator Author

jcollie commented Dec 31, 2024

@artist-artix Please open a new discussion about the interaction of gtk-titlebar, window-decoration, and toggle_window_decorations. It's broken now, but it was broken before, just in a different way. Part of the problem is that the interaction between these options isn't well defined/documented so the behavior has changed as different people have worked on the code. We'll use the new discussion to iron out those details and update the documentation and the code to match.

@artist-artix
Copy link

I have opened #4172

I checked out 1.0.0 and tested:

gtk-titlebar: false
window-decoration: true

Titlebar was shown on launch but could be hidden with toggle_window_decorations

This is exactly as expected and how it is supposed to work when reading the documentation. So in 1.0 this was NOT broken.

@jcollie
Copy link
Collaborator Author

jcollie commented Dec 31, 2024

This is exactly as expected and how it is supposed to work when reading the documentation. So in 1.0 this was NOT broken.

That is not how I would expect things to work, and yes I've read the documentation. So as I said there's both documentation problems and code problems. Please open a new discussion so that we can figure out what the interactions should be between these options rather than continuing the discussion here.

@jcollie jcollie deleted the gtk-fix-window-decoration branch January 3, 2025 01:12
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.

4 participants