After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 702010 - gnome_bg_changes_with_time should return false if bg doesn't have a filename
gnome_bg_changes_with_time should return false if bg doesn't have a filename
Status: RESOLVED FIXED
Product: gnome-desktop
Classification: Core
Component: Background
3.8.x
Other Linux
: Normal normal
: ---
Assigned To: Desktop Maintainers
Desktop Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-06-11 13:35 UTC by Sebastien Bacher
Modified: 2013-06-11 16:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gnome-bg: if there is no filename the background is not a slideshow (730 bytes, patch)
2013-06-11 13:35 UTC, Sebastien Bacher
reviewed Details | Review
Slightly modified version that does not display warnings (797 bytes, patch)
2013-06-11 16:29 UTC, Sebastien Bacher
committed Details | Review
updated version with warning (767 bytes, patch)
2013-06-11 16:31 UTC, Sebastien Bacher
rejected Details | Review

Description Sebastien Bacher 2013-06-11 13:35:37 UTC
Created attachment 246521 [details] [review]
gnome-bg: if there is no filename the background is not a  slideshow

The GnomeBg object might only have colors and no image (that's happening for example in the gnome-control-center color previews), in which case the current code end up calling g_file_new_for_path() on NULL paths, which triggers warnings
Comment 1 Bastien Nocera 2013-06-11 13:44:49 UTC
Review of attachment 246521 [details] [review]:

That looks fine, can you fix the patch to include a link to the bug and the subject to fit on one line?
Comment 2 Sebastien Bacher 2013-06-11 16:29:31 UTC
Created attachment 246545 [details] [review]
Slightly modified version that does not display warnings

Hum, in fact the previous version still triggers a warning, do you consider it a programmer error to give a GnomeBG object without a filename/uri?

That's what the old gnome-control-center code was doing:
https://git.gnome.org/browse/gnome-control-center/tree/panels/background/bg-colors-source.c?h=gnome-3-4#n91

That variant of the patch just return FALSE without warning in this case
Comment 3 Sebastien Bacher 2013-06-11 16:31:26 UTC
Created attachment 246547 [details] [review]
updated version with warning

That variant raises a warning, if you prefer that (e.g consider that it's the caller job to check if there is a filename before calling that function)
Comment 4 Bastien Nocera 2013-06-11 16:34:53 UTC
Review of attachment 246547 [details] [review]:

Looks good.
Comment 5 Bastien Nocera 2013-06-11 16:35:45 UTC
Review of attachment 246545 [details] [review]:

Probably better if the callers are expecting this not to warn.
Comment 6 Bastien Nocera 2013-06-11 16:36:33 UTC
Review of attachment 246547 [details] [review]:

As per the last review, we'll go for the version that doesn't warn.
Comment 7 Sebastien Bacher 2013-06-11 16:38:37 UTC
Thanks, commit to trunk:
https://git.gnome.org/browse/gnome-desktop/commit/?id=e19df898281f83aa6db3d8fe57c720b29d98abee