GNOME Bugzilla – Bug 791325
Gio handling of thumbnail:: attribute namespace causes inconsistent behavior
Last modified: 2017-12-08 15:20:07 UTC
As documented in a stackexchange question I posted earlier today, I discovered some special-casing in the Gio handling of thumbnail:: attributes that makes it very difficult to query the attributes individually. The Q&A is here, for anyone looking to play along at home: https://unix.stackexchange.com/questions/409086/how-do-i-query-individual-thumbnail-namespace-attributes-with-the-gio-command The problem occcurs when trying to query attributes like thumbnail::is-valid and thumbnail::failed individually, which will be unsuccessful unless the thumbnail::path attribute is also requested — even if there is no thumbnail::path attribute set on the file. Some examples using the `gio info` shell command: ================ # I can query individual attributes in other namespaces with no difficulty % gio info -a access::can-read test.png uri: file:///var/tmp/test.png attributes: access::can-read: TRUE # And I can list the thumbnail:: namespace attributes as a group % gio info -a thumbnail test.png uri: file:///var/tmp/test.png attributes: thumbnail::path: /home/ferd/.cache/thumbnails/large/0953b0d1f71f9066deee9ac3fb72243b.png thumbnail::is-valid: TRUE # But attempting to query thumbnail::is-valid alone is unsuccessful % gio info -a thumbnail::is-valid test.png uri: file:///var/tmp/test.png attributes: # Unless I also request thumbnail::path % gio info -a thumbnail::path,thumbnail::is-valid test.png uri: file:///var/tmp/test.png attributes: thumbnail::path: /home/ferd/.cache/thumbnails/large/0953b0d1f71f9066deee9ac3fb72243b.png thumbnail::is-valid: TRUE # This is true even if the thumbnail::path attribute is not present! % gio info -a thumbnail failed.mp3 uri: file:///var/tmp/failed.mp3 attributes: thumbnail::failed: TRUE thumbnail::is-valid: TRUE % gio info -a thumbnail::failed failed.mp3 uri: file:///var/tmp/failed.mp3 attributes: % gio info -a thumbnail::failed,thumbnail::path failed.mp3 uri: file:///var/tmp/failed.mp3 attributes: thumbnail::failed: TRUE ================ stackexchange user Sebastian, who answered my question there, was able to track this behavior down to glib/gio/glocalfileinfo.c:1977. ref: https://git.gnome.org/browse/glib/tree/gio/glocalfileinfo.c?h=2.54.2#n1977 The behavior seems extremely inconsistent with the handling of other file attributes, especially in requiring that a (nonexistent) thumbnail::path attribute be requested just to check the value of thumbnail::failed.
Created attachment 365183 [details] [review] gio: Fix querying of thumbnail attributes other than thumbnail::path The thumbnail attributes would previously only be set if thumbnail::path was included in the query — so querying for just thumbnail::is-valid would return no results. This fixes the behaviour of gio info -a thumbnail::is-valid ./some-file.png vs gio info -a thumbnail ./some-file.png The first command would previously list nothing. The second would previously list a thumbnail::path and thumbnail::is-valid. Signed-off-by: Philip Withnall <withnall@endlessm.com>
Yup, definitely a bug, thanks for reporting it. The above patch should fix things. Waiting for review from another GLib developer. Ondrej, fancy reviewing?
Review of attachment 365183 [details] [review]: Yes, this looks ok and I will do a similar patch for gvfs, it can be useful for debugging and we should be consistent, but I don't think it makes sense to obtain "is-valid", or "failed" separately without "path" in production, because the values are related together...
(In reply to Ondrej Holy from comment #3) > Review of attachment 365183 [details] [review] [review]: > > Yes, this looks ok and I will do a similar patch for gvfs, it can be useful > for debugging and we should be consistent, but I don't think it makes sense > to obtain "is-valid", or "failed" separately without "path" in production, > because the values are related together... Sure, CC me on the gvfs patch if you want review on it. Thanks for the fast review on this one!
Attachment 365183 [details] pushed as 62dece1 - gio: Fix querying of thumbnail attributes other than thumbnail::path
Also pushed to glib-2-54: 62dece198 (HEAD -> master, origin/master, origin/HEAD) gio: Fix querying of thumbnail attributes other than thumbnail::path
Thanks, I've already pushed the following commit and found that gvfs doesn't use "is-valid" attribute at all. It would be probably nice to implement this attribute in gvfs also, but I don't want to duplicate thumbnail-verify.c code. Thumbnailing seems somehow standardized, can't be provided public API for checking thumbnails? https://git.gnome.org/browse/gvfs/commit/?id=ebc61e107398704b69fb326b9fcece81a3be9e70
(In reply to Ondrej Holy from comment #7) > Thanks, I've already pushed the following commit and found that gvfs doesn't > use "is-valid" attribute at all. It would be probably nice to implement this > attribute in gvfs also, but I don't want to duplicate thumbnail-verify.c > code. Thumbnailing seems somehow standardized, can't be provided public API > for checking thumbnails? > > https://git.gnome.org/browse/gvfs/commit/ > ?id=ebc61e107398704b69fb326b9fcece81a3be9e70 I’d prefer to see the thumbnailing code move out to a library which accompanies the [spec](https://specifications.freedesktop.org/thumbnail-spec/latest/). However, that should be discussed in a separate bug. Please open one with details of the API you want exposed. Thanks.
It should be noted that these attributes are entirely outside the Thumbnail Spec. The spec only covers generation of the thumbnail PNG, plus metadata that should be stored in THAT file (as PNG tEXt blocks), but does not address any metadata being associated with the source file. AFAICT that's entirely a Gnome ThumbnailFactory implementation choice. (And I probably shouldn't be poking around in there, since there's no real standard for their format/use and no guarantees of future compatibility. But I happened to be peeking and noticed the weird behavior.)
Sure, but gio/thumbnail-verify.c 250 lines of code which follows the spec and would be useful to most implementers of the spec.