elementary
Port to Gtk4
Fixes #643
TODO
- [x] Extract what can be done in Gtk3
- [x] Finish drop onto terminal widget
- [x] Fix cancelling close tab with foreground process
- [ ] Get tests working.
Notes:
- CheckButtons do not seem to work with only an action_target (no action name) in Gtk4 so the theme string is now added as data. Smaller diff than creating an action to change the theme.
- Theme buttons needed to have different css class names added to them when their provider is added to the display else they overwrite each other. Adding the providers to the buttons own style context did not work.
- Application tests have been inactivated pending finding out why they do not work under Gtk4. CI now expected to fail for stable and unstable but pass for development target.
vte-2.91-gtk4(?) seems to have a bug in the native handling of copy paste. This did not work as is does under Gtk3 even if the keypress controller was removed. So for now we handle both modes.- Since we already have a keycontroller, it is easier to implement keyboard zooming in the handler rather than create zoom actions and Gtk.Shortcuts.
- Application.add_accels_for_action only works for global action so added shortcuts for terminal actions using a ShortcutController.
- Ctrl-Scroll Zoom is simplified and is smoother. KeyController is no longer involved. At present any combination of modifiers including
<Control>works but that is easily changed if required.
CI tests failing both locally and here - some obscure errors about MESA, glx etc. Not sure how to fix this. Will see if any other Gtk4 projects have working tests.
@danirabbit I have a problem with styling the theme buttons in the SettingsPopover. It looks like valid CSS is being loaded into the css_provider but it has no effect. Maybe you can shed light?
@danirabbit Thanks for the review! I'll double-check whether there is anything that is not essential to change and address the other issues. It hard not to try and make "improvements" while porting! I am now dog-fooding this branch as it feels close to being ready.
You've introduced a lot of
todoandfixmeandnotecomments here that are just things like "Make sure this works in GTK4". So I think those all need to be resolved/removed before merging
Yeah - these were merged in from master I guess. I'll remove.
@jeremypw can you do any of these test changes in separate branches? The diff is now over 1,100 lines :(
@danirabbit Sure, if its OK to omit failing (headless) tests for the first Gtk4 merge. Considering most elementary projects do not have any unit tests I should think this wouldn't be a problem.
@danirabbit Not sure I can properly test match_keycode with non-standard hardware as I do not have any. Need to get a suitable reviewer to test it really. Other than that I think I have dealt with TODOs and FIXMEs that are not present in master and are essential for the first merge or can readily be done. In some cases they refer to possible upstream issues and do not need to be fixed immediately as functional work rounds are in place.
Checked that the copy/paste shortcuts still work when a non-English keyboard layout is selected.
@danirabbit I think this is pretty much ready to merge subject to your approval. I don't think the issue with headless testing in Gtk4 should necessarily hold up going ahead with the port to Gtk4 bearing in mind that this is the only elementary project using headless tests afaik. Hopefully a dev with the necessary skills/knowledge will be able to reproduce them in Gtk4/Wayland at some point with a separate PR.
Hey sorry I haven't followed up on this in a while! I'm out right now recovering from surgery so it might be a couple weeks before I can look at this again. Maybe someone else can pick up review here
@danirabbit Oh OK, sorry I didn't realize. No hurry - please take all the time you need to recover.
@lenemter Thanks for taking a look at this!
Looks like color dialog styles got broken a bit:
Could you point out the main differences? I must be a bit blind but they look pretty close to me.
Ah, the leading margin is a little larger in Gtk4. And the trailing margin and margin between grid and button.
I don't think I made any changes to the explicit margins in ColorPreferencesDialog so I guess the changes are down to differences in the applicable css in Granite-7/Gtk4?
Found an interesting bug:
- Open settings popover
- Disable automatic system theme and select a theme
- Close popover and open it again
- Start toggling "Auto-Hide tab Bar"
- After a while the popover closes and never opens again. When this happens these warnings are produced:
(io.elementary.terminal:39260): Gtk-CRITICAL **: 18:23:05.082: GtkBox 0x5cb2fe443dd0 reports a minimum width of 32, but minimum width for height of 1048576 is 207. Expect overlapping widgets.
(io.elementary.terminal:39260): Gtk-CRITICAL **: 18:23:05.082: GtkBox 0x5cb2fe443dd0 reports a minimum width of 32, but minimum width for height of 1048576 is 207. Expect overlapping widgets.
(io.elementary.terminal:39260): Gtk-CRITICAL **: 18:23:05.082: GtkBox 0x5cb2fe443dd0 reports a minimum width of 32, but minimum width for height of 1048576 is 207. Expect overlapping widgets.
@lenemter Couldn't reproduce that bug on Wayland (Secure Session) but it happened on X (Classic Session). It may be an upstream bug. I'll look into it further.
It seems that bug occurs with any of the toggle switches. It may be related to the revealers associated with them - not sure these are really needed?
Oh, these are part of the Granite.SwitchModelButton so the bug may be there. Not sure its worth changing this widget as X is on the way out.
Hmm, triggering this bug under gdb with G_DEBUG=fatal-criticals causes the whole desktop to lockup so cannot get the backtrace. Looks like an upstream bug.
@danirabbit Conflict fixed. Would you mind if I release v 7.1.3 first as that is almost ready? There is only one PR targeted at it remaining and that could be dropped from the release if necessary.
@jeremypw not at all, though it looks like you have some new features so probably merits a 7.2 ;)
@jeremypw just wanna double check before hitting the button that you're okay if we merge :)
Should the development branch be renamed main after merging the Gtk4 port?
Yup! I realized we don't have a packaging update prepared, so I will do that and then merge and rename the development branch etc