Page MenuHomePhabricator

MediaViewer downloads high-res image twice if original is a medium-size JPEG
Closed, ResolvedPublicBUG REPORT

Description

This is a regression caused by T414338 (currently on group1).

Steps to reproduce

For images where the original is relatively small (<1280px) the thumbnail is often the resolution as what Mediaviewer would show. It seems in this case we're failing to reuse the image and instead downloading it twice, thus wasting consumer bandwidth.

  1. https://en.wiktionary.org/wiki/banana.
    • Observe image request: https://upload.wikimedia.org/wikipedia/commons/d/de/Bananavarieties.jpg?utm_source=en.wiktionary.org&utm_campaign=parser&utm_content=thumbnail_unscaled
  2. Click to open in mediaviewer.
    • Observe second uncached download: https://upload.wikimedia.org/wikipedia/commons/d/de/Bananavarieties.jpg. This fails to use the browser cache due to not having the same query string.

It seems MediaViewer is both lucky and unlucky. It is lucky because it seems to not rely on the imageinfo API for thumbnails in cases where this is not needed (it does make an imageinfo API request for the EXIF data, but not for the thumbnail for this case). It is unlucky because something else is going wrong causing it to still strip or otherwise fail to reuse this image as-is.

Other information

It is totally fine (and expected) that MediaViewer uses images with provenance set to "imageinfo API" when it needs to use the imageinfo API to get a higher-resolution image. It does not need to match the Parser output or anything like that.

But, when the image from the Parser output is the same resolution as what it needs, it should be re-used from the page as-is instead of obtaining a different URL.

This happens most commonly when the original is medium-size (as the case above), and thus the original was used as the thumbnail. I might also happen in other cases such as when the viewport is relatively small, or when there is no higher thumbnail step available.

Between the logic MediaViewer already has for detecting if an image URL can be used statelessly without API call, and mw.util.parseImageUrl(), and mw.util.adjustThumbWidthForSteps(); it seems this should be easy to connect together. Or perhaps it is connected already but something is accidentally cleaning the URL and stripping it?

Event Timeline

Change #1275967 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/extensions/MultimediaViewer@master] Improve fetchThumbnail test and remove unused optional-sampleURL code

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

Change #1276086 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/extensions/MultimediaViewer@master] mmv.bootstrap: Avoid double download when thumb is unscaled original

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

Change #1275967 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Improve fetchThumbnail test and remove unused optional-sampleURL code

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

Change #1276086 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] mmv.bootstrap: Avoid double download when thumb is unscaled original

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