From 4672f2eb57c2d5970f2c0d98b673afc43e5a9952 Mon Sep 17 00:00:00 2001 From: Simon Legner Date: Tue, 28 May 2024 11:07:27 +0200 Subject: [PATCH] ThumbnailInfo: support multi lingual SVG Bug: T208564 Change-Id: Icf082ca0dc94bc4739fd31b795de76f8a0083b70 --- bundlesize.config.json | 2 +- resources/mmv/mmv.js | 16 +-- .../provider/mmv.provider.ThumbnailInfo.js | 8 +- resources/mmv/ui/mmv.ui.metadataPanel.js | 2 +- resources/mmv/ui/mmv.ui.stripeButtons.js | 17 +++- tests/qunit/mmv/mmv.test.js | 4 +- .../mmv.provider.ThumbnailInfo.test.js | 97 +++++++++++-------- .../qunit/mmv/ui/mmv.ui.metadataPanel.test.js | 3 +- .../qunit/mmv/ui/mmv.ui.stripeButtons.test.js | 8 +- 9 files changed, 98 insertions(+), 59 deletions(-) diff --git a/bundlesize.config.json b/bundlesize.config.json index 2e5e8881f..984f7e7c5 100644 --- a/bundlesize.config.json +++ b/bundlesize.config.json @@ -2,7 +2,7 @@ "modules": [ { "resourceModule": "mmv", - "maxSize": "26.6 kB" + "maxSize": "26.9 kB" }, { "resourceModule": "mmv.ui.restriction", diff --git a/resources/mmv/mmv.js b/resources/mmv/mmv.js index b2ab562c5..a0dc1037b 100644 --- a/resources/mmv/mmv.js +++ b/resources/mmv/mmv.js @@ -206,7 +206,7 @@ class MultimediaViewer { this.currentIndex = image.index; - this.currentImageFileTitle = image.filePageTitle; + this.currentImage = image; if ( !this.isOpen ) { $( document ).trigger( $.Event( 'mmv-setup-overlay' ) ); @@ -632,9 +632,9 @@ class MultimediaViewer { guessing = true; thumbnailPromise = this.guessedThumbnailInfoProvider.get( fileTitle, sampleUrl, width, originalWidth, originalHeight - ).then( null, () => this.thumbnailInfoProvider.get( fileTitle, width ) ); + ).then( null, () => this.thumbnailInfoProvider.get( fileTitle, sampleUrl, width ) ); } else { - thumbnailPromise = this.thumbnailInfoProvider.get( fileTitle, width ); + thumbnailPromise = this.thumbnailInfoProvider.get( fileTitle, sampleUrl, width ); } imagePromise = thumbnailPromise.then( ( thumbnail ) => this.imageProvider.get( thumbnail.url ) ); @@ -644,7 +644,7 @@ class MultimediaViewer { // As a side effect this introduces an extra (harmless) retry of a failed thumbnailInfoProvider.get call // because thumbnailInfoProvider.get is already called above when guessedThumbnailInfoProvider.get fails. imagePromise = imagePromise - .then( null, () => this.thumbnailInfoProvider.get( fileTitle, width ) + .then( null, () => this.thumbnailInfoProvider.get( fileTitle, sampleUrl, width ) .then( ( thumbnail ) => this.imageProvider.get( thumbnail.url ) ) ); } @@ -748,7 +748,7 @@ class MultimediaViewer { */ setTitle() { // update title after route change, see T225387 - document.title = this.createDocumentTitle( this.currentImageFileTitle ); + document.title = this.createDocumentTitle( this.currentImage && this.currentImage.filePageTitle ); } /** @@ -813,13 +813,13 @@ class MultimediaViewer { } ).on( 'mmv-resize-end.mmvp', () => { this.resize( this.ui ); } ).on( 'mmv-request-thumbnail.mmvp', ( e, size ) => { - if ( this.currentImageFileTitle ) { - return this.thumbnailInfoProvider.get( this.currentImageFileTitle, size ); + if ( this.currentImage && this.currentImage.filePageTitle ) { + return this.thumbnailInfoProvider.get( this.currentImage.filePageTitle, this.currentImage.src, size ); } else { return $.Deferred().reject(); } } ).on( 'mmv-viewfile.mmvp', () => { - this.imageInfoProvider.get( this.currentImageFileTitle ).done( ( imageInfo ) => { + this.imageInfoProvider.get( this.currentImage.filePageTitle ).done( ( imageInfo ) => { document.location = imageInfo.url; } ); } ); diff --git a/resources/mmv/provider/mmv.provider.ThumbnailInfo.js b/resources/mmv/provider/mmv.provider.ThumbnailInfo.js index 74310fe8e..a3d742a32 100644 --- a/resources/mmv/provider/mmv.provider.ThumbnailInfo.js +++ b/resources/mmv/provider/mmv.provider.ThumbnailInfo.js @@ -42,12 +42,15 @@ class ThumbnailInfo extends Api { * is smaller). * * @param {mw.Title} file + * @param {string} sampleUrl a thumbnail URL for the same file (but with different size). * @param {number} width thumbnail width in pixels * @param {number} height thumbnail height in pixels * @return {jQuery.Promise.} */ - get( file, width, height ) { - const cacheKey = `${ file.getPrefixedDb() }|${ width || '' }|${ height || '' }`; + get( file, sampleUrl, width, height ) { + const match = sampleUrl.match( /(lang|page)([\d\-a-z]+)-(\d+)px/ ); // multi lingual SVG or PDF page + const iiurlparam = match ? `${ match[ 1 ] }${ match[ 2 ] }-${ width }px` : undefined; + const cacheKey = [ file.getPrefixedDb(), width || '', height || '', iiurlparam || '' ].join(); return this.getCachedPromise( cacheKey, () => this.apiGetWithMaxAge( { formatversion: 2, @@ -55,6 +58,7 @@ class ThumbnailInfo extends Api { prop: 'imageinfo', titles: file.getPrefixedDb(), iiprop: 'url', + iiurlparam, iiurlwidth: width, iiurlheight: height } ).then( ( data ) => this.getQueryPage( data ) ).then( ( page ) => { diff --git a/resources/mmv/ui/mmv.ui.metadataPanel.js b/resources/mmv/ui/mmv.ui.metadataPanel.js index aa26de187..7683403e2 100644 --- a/resources/mmv/ui/mmv.ui.metadataPanel.js +++ b/resources/mmv/ui/mmv.ui.metadataPanel.js @@ -739,7 +739,7 @@ class MetadataPanel extends UiElement { this.$datetimeUpdatedLi.removeClass( 'empty' ); } - this.buttons.set( imageData ); + this.buttons.set( image, imageData ); this.description.set( imageData.description, image.caption ); this.setLicense( imageData.license, imageData.descriptionUrl ); diff --git a/resources/mmv/ui/mmv.ui.stripeButtons.js b/resources/mmv/ui/mmv.ui.stripeButtons.js index e2c6acc74..22976ba92 100644 --- a/resources/mmv/ui/mmv.ui.stripeButtons.js +++ b/resources/mmv/ui/mmv.ui.stripeButtons.js @@ -44,16 +44,29 @@ class StripeButtons extends UiElement { /** * @inheritdoc + * @param {LightboxImage} image * @param {ImageModel} imageInfo */ - set( imageInfo ) { + set( image, imageInfo ) { + const match = image && image.src ? + image.src.match( /(lang|page)([\d\-a-z]+)-(\d+)px/ ) : // multi lingual SVG or PDF page + null; + const params = {}; + if ( match ) { + params[ match[ 1 ] ] = match[ 2 ]; + } + let descriptionUrl = imageInfo.descriptionUrl; let isCommons = String( descriptionUrl ).includes( '//commons.wikimedia.org/' ); if ( imageInfo.pageID ) { // The file has a local description page, override the description URL - descriptionUrl = imageInfo.title.getUrl(); + descriptionUrl = imageInfo.title.getUrl( params ); isCommons = false; + } else { + const parsedUrl = new mw.Uri( descriptionUrl ); + parsedUrl.extend( params ); + descriptionUrl = parsedUrl.toString(); } this.$descriptionPage.empty() diff --git a/tests/qunit/mmv/mmv.test.js b/tests/qunit/mmv/mmv.test.js index 7849ab613..aa27e443b 100644 --- a/tests/qunit/mmv/mmv.test.js +++ b/tests/qunit/mmv/mmv.test.js @@ -460,7 +460,7 @@ const { MultimediaViewerBootstrap } = require( 'mmv.bootstrap' ); const originalWidth = 50; const viewer = getMultimediaViewer(); - viewer.thumbnailInfoProvider.get = function ( fileTitle, width ) { + viewer.thumbnailInfoProvider.get = function ( fileTitle, sampleUrl, width ) { assert.strictEqual( width, expectedWidth ); return $.Deferred().reject(); }; @@ -603,7 +603,7 @@ const { MultimediaViewerBootstrap } = require( 'mmv.bootstrap' ); const oldDocumentTitle = document.title; this.sandbox.stub( mw.loader, 'using' ).returns( $.Deferred().resolve( viewer ) ); - viewer.currentImageFileTitle = title; + viewer.currentImage = { filePageTitle: title }; bootstrap.setupEventHandlers(); viewer.setTitle(); diff --git a/tests/qunit/mmv/provider/mmv.provider.ThumbnailInfo.test.js b/tests/qunit/mmv/provider/mmv.provider.ThumbnailInfo.test.js index 8f8f672ea..bcf4542de 100644 --- a/tests/qunit/mmv/provider/mmv.provider.ThumbnailInfo.test.js +++ b/tests/qunit/mmv/provider/mmv.provider.ThumbnailInfo.test.js @@ -84,55 +84,76 @@ const { ThumbnailInfo } = require( 'mmv' ); assert.true( thumbnailInfoProvider instanceof ThumbnailInfo ); } ); - QUnit.test( 'ThumbnailInfo get test', ( assert ) => { - let apiCallCount = 0; - const api = { get: function () { - apiCallCount++; - return $.Deferred().resolve( { - query: { - pages: [ - { - ns: 6, - title: 'File:Stuff.jpg', - missing: true, - imagerepository: 'shared', - imageinfo: [ - { - thumburl: 'https://upload.wikimedia.org/wikipedia/commons/thumb/1/19/Stuff.jpg/51px-Stuff.jpg', - thumbwidth: 95, - thumbheight: 200, - url: 'https://upload.wikimedia.org/wikipedia/commons/1/19/Stuff.jpg', - descriptionurl: 'https://commons.wikimedia.org/wiki/File:Stuff.jpg' - } - ] - } - ] - } - } ); - } }; + QUnit.test( 'ThumbnailInfo get test', function ( assert ) { + const api = { get: this.sandbox.stub().returns( $.Deferred().resolve( { + query: { + pages: [ + { + ns: 6, + title: 'File:Stuff.jpg', + missing: true, + imagerepository: 'shared', + imageinfo: [ + { + thumburl: 'https://upload.wikimedia.org/wikipedia/commons/thumb/1/19/Stuff.jpg/51px-Stuff.jpg', + thumbwidth: 95, + thumbheight: 200, + url: 'https://upload.wikimedia.org/wikipedia/commons/1/19/Stuff.jpg', + descriptionurl: 'https://commons.wikimedia.org/wiki/File:Stuff.jpg' + } + ] + } + ] + } + } ) ) }; const file = new mw.Title( 'File:Stuff.jpg' ); const thumbnailInfoProvider = new ThumbnailInfo( api ); - return thumbnailInfoProvider.get( file, 100 ).then( ( thumbnail ) => { + return thumbnailInfoProvider.get( file, '', 100 ).then( ( thumbnail ) => { assert.strictEqual( thumbnail.url, 'https://upload.wikimedia.org/wikipedia/commons/thumb/1/19/Stuff.jpg/51px-Stuff.jpg', 'URL is set correctly' ); assert.strictEqual( thumbnail.width, 95, 'actual width is set correctly' ); assert.strictEqual( thumbnail.height, 200, 'actual height is set correctly' ); } ).then( () => { - assert.strictEqual( apiCallCount, 1 ); + assert.strictEqual( api.get.calledOnce, true ); + assert.deepEqual( api.get.getCall( 0 ).args[ 0 ], { + formatversion: 2, + action: 'query', + prop: 'imageinfo', + titles: 'File:Stuff.jpg', + iiprop: 'url', + iiurlheight: undefined, + iiurlparam: undefined, + iiurlwidth: 100 + } ); // call the data provider a second time to check caching - return thumbnailInfoProvider.get( file, 100 ); + return thumbnailInfoProvider.get( file, '', 100 ); } ).then( () => { - assert.strictEqual( apiCallCount, 1 ); + assert.strictEqual( api.get.calledOnce, true ); // call a third time with different size to check caching - return thumbnailInfoProvider.get( file, 110 ); + return thumbnailInfoProvider.get( file, '', 110 ); } ).then( () => { - assert.strictEqual( apiCallCount, 2 ); + assert.strictEqual( api.get.calledTwice, true ); // call it again, with a height specified, to check caching - return thumbnailInfoProvider.get( file, 110, 100 ); + return thumbnailInfoProvider.get( file, '', 110, 100 ); } ).then( () => { - assert.strictEqual( apiCallCount, 3 ); + assert.strictEqual( api.get.calledThrice, true ); + // call it again, with multi lingual SVG + const file2 = new mw.Title( 'File:Multilingual_SVG_example.svg' ); + const sampleUrl = 'https://upload.wikimedia.org/wikipedia/commons/thumb/1/1e/Multilingual_SVG_example.svg/langit-120px-Multilingual_SVG_example.svg.png'; + return thumbnailInfoProvider.get( file2, sampleUrl, 100 ); + } ).then( () => { + assert.deepEqual( api.get.getCall( api.get.callCount - 1 ).args[ 0 ], { + action: 'query', + formatversion: 2, + iiprop: 'url', + iiurlheight: undefined, + iiurlparam: 'langit-100px', + iiurlwidth: 100, + prop: 'imageinfo', + titles: 'File:Multilingual_SVG_example.svg' + } ); } ); } ); @@ -144,7 +165,7 @@ const { ThumbnailInfo } = require( 'mmv' ); const done = assert.async(); const thumbnailInfoProvider = new ThumbnailInfo( api ); - thumbnailInfoProvider.get( file, 100 ).fail( () => { + thumbnailInfoProvider.get( file, '', 100 ).fail( () => { assert.true( true, 'promise rejected when no data is returned' ); done(); } ); @@ -166,7 +187,7 @@ const { ThumbnailInfo } = require( 'mmv' ); const done = assert.async(); const thumbnailInfoProvider = new ThumbnailInfo( api ); - thumbnailInfoProvider.get( file, 100 ).fail( () => { + thumbnailInfoProvider.get( file, '', 100 ).fail( () => { assert.true( true, 'promise rejected when imageinfo is missing' ); done(); } ); @@ -190,7 +211,7 @@ const { ThumbnailInfo } = require( 'mmv' ); const done = assert.async(); const thumbnailInfoProvider = new ThumbnailInfo( api ); - thumbnailInfoProvider.get( file ).fail( ( errorMessage ) => { + thumbnailInfoProvider.get( file, '' ).fail( ( errorMessage ) => { assert.strictEqual( errorMessage, 'file does not exist: File:Stuff.jpg', 'error message is set correctly for missing file' ); done(); @@ -216,7 +237,7 @@ const { ThumbnailInfo } = require( 'mmv' ); const done = assert.async(); const thumbnailInfoProvider = new ThumbnailInfo( api ); - thumbnailInfoProvider.get( file, 100 ).fail( () => { + thumbnailInfoProvider.get( file, '', 100 ).fail( () => { assert.true( true, 'promise rejected when thumbnail info is missing' ); done(); } ); diff --git a/tests/qunit/mmv/ui/mmv.ui.metadataPanel.test.js b/tests/qunit/mmv/ui/mmv.ui.metadataPanel.test.js index 85aff9c69..7adcd8d5f 100644 --- a/tests/qunit/mmv/ui/mmv.ui.metadataPanel.test.js +++ b/tests/qunit/mmv/ui/mmv.ui.metadataPanel.test.js @@ -112,7 +112,8 @@ QUnit.test( '.setImageInfo()', function ( assert ) { ); const title = 'Foo bar'; const image = { - filePageTitle: mw.Title.newFromText( 'File:' + title + '.jpg' ) + filePageTitle: mw.Title.newFromText( 'File:' + title + '.jpg' ), + src: 'https://upload.wikimedia.org/wikipedia/commons/3/3a/Foobar.jpg' }; const imageData = { title: image.filePageTitle, diff --git a/tests/qunit/mmv/ui/mmv.ui.stripeButtons.test.js b/tests/qunit/mmv/ui/mmv.ui.stripeButtons.test.js index 6d3e4963b..b49df5a64 100644 --- a/tests/qunit/mmv/ui/mmv.ui.stripeButtons.test.js +++ b/tests/qunit/mmv/ui/mmv.ui.stripeButtons.test.js @@ -48,7 +48,7 @@ const { StripeButtons } = require( 'mmv' ); const buttons = createStripeButtons(); const fakeImageInfo = { descriptionUrl: '//commons.wikimedia.org/wiki/File:Foo.jpg' }; - buttons.set( fakeImageInfo ); + buttons.set( null, fakeImageInfo ); buttons.empty(); assert.true( true, 'No error on set()/empty().' ); @@ -62,16 +62,16 @@ const { StripeButtons } = require( 'mmv' ); const descriptionUrlCommons = 'https://commons.wikimedia.org/desc'; const descriptionUrl2 = 'http://example.com/different-desc'; - buttons.set( { descriptionUrl } ); + buttons.set( null, { descriptionUrl } ); assert.strictEqual( $button.hasClass( 'mw-mmv-repo-button-commons' ), false, 'Button does not have commons class non-Commons files' ); assert.strictEqual( $button.find( 'a' ).addBack().filter( 'a' ).attr( 'href' ), descriptionUrl, 'Description page link is correct' ); - buttons.set( { descriptionUrl: descriptionUrlCommons } ); + buttons.set( null, { descriptionUrl: descriptionUrlCommons } ); assert.strictEqual( $button.hasClass( 'mw-mmv-repo-button-commons' ), true, 'Button commons class for Commons files' ); - buttons.set( { + buttons.set( null, { descriptionUrl, pageID: 1, title: { getUrl: () => descriptionUrl2 }