GNOME Bugzilla – Bug 740123
Add the style context provider with FALLBACK priority
Last modified: 2021-03-23 20:08:59 UTC
We shouldn't use a higher priority than the theme. Since Adwaita doesn't set this, it'll still be used, and themes can override it.
Created attachment 290719 [details] [review] Add the style context provider with FALLBACK priority Adwaita doesn't set these properties any more, but other themes might want to. Set a lower priority than the theme.
Last time I checked (when I wrote this code), FALLBACK prio didn't work. Does it now?
I can override the colour in my theme, and using Adwaita both the light and dark variants set the bg as theme_base_color in gnome-terminal. Any other check required?
Cheers, please push
Pushed as a264918ad22973c69897c559260e77cb14018235.
Reverted since this caused bug 740123.
... bug 750559 I mean.
Let's repoen the bug then. Did anybody look into *why* that might happen?
Created attachment 307535 [details] [review] Add the style context provider with FALLBACK priority Let's try again. This time we have two style providers. One of them is at APPLICATION priority and cannot be overridden by the theme. It sets the padding on VteTerminal widgets. The other one is at FALLBACK priority and is for colours. This can be overridden by the theme. This reverts commit bd86e7637d89a55941674756e3e223c82aee2305. --- I can't reliably reproduce this. It doesn't seem to happen ever with gnome-terminal 3.16.2 or even every time with 3.14.2-1, using affected vte versions. I got it maybe 1 in 10 times. After applying this patch (to always set padding on VteTerminal), I didn't get it at all. Can others who do know how to reproduce please try?
The patch in attachment 307535 [details] [review] seems to fix it for me, although I've only been using the Adwaita theme so far. Any recommendations for other themes to test it with? With the old style context provider patch, the first gnome-terminal window would have the wrong size most of the time (occasionally it would have the correct size) and subsequent gnome-terminal windows would always have the wrong size. With the new style context provider patch, I have tested opening Debian's gnome-terminal 3.16.2-1 twenty times and the initial window and subsequent windows both opened with the correct size. I repeated the test another ten times with a "nearly git master" gnome-terminal (I reverted commit ba2ef8e000e2 for testing because it was incompatible with the Debian-installed "org.gnome.Terminal.gschema.xml") and again the initial and subsequent terminal windows opened with the correct size (which they didn't with the old patch).
Further to comment #10: With the old style context provider patch (for either vte master or Debian's libvte-2.91-0) and Debian's gnome-terminal 3.16.2-1, I can't reproduce the bug at all with the Albatross, Blackbird, Bluebird and Greybird themes, but could reproduce it very reliably with the Adwaita theme. For vte master with the new style context provider patch, I can't reproduce the bug with any of those themes.
I don't see why you need to split the css. It's using theme colours (@theme_{base,fg}_color) already, so doesn't that get taken from the theme?
If you want to use a colour other than those for the terminal then you can't. We style TerminalScreen and set background-color and color. Without a patch like this then the theme cannot do that since it is hardcoded in a way that themes can't override to use @theme_foo_color.
Can I commit this new patch?
If you want to change the terminal colours, you can already do this by installing a gsettings override for gnome-terminal. Why is it necessary to provide another way? (Also, have you analysed why the original patch failed to set the border, and why the same probelm wouldn't happen for the colours, with the new patch?)
How? And why is it good for theme authors to have to provide a special method for the terminal instead of theming it like they can do with everything else? I would guess that vte is relying on the padding being 1, 1, 1, 1 somehow and so it doesn't support themes setting this to anything else. So here we don't make the padding overridable, but just the colours.
*** Bug 778431 has been marked as a duplicate of this bug. ***
Ok, here is my suggestion from #778431 Using GTK_STYLE_PROVIDER_PRIORITY_APPLICATION for the padding and GTK_STYLE_PROVIDER_PRIORITY_THEME for bg and fg color. In result gnome-terminal or mate-terminal use your proposed colors if nothing is set in gtk+ theme or can use a theme setting if available in themes. Ie. https://github.com/mate-desktop/mate-themes/blob/master/desktop-themes/Blue-Submarine/gtk-3.0/mate-applications.css#L897 And many other themes. It looks like that Ubuntu already patched vte to allow that as there or in linuxmint theme settings will used. Please allow this for other distros too. I do not maintain vte in fedora so i can't patch it as upstream. And with current version in fedora your proposed colors are used if i mark the checkbox 'use theme settings' in gnome or mate-terminal.
Created attachment 345511 [details] [review] Allow themes to set a bg and fg color for vte-terminal
err typo...i meant I do not maintain vte in fedora so i can't patch it as downstream.
Any response to the technical issue in comment 15 ?
Any news? Patch is in ubuntu-16.10 so i am in doubt that anyone will test it more. And i can't reproduce https://bugzilla.gnome.org/show_bug.cgi?id=750559 with fedora-24/25/26.
(In reply to Christian Persch from comment #21) > Any response to the technical issue in comment 15 ? Well?
It's confirmed here. https://bugzilla.gnome.org/show_bug.cgi?id=750559#c17
I don't see it? > (Also, have you analysed why the original patch failed to set the border, and ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > why the same probelm wouldn't happen for the colours, with the new patch?) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
https://bugzilla.gnome.org/show_bug.cgi?id=750559#c17 (In reply to Iain Lane from comment #13) > Can people who were able to reproduce reliably (I couldn't...) try the new > patch on bug #740123? I have this problem on Ubuntu Gnome, which still includes the old version of the patch. Replacing it with the new version of the patch fixes it for me. This is a confirmation for me. But do what you want, i am given up to wait for a simple fix which doesn't hurt.