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 720533 - Large icons in the list view
Large icons in the list view
Status: RESOLVED FIXED
Product: gnome-documents
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GNOME documents maintainer(s)
GNOME documents maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2013-12-16 15:20 UTC by Allan Day
Modified: 2014-02-05 17:01 UTC
See Also:
GNOME target: ---
GNOME version: 3.11/3.12


Attachments
screenshot (297.70 KB, image/png)
2013-12-16 15:20 UTC, Allan Day
  Details
documents: Having a thumbPath does not mean thumbnailing had succeeded (1.04 KB, patch)
2013-12-24 16:36 UTC, Debarshi Ray
reviewed Details | Review
documents: Add debug messages for failing to use a thumbnail file (1.29 KB, patch)
2013-12-24 16:40 UTC, Debarshi Ray
committed Details | Review
documents: Clear the thumbnail path if the thumbnail is invalid (1.69 KB, patch)
2014-02-05 13:20 UTC, Debarshi Ray
committed Details | Review
documents: Reduce the number of thumbnailing invariants (1.89 KB, patch)
2014-02-05 13:41 UTC, Debarshi Ray
committed Details | Review

Description Allan Day 2013-12-16 15:20:40 UTC
Created attachment 264291 [details]
screenshot

Some items have very large icons in the documents list - around 120x120px. It looks bad.
Comment 1 Debarshi Ray 2013-12-24 12:13:16 UTC
Thankfully not a regression. This has been around for a while -- atleast since F19 / 3.8 days.

It only happens when you switch from the grid to the list view while the application is running. Does not happen if you start it in list mode.
Comment 2 Debarshi Ray 2013-12-24 16:36:21 UTC
Created attachment 264846 [details] [review]
documents: Having a thumbPath does not mean thumbnailing had succeeded
Comment 3 Debarshi Ray 2013-12-24 16:40:29 UTC
Created attachment 264847 [details] [review]
documents: Add debug messages for failing to use a thumbnail file
Comment 4 Cosimo Cecchi 2014-01-16 20:43:06 UTC
Review of attachment 264847 [details] [review]:

Looks good
Comment 5 Cosimo Cecchi 2014-01-16 20:45:07 UTC
Review of attachment 264846 [details] [review]:

I'd rather we cleared out the thumbnail path entirely if we failed to thumbnail. At least that's what other code assumes if I remember correctly.
Maybe we could also take this as an occasion to clean up that code and use the newly introduced G_FILE_ATTRIBUTE_THUMBNAIL_IS_VALID feature in GIO?
Comment 6 Debarshi Ray 2014-02-05 13:20:05 UTC
(In reply to comment #5)
> Review of attachment 264846 [details] [review]:
> 
> I'd rather we cleared out the thumbnail path entirely if we failed to
> thumbnail. At least that's what other code assumes if I remember correctly.

Ok

> Maybe we could also take this as an occasion to clean up that code and use the
> newly introduced G_FILE_ATTRIBUTE_THUMBNAIL_IS_VALID feature in GIO?

Ofcourse, but I want to do it in a separate patch so that I can backport the fix to older releases.
Comment 7 Debarshi Ray 2014-02-05 13:20:39 UTC
Created attachment 268171 [details] [review]
documents: Clear the thumbnail path if the thumbnail is invalid
Comment 8 Debarshi Ray 2014-02-05 13:41:55 UTC
Created attachment 268174 [details] [review]
documents: Reduce the number of thumbnailing invariants

A little cleanup.
Comment 9 Cosimo Cecchi 2014-02-05 16:38:16 UTC
Review of attachment 268174 [details] [review]:

OK
Comment 10 Cosimo Cecchi 2014-02-05 16:52:08 UTC
Review of attachment 268171 [details] [review]:

This looks good to me too. I wonder if shouldn't record the fact that we failed to thumbnail this specific document somewhere, to avoid retrying ad-infinitum later, but it's probably OK for now.
Comment 11 Debarshi Ray 2014-02-05 16:58:32 UTC
(In reply to comment #10)
> Review of attachment 268171 [details] [review]:
> 
> I wonder if shouldn't record the fact that we failed
> to thumbnail this specific document somewhere, to avoid retrying ad-infinitum

refreshIcon already looks at this._failedThumbnailing and bails out if it had failed before. Or did you mean something else?
Comment 12 Debarshi Ray 2014-02-05 17:00:47 UTC
Comment on attachment 268171 [details] [review]
documents: Clear the thumbnail path if the thumbnail is invalid

Thanks for the review!
Comment 13 Debarshi Ray 2014-02-05 17:01:56 UTC
Let's use a separate bug for G_FILE_ATTRIBUTE_THUMBNAIL_IS_VALID and further improvements.

This problem has been fixed in our software repository. The fix will go into the next software release. Thank you for your bug report.