Page MenuHomePhabricator

Blurry image thumbnails accompany article suggestions
Closed, ResolvedPublic


Via Beta features:
When you visit the page image thumbnails for the suggested results are blurry

77WcPYL.png (344×553 px, 94 KB)

Event Timeline

Jdlrobson raised the priority of this task from to Needs Triage.
Jdlrobson updated the task description. (Show Details)
Jdlrobson added a project: RelatedArticles.
Jdlrobson added a subscriber: Jdlrobson.
Prtksxna set Security to None.
Prtksxna removed a subscriber: Prtksxna.
jhobs triaged this task as Medium priority.Jan 18 2016, 6:13 PM

Yep, they look nasty because they're blown up. The thumbnails requested are 53px, when the width of the div is 80px. Just request the 80px. Sure, that's 2kb extra per image, but they're loaded async and this is the desktop site, it's nothing compared to the weight of in-article thumbs.

What I see (53px):

Screenshot 2016-02-08 13.39.55.png (139×867 px, 63 KB)

What it would look like with 80px:

Screenshot 2016-02-08 13.40.34.png (148×868 px, 65 KB)

Interestingly, the PHP image in the middle is already 80px. I suspect some incorrect logic in how cropping is handled. If the image is in portrait mode, regardless of its height, 80px width is all you need for the thumbnail. If the image is in landscape mode, then I guess you want a bigger width than 80px.

Anyway, you can see that for yourself on

Quick comment: the image has to be 160px, not 80px, to look ok on UHD monitors (e.g. Macbook Pro Retina displays).

Thanks @SSneg great point.
Hopefully someone can get this fixed this week. We've sadly been a bit short staffed the past month (2 active devs! ;))

dr0ptp4kt added a subscriber: dr0ptp4kt.

Moving to top of sprint 68 (i.e., to be pulled into sprint 67 once other things done or if there's idle time).

@dr0ptp4kt I purposely pushed this in to I as it keeps being punted into next sprint and we've now had 2 reports on the beta features page. It's a small fix and I'll like us to get to it this sprint, given all the other tasks are large and it would be useful to have some smaller tasks for the team to work on when they have spare time to avoid burn out.

We are requesting pithumbsize = 80 and PageImages is returning images where the height is 80px but the width is less than 80px.

Ideally pithumbsize should either apply to both width and height or should allow us to specify which to use.

Change 276394 had a related patch set uploaded (by Jhobs):
Allow specification of cover attribute for thumbnails

Change 276396 had a related patch set uploaded (by Jhobs):
Fix blurry images

Could all the team please input on the discussion in ?
Thank you.

Jdlrobson changed the task status from Open to Stalled.Mar 24 2016, 8:29 PM

The patch has two issues

  1. What to name the parameter
  2. Whether it is acceptable to run transform twice for the % of cases it impacts or whether we should explore updating the transform function in includes/filerepo/file/File.php
jhobs changed the task status from Stalled to Open.Mar 31 2016, 5:27 PM

Since there hasn't been much discussion on the patch in a while, I've changed the attribute to be named "thumbmode" and think that's a fair solution for now. If we want to eventually update the transform function, I can add in a FIXME.

In fact, I'm just gonna add in the FIXME now.

As a short term measure, let's just request a larger thumbnail to reduce the likelihood of the image being too small.
I'm worried that every day we leave this, opinion of this extension is impacted and it's not clear if the overhead in the better solution is acceptable right now. I have captured the more generic problem in a separate task.

Change 282423 had a related patch set uploaded (by Jhobs):
Temporary fix for blurry images

dr0ptp4kt lowered the priority of this task from Medium to Low.Apr 11 2016, 5:54 PM

Change 282423 merged by jenkins-bot:
Temporary fix for blurry images

Looks good to me:
Compare before and after:

Screen Shot 2016-04-14 at 3.51.13 PM.png (118×437 px, 28 KB)

Screen Shot 2016-04-14 at 3.51.24 PM.png (132×424 px, 26 KB)

Note: This was in previous sprint and for some reason didn't get copied over to the next sprint so I've added the "Unplanned-Sprint-Work" tag. If its omission was accidental (looking at you @dr0ptp4kt) we may want to remove that tag. cc @MBinder_WMF

Seems accidental, but I leave it up to @dr0ptp4kt

Change 276394 abandoned by Jhobs:
Allow specification of thumbmode attribute for thumbnails

Per T132132#2225443