GNOME Bugzilla – Bug 780191
Improved css provider file loading
Last modified: 2017-03-29 21:35:11 UTC
Created attachment 348171 [details] [review] Improved css file loading patch Different steps are made in order to load a css provider. This patches removes several lines by using gtk_css_provider_load_from_resource.
Review of attachment 348171 [details] [review]: Nice catch. Only one minor nitpick. There's something I wanted to do for quite some time: could you make this code generic to load CSS for any theme? So instead of: "/org/gnome/todo/theme/Adwaita.css" We'd have: "/org/gnome/todo/theme/%s.css" And it'd check at runtime the name of the current theme, and if we ship CSS for the current theme. If we do, load that CSS; otherwise, load Adwaita. What do you think? ::: src/gtd-application.c @@ +230,3 @@ + gtk_css_provider_load_from_resource (priv->provider, + "/org/gnome/todo/theme/Adwaita.css"); Can be in a single line.
Created attachment 348188 [details] [review] Load a css provider based on the current theme Changed the patch to load a css provider based on the current theme. On any error when loading it, errors when parsing, the file does not exist, etc.. it loads Adwaita theme. This patch also fixes the memory leak with css_file in case of any error when loading it (which should be very unlikely).
Review of attachment 348188 [details] [review]: Looks good.
Comment on attachment 348188 [details] [review] Load a css provider based on the current theme Thanks
I was thinking about the approach of checking if a resource exists by trying to open it with gtk_css_provider_load_from_file and then check the error parameter was the proper way of solve the problem, so I started looking at GTK code and I have discovered that error parameter does not exist anymore. I am trying to figure out a different approach to solve the problem so this will be valid for GTK 4.0 too. I have sent a mail to the gtk-devel-list in order to get an advice from someone more experienced.
Created attachment 348313 [details] [review] Fixed css provider file loading for GTK 4.0 Changed the way an external css provider file is loaded. In the first place, the code checks if the file exists using GFile functions and loads it if it does, otherwise it loads the Adwaita resource. The solution is not pefect though, as there is a race condition where the file may not exist after file exist check.
Review of attachment 348313 [details] [review]: This patch introduces a warning: 'warning: unused variable ‘error’ [-Wunused-variable]' Please remove that variable.
Created attachment 348552 [details] [review] Fixed css provider file loading for GTK 4.0 Updated css provider file loading patch by removing the unused error variable.
Thanks for the patches.