Fix: don't assume thumbnail URLs contain pixel size

Don't assume that thumbnail URLs contain a dimension delimiter of "px-".
Previously, thumbnail URLs always contained the width. e.g.:

  https://upload.wikimedia.org/wikipedia/commons/a/aa/100px-Red_Giant_Earth_warm.jpg

However, thumbnail URLs that actually point to the original are not
sizable:

  https://upload.wikimedia.org/wikipedia/commons/a/aa/Red_Giant_Earth_warm.jpg

These are provided, for example, when the thumbnail size requested is
larger than the original. There was code designed to handle this
scenario but it only applies when RESTBase and page preview thumbnail
sizes happen to be in sync. In other words, if RESTBase requests a large
thumbnail on behalf of page previews, and page previews only requested a
small thumbnail, the original may be unexpectedly provided. A
conditional is introduced in this patch to verify that "px-" is actually
detected. If it is not present, the original is used.

Bug: T187955
Change-Id: If4e29dd870aecd6d461cc8203f6576d1bb8844f2
This commit is contained in:
Stephen Niedzielski 2018-02-21 17:52:10 -06:00
parent 35daa2a689
commit e406d4556f
5 changed files with 51 additions and 2 deletions

View file

@ -38,7 +38,7 @@
"bundlesize": [
{
"path": "resources/dist/index.js",
"maxSize": "11.2KB"
"maxSize": "11.5KB"
}
]
}

Binary file not shown.

Binary file not shown.

View file

@ -118,6 +118,7 @@ function generateThumbnailData( thumbnail, original, thumbSize ) {
var parts = thumbnail.source.split( '/' ),
lastPart = parts[ parts.length - 1 ],
filename,
filenamePxIndex,
width,
height;
@ -126,7 +127,23 @@ function generateThumbnailData( thumbnail, original, thumbSize ) {
// makes this function resilient to the thumbnail not having the same
// extension as the original image, which is definitely the case for SVG's
// where the thumbnail's extension is .svg.png.
filename = lastPart.substr( lastPart.indexOf( 'px-' ) + 3 );
filenamePxIndex = lastPart.indexOf( 'px-' );
if ( filenamePxIndex === -1 ) {
// The thumbnail size is not customizable. Presumably, RESTBase requested a
// width greater than the original and so MediaWiki returned the original's
// URL instead of a thumbnail compatible URL. An original URL does not have
// a "thumb" path, e.g.:
//
// https://upload.wikimedia.org/wikipedia/commons/a/aa/Red_Giant_Earth_warm.jpg
//
// Instead of:
//
// https://upload.wikimedia.org/wikipedia/commons/thumb/a/aa/Red_Giant_Earth_warm.jpg/512px-Red_Giant_Earth_warm.jpg
//
// Use the original.
return original;
}
filename = lastPart.substr( filenamePxIndex + 3 );
// Scale the thumbnail's largest dimension.
if ( thumbnail.width > thumbnail.height ) {

View file

@ -75,6 +75,25 @@ var DEFAULT_CONSTANTS = {
dir: 'ltr',
timestamp: '2017-02-17T22:29:56Z'
},
// As above, a no "px-" thumbnail is provided which is not customizable.
RESTBASE_RESPONSE_WITH_NO_PX_IMAGE = {
title: 'Barack Obama',
extract: 'Barack Hussein Obama II born August 4, 1961) ...',
thumbnail: {
source: 'https://upload.wikimedia.org/wikipedia/commons/8/8d/President_Barack_Obama.jpg',
width: 800,
height: 1000
},
originalimage: {
source: 'https://upload.wikimedia.org/wikipedia/commons/8/8d/President_Barack_Obama.jpg',
width: 800,
height: 1000
},
lang: 'en',
dir: 'ltr',
timestamp: '2017-01-30T10:17:41Z',
description: '44th President of the United States of America'
},
RESTBASE_RESPONSE_WITH_LANDSCAPE_IMAGE = {
title: 'Landscape',
extract: 'Landscape',
@ -204,6 +223,19 @@ QUnit.test( 'RESTBase gateway doesn\'t stretch thumbnails', function ( assert )
);
} );
QUnit.test( 'RESTBase gateway handles thumbnail URLs with missing dimensions', function ( assert ) {
var model,
gateway = createRESTBaseGateway();
model = gateway.convertPageToModel(
RESTBASE_RESPONSE_WITH_NO_PX_IMAGE, 300, provideParsedExtract );
assert.equal(
model.thumbnail.source,
'https://upload.wikimedia.org/wikipedia/commons/8/8d/President_Barack_Obama.jpg',
'If restbase handles missing image dimensions in thumbnail URLs'
);
} );
QUnit.test( 'RESTBase gateway handles awkward thumbnails', function ( assert ) {
var gateway = createRESTBaseGateway(),
response,