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

Delete gtk-csd.sh #103

Merged
merged 3 commits into from Jan 29, 2021
Merged

Delete gtk-csd.sh #103

merged 3 commits into from Jan 29, 2021

Conversation

davidmhewitt
Copy link
Member

@davidmhewitt davidmhewitt commented Nov 13, 2018

Fixes #207
Fixes elementary/gala#244

Can anyone remember why this is still here and what issues removing it would cause?

This is the cause of elementary/gala#244 

Can anyone remember why this is still here and what issues removing it would cause?
@cassidyjames
Copy link
Contributor

I believe it's because we switched to CSD before it was standard, i.e. back in Freya or Loki. If it's now standard I guess that option could be unnecessary and unsupported?

@davidmhewitt
Copy link
Member Author

davidmhewitt commented Nov 13, 2018

@cassidyjames Indeed, that would make sense.

Although, it looks like @danrabbit already had a stab at removing it a couple of years back and then it got restored for some reason, so that makes me suspicious: 6d6d329

@danirabbit
Copy link
Member

I think it might have been something to do with dialog decorations maybe? We should give it a try again and be on the look out for regressions

@cassidyjames
Copy link
Contributor

@danrabbit could this be why root dialogs don't have the correct decorations? If this isn't set in the root profile or something?

@danirabbit
Copy link
Member

@cassidyjames oh that's very possible. Like on the greeter for example?

@cassidyjames
Copy link
Contributor

@danrabbit ah yeah that's another example. But like when running the installer as root, or if you use Files' "Run as Administrator" and then look at a properties dialog.

@davidmhewitt
Copy link
Member Author

Marking this as blocked as it definitely does some weird stuff to dialogs. All dialogs now have a titlebar and it seems it somehow breaks the file/folder chooser completely too.

I'll look into these issues and see if there's a middle ground solution.

@tintou
Copy link
Member

tintou commented Nov 14, 2018

So to me it seems like it is an issue with WxWidgets, after investigating, I think that this is cause by https://github.com/wxWidgets/wxWidgets/blob/cc931612eec2e3ea49200ebff45042135f3c3f9c/src/gtk/toplevel.cpp#L350
When for us the realize function set it to:
https://gitlab.gnome.org/GNOME/gtk/blob/gtk-3-24/gtk/gtkwindow.c#L7488 (client_decorated is set to TRUE in https://gitlab.gnome.org/GNOME/gtk/blob/gtk-3-24/gtk/gtkwindow.c#L4155 )

@davidmhewitt
Copy link
Member Author

@tintou Yes, the issue wouldn't occur with wxWidgets based apps if they stopped requesting decorations. But I think they possibly have that there as leftover/compatibility with GTK2, so I don't know how open they would be to removing it and it wouldn't fix anything in this cycle anyway.

Also, it's not just wxWidgets, there seem to be a number of other apps that do this (including eclipse and MySQL workbench, which is native GTK).

@davidmhewitt
Copy link
Member Author

davidmhewitt commented Nov 14, 2018

The other issue that compounds this is that Gtk.Dialog requests server side decoration instead of CSD if you turn off the actions in the headerbar thing, which we do. This leaves them being server side decorated and a bit weird looking without the shell script, although I wonder if there's something we could do with the stylesheet to make them fit in again?

I would assume that some apps might want to request server side decorations for a reason, so forcing CSD on them too with this shell script is how we've ended up with double decorations in some apps.

Another option is patching GTK to enable CSD on dialogs even when not using the headerbar action buttons, but I'd guess we wouldn't want to do that either.

🤷‍♀️

@danirabbit
Copy link
Member

danirabbit commented Nov 15, 2018

@davidmhewitt Ah yes that was the issue. Maybe it's time to revisit with Gtk+. I remember us bringing this up before and the response being "But why would you want CSD without moving the action area too?". I think we probably should pursue a patch upstream that let's us keep using CSD without moving the dialog action area and also not having to have this force CSD script thing.

@davidmhewitt
Copy link
Member Author

davidmhewitt commented Nov 20, 2018

Ok, I'm now running a patched version of GTK that doesn't request server side decorations for dialogs that don't use the headerbar as the action area, and the file/folder chooser is no longer broken.

However, some of the dialogs are pretty chunky now:
image

