Page MenuHomePhabricator

Blurry image thumbnails accompany article suggestions
Closed, ResolvedPublic

Description

Via Beta features:
When you visit https://en.wikipedia.org/wiki/Diablo_(video_game) the page image thumbnails for the suggested results are blurry

Details

Related Gerrit Patches:
mediawiki/extensions/RelatedArticles : masterFix blurry images
mediawiki/extensions/PageImages : masterAllow specification of thumbmode attribute for thumbnails
mediawiki/extensions/RelatedArticles : masterTemporary fix for blurry images

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.
Restricted Application added subscribers: StudiesWorld, Aklapper. · View Herald TranscriptJan 13 2016, 7:54 PM
Prtksxna updated the task description. (Show Details)Jan 15 2016, 1:15 PM
Prtksxna set Security to None.
Prtksxna removed a subscriber: Prtksxna.
jhobs triaged this task as Medium priority.Jan 18 2016, 6:13 PM
Gilles added a subscriber: Gilles.Feb 8 2016, 12:43 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):

What it would look like with 80px:

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 https://en.wikipedia.org/wiki/Monkey_patch

SSneg added a subscriber: SSneg.Mar 2 2016, 8:48 PM

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

SSneg removed a subscriber: SSneg.Mar 2 2016, 8:48 PM
Jdlrobson added a subscriber: SSneg.Mar 2 2016, 9:02 PM

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.

jhobs claimed this task.Mar 7 2016, 5:02 PM

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

https://gerrit.wikimedia.org/r/276394

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

https://gerrit.wikimedia.org/r/276396

Could all the team please input on the discussion in https://gerrit.wikimedia.org/r/#/c/276394/3 ?
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

https://gerrit.wikimedia.org/r/282423

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

https://gerrit.wikimedia.org/r/282423

Jdlrobson closed this task as Resolved.Apr 14 2016, 2:52 PM

Looks good to me:
Compare before and after:

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

Reason:
Per T132132#2225443

https://gerrit.wikimedia.org/r/276394

Change 276396 abandoned by Jdlrobson:
Fix blurry images

Reason:
https://gerrit.wikimedia.org/r/#/c/276394/ was abandoned

https://gerrit.wikimedia.org/r/276396