GNOME Bugzilla – Bug 650971
User-Theme extension causes other extensions to ignore CSS rules
Last modified: 2011-06-15 19:58:18 UTC
When using the User-Theme extension, other extensions don't respect the CSS rules in their .css file anymore. Here's a screenshot with an extension before installing the user-theme extension:http://lh3.googleusercontent.com/_1QSDkzYY2vc/TdpoBypFL_I/AAAAAAAAEh0/BjQ6kq5RUWc/gnome-shell-weather-extension.png And here's a screenshot after installing user-theme extension: http://i.imgur.com/WhwoW.png This doesn't happen just with the extension in the screenshot but with all extensions that display something on the panel and require CSS. However, if you want to take a look at the css for the extension in the screenshot, see here: https://github.com/simon04/gnome-shell-extension-weather I've tried this on two Fedora 15 systems with the same result.
I can confirm this on archlinux with gnome shell 3.0.1 User-theme hides local extensions css class.
(In reply to comment #1) > I can confirm this on archlinux with gnome shell 3.0.1 > User-theme hides local extensions css class. Any idea why? The user-theme extension is 50 lines of code that doesnt do anything other than set the path to the shell-theme stylesheet [1]. Its interaction with gnome shell is limited to Main.setThemeStylesheet(_stylesheet). The problem must be in the shell or shell-toolkit [1] http://git.gnome.org/browse/gnome-shell-extensions/tree/extensions/user-theme/extension.js
@John: it could be, I'm not a developer so I don't really know. What's certain is that the user-theme extension triggers this somehow. (I've tried it on a fresh install too with just one extension that uses CSS, with and without the user-theme extension to make sure).
Without looking into it too deeply, this is what I think is happening: Extensions' stylesheets are added to the active theme. When the user-theme extension calls setThemeStylesheet(), the theme which holds the previously loaded extensions' stylesheets is replaced with another theme, which doesn't. I suspect extensions are loaded alphabetically, so z-extension's CSS will probably still work ...
(In reply to comment #4) > Without looking into it too deeply, this is what I think is happening: > > Extensions' stylesheets are added to the active theme. When the user-theme > extension calls setThemeStylesheet(), the theme which holds the previously > loaded extensions' stylesheets is replaced with another theme, which doesn't. > > I suspect extensions are loaded alphabetically, so z-extension's CSS will > probably still work ... Yeah that looks like it. The following two calls fight http://git.gnome.org/browse/gnome-shell/tree/js/ui/main.js#n399 http://git.gnome.org/browse/gnome-shell/tree/js/ui/extensionSystem.js#n136 themeContext.load_theme vs. themeContext.load_stylesheet
Same as bug 642876 ?
(In reply to comment #6) > Same as bug 642876 ? IMO, yes. Thats similar to the fix I was just hacking on right now. As a side note: I think moving the theme-name schema into g-s (bug 650701) would also be an appropriate part of this fix. Then the only supported method for theme changes would be the setting of that key (and should take the race out of theme changes while loading/overwriting extension stylesheets)
I agree with John. Part of the fix should be to move the theme-name schema into g-s.
(In reply to comment #7) > (In reply to comment #6) > > Same as bug 642876 ? > > IMO, yes. Thats similar to the fix I was just hacking on right now. > > As a side note: I think moving the theme-name schema into g-s (bug 650701) > would also be an appropriate part of this fix. Then the only supported method > for theme changes would be the setting of that key (and should take the race > out of theme changes while loading/overwriting extension stylesheets) How so? If the key changes, the previous theme is freed, and with it extensions' CSS. I honestly don't see much of a difference ...
(In reply to comment #9) > (In reply to comment #7) > > (In reply to comment #6) > > > Same as bug 642876 ? > > > > IMO, yes. Thats similar to the fix I was just hacking on right now. > > > > As a side note: I think moving the theme-name schema into g-s (bug 650701) > > would also be an appropriate part of this fix. Then the only supported method > > for theme changes would be the setting of that key (and should take the race > > out of theme changes while loading/overwriting extension stylesheets) > > How so? If the key changes, the previous theme is freed, and with it > extensions' CSS. I honestly don't see much of a difference ... Obviously the old css would still have to be stored and then re-added after a theme change. I was referring to the fact that doing so mid way through loading extensions containing said css seems icky. It would also be appreciated if the g-s team would give us this inch (and move the schema into g-s proper).
(In reply to comment #10) > Obviously the old css would still have to be stored and then re-added after a > theme change. I was referring to the fact that doing so mid way through loading > extensions containing said css seems icky. True, but as the proposed key could be changed from an extension, it wouldn't fix this problem (think of a theme packaged as extension, setting itself as default theme ...)
*** Bug 649742 has been marked as a duplicate of this bug. ***
> Part of the fix should be to move the theme-name schema into g-s. The primary reason for this is that the user-theme extension assumes that it is the only extension that uses the extension theme schema. However there are already a number of theme selector utilities such as gnome-tweak-tool and extensions such as ThemeSelector that use this schema and the number of users of this schema is only going to grow over time. The problem occurs when a user removes the user-theme extension using a package manager such as yum because the schema is also removed.
(In reply to comment #11) > (In reply to comment #10) > > Obviously the old css would still have to be stored and then re-added after a > > theme change. I was referring to the fact that doing so mid way through loading > > extensions containing said css seems icky. > > True, but as the proposed key could be changed from an extension, it wouldn't > fix this problem (think of a theme packaged as extension, setting itself as > default theme ...) Sure, and this is javascript so there are also approximately seventy bazillion ways developers can monkey patch the shell and break it nicely to their liking. I will be happy if fix way encouraged people to do correct things. Will you take a patch that fixes this and moves the schema into g-s or not?
> I will be happy if fix way encouraged people to do correct things. ^^^ the fixed way
*** Bug 651386 has been marked as a duplicate of this bug. ***
Created attachment 189086 [details] [review] StTheme: retrive the list of custom stylesheets and use it in loadTheme() Using the list of stylesheets loaded with st_theme_load_stylesheet(), one can build an StTheme that is completely identical to the previous one, except for one property (application-stylesheet). This allows rt and the user-theme extension to work while respecting the theming of other extensions.
(In reply to comment #17) > Created an attachment (id=189086) [details] [review] > StTheme: retrive the list of custom stylesheets and use it in loadTheme() > > Using the list of stylesheets loaded with st_theme_load_stylesheet(), > one can build an StTheme that is completely identical to the previous > one, except for one property (application-stylesheet). > This allows rt and the user-theme extension to work while respecting > the theming of other extensions. (In reply to comment #17) > Created an attachment (id=189086) [details] [review] > StTheme: retrive the list of custom stylesheets and use it in loadTheme() > > Using the list of stylesheets loaded with st_theme_load_stylesheet(), > one can build an StTheme that is completely identical to the previous > one, except for one property (application-stylesheet). > This allows rt and the user-theme extension to work while respecting > the theming of other extensions. Just tested this and it works for me here. Thankyou Giovanni.
Works for me also. Cascading order precedence maintained.
Ping? Is this OK to commit? I have now received a few personal emails from users telling me in varying degrees of politeness that user-theme extension has broken their GNOME3.
I agree that this fix should go in as soon as possible.
Works here as well, this really needs to go in! This is an official extension, and one of the most popular ones. Can't have this keep ruining so many other extensions.
Work for me too :)
Ping? Seriously.
Review of attachment 189086 [details] [review]: ::: js/ui/main.js @@ +402,3 @@ let theme = new St.Theme ({ application_stylesheet: cssStylesheet }); + + if (previousTheme) { Don't we want a check if previousTheme != theme too?
(In reply to comment #25) > Review of attachment 189086 [details] [review]: > > ::: js/ui/main.js > @@ +402,3 @@ > let theme = new St.Theme ({ application_stylesheet: cssStylesheet }); > + > + if (previousTheme) { > > Don't we want a check if previousTheme != theme too? That's not possible, theme is always a new object, and objects are compared by pointer in JS. (That's true even if constructor is overridden for the GObject, although gjs probably won't like that)
Comment on attachment 189086 [details] [review] StTheme: retrive the list of custom stylesheets and use it in loadTheme() OK.
Attachment 189086 [details] pushed as 2674d96 - StTheme: retrive the list of custom stylesheets and use it in loadTheme()