GNOME Bugzilla – Bug 741453
Uses the screensaver background in place of the regular one
Last modified: 2015-09-08 16:46:12 UTC
If the screensaver background (org.gnome.desktop.screensaver → picture-uri) is set to a XML file, it is used in place of the regular background. If the screensaver background is set to a regular file, the bug doesn’t happen. I’m guessing some caching mechanism is doing it wrong.
Correction: the bug only happens if *both* backgrounds are XML. Note that changing them after the shell startup will get correct results, but at the next restart of the shell the bug shows up again.
Created attachment 292627 [details] [review] Fix the race condition
Review of attachment 292627 [details] [review]: ::: js/ui/background.js @@ +156,3 @@ onLoaded: null }); + if (this._animations[params.filename]) { Not sure keying this off filename is a good idea. It means the animations list will grow over time without bound. I think I would instead key it off the schema name in the same way backgroundSources are handled. It does mean the xml file will get parsed twice if both screensaver and normal background are set to the same value, but i don't think that's that big of a deal.
Created attachment 292631 [details] [review] Patch for GNOME 3.14 Here you go.
Created attachment 292632 [details] [review] Patch for master
Review of attachment 292632 [details] [review]: commit message still says "based on filename" ::: js/ui/background.js @@ +166,3 @@ onLoaded: null }); + if (this._animations[params.settingsSchema] && _fileEqual0(this._animations[params.settingsSchema].filename, params.file)) { Animation object calls it "file" not "filename" @@ -404,3 @@ - _loadAnimation: function(file) { - this._cache.getAnimation({ file: file, - onLoaded: Lang.bind(this, function(animation) { spacing is weird here before you touched it. mind doing a prerequisite commit that fixes the spacing first?
Created attachment 292637 [details] [review] Fix indentation
Created attachment 292638 [details] [review] Patch for master
Created attachment 292639 [details] [review] Patch for GNOME 3.14 And voilà.
Review of attachment 292637 [details] [review]: cool
Review of attachment 292638 [details] [review]: seems right (assuming it's tested)
Review of attachment 292639 [details] [review]: ::: js/ui/background.js @@ +397,1 @@ onLoaded: Lang.bind(this, function(animation) { would prefer if you pushed the fix indentation patch too.
(In reply to comment #11) > seems right (assuming it's tested) I’m running 3.14 so I can’t vouch for master. (In reply to comment #12) > would prefer if you pushed the fix indentation patch too. Will do.
Review of attachment 292638 [details] [review]: ::: js/ui/background.js @@ +165,2 @@ params = Params.parse(params, { file: null, onLoaded: null }); You are missing the default for settingsSchema here
I pushed both patches to master including darkxt's comment in #14 Feel free to close the bug report now.
indentation fix: 785c90f4b85478b690de71c4448f640a27425c41 background race fix: 8a4c86263362c0b2d2f87b21b8f783ccc5690bf8