I suspect that's because dialogs have a headerbar widget (in their .ui file in the GTK source code) ready to contain the action widgets. Patching that out looks like it'd be a bit harder than just patching the bit that disables SSD.

So I guess the next question is, can we fix that with the stylesheet without breaking anything else? Here's the relevant inspector window with the end session dialog selectors from above visible:

image

@davidmhewitt
Copy link
Member Author

It's worth noting that a patched GTK will fix any other unsightly dialog decorations like running files as root and in the greeter session. So this may be worth pursuing for reasons additional to fixing the apps the request and receive double decorations.

@cassidyjames
Copy link
Contributor

We might just be able to target HeaderBars inside of GtkDialogs with CSS, but should really check if that affects other apps and windows as well.

@danirabbit
Copy link
Member

@davidmhewitt The headerbar in the dialog should also have the class default-decoration. See this screenshot from what we have currently:

screenshot from 2018-11-21 13 35 50 2x

@davidmhewitt
Copy link
Member Author

Not necessarily... the default-decoration class only gets added when the Gtk.Window class constructs a default headerbar, I think. As soon as that headerbar is overriden, you no longer get that class.

Since the dialog is adding its own "custom" headerbar, which granted is exactly the same as the one that Gtk.Window would add in the case when there aren't action buttons in it, the class doesn't get added in this case.

@davidmhewitt
Copy link
Member Author

I have prepared a patch for GTK that adds a new XSettings override option called Gtk/DialogsAlwaysCSD. When enabled, GTK dialogs are constructed using a CSD headerbar (with the titlebar style constant and the default-decoration class applied). The has-subtitle property is also set to false as this was making the headerbars chunkier.

If we apply this patch, along with dropping the GTK_CSD env var, as per this PR and setting the XSettings overrides to enable this new setting by default, then we resolve the issues around having this shell script still around, with no visible differences to anything.

Is this something we'd want to pursue?

@davidmhewitt
Copy link
Member Author

For reference, here is the GTK patch:
https://gist.github.com/davidmhewitt/1cbb7329701ba766051c12959c804621

@danirabbit
Copy link
Member

@davidmhewitt That sounds ideal! Have you proposed this patch upstream in the Gtk Gitlab?

@davidmhewitt
Copy link
Member Author

@danrabbit Not yet, I wrote and tested that patch against the version we're running in Juno. I don't understand GTK versioning well enough to know which branch I should propose merging this into in their repository. Anyone have any ideas?

@tintou
Copy link
Member

tintou commented Feb 5, 2019

@davidmhewitt https://gitlab.gnome.org/GNOME/gtk/commits/gtk-3-24 is the latest and last GTK+3 branch

@davidmhewitt
Copy link
Member Author

Upstream merge request opened here: https://gitlab.gnome.org/GNOME/gtk/merge_requests/566

worldofpeace added a commit to worldofpeace/nixpkgs that referenced this pull request Aug 5, 2019
Causes various issues when not set
* elementary/files#971
* elementary/default-settings#103
* cassidyjames/ideogram#26

However this can cause certain problems in gala
* elementary/gala#244
@danirabbit danirabbit added this to In progress in 6.0 Odin Release via automation Jan 11, 2021
@danirabbit danirabbit moved this from In progress to Public Beta in 6.0 Odin Release Jan 27, 2021
@cassidyjames
Copy link
Contributor

In case it's not obvious how to test this:

sudo nano /etc/profile.d/gtk-csd.h

Comment out the export line, save, then reboot. You'll notice it fixes double-decorations on things like wxwidgets apps, but does show the titlebar on dialogs all over the place, which we'll need to address. I'm kind of the opinion that we rip off the Band-aid and then start filing issues and addressing dialogs as we see them.

@tintou tintou merged commit 4893cee into master Jan 29, 2021
6.0 Odin Release automation moved this from Public Beta to Done Jan 29, 2021
@tintou tintou deleted the remove-gtk-csd branch January 29, 2021 08:45
@gniezen
Copy link

gniezen commented Jan 29, 2021

FWIW, the double-window border bug was fixed by wxWidgets v3.0.5 (https://forum.kicad.info/t/fix-available-for-double-window-border-on-elementaryos/22257/8) which is not in the repo's yet, but works when installed manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Stop setting GTK_CSD=1 Windows requesting decorations get double decorations
5 participants