From 97be4e21ad8a3685265dd39e4b12050da81ecc6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartosz=20Dziewo=C5=84ski?= Date: Mon, 18 Oct 2021 16:29:07 +0200 Subject: [PATCH] ve.ui.MWMediaDialog: Clean up image metadata display * Fix incorrect use of .append() instead of .text() (which was causing some l10n messages to be treated as raw HTML) * Avoid escaping and parsing HTML several times when plain text was intended * Remove some unused options and variables Follow-up to 839b64d882ee188ae31fa9853c40547d8aca93b2. Change-Id: I124257c73fe09713afefccdec8e90200e6ae433d --- .../ve-mw/ui/dialogs/ve.ui.MWMediaDialog.js | 80 +++++++++---------- .../widgets/ve.ui.MWMediaInfoFieldWidget.js | 62 +++++++------- 2 files changed, 70 insertions(+), 72 deletions(-) diff --git a/modules/ve-mw/ui/dialogs/ve.ui.MWMediaDialog.js b/modules/ve-mw/ui/dialogs/ve.ui.MWMediaDialog.js index 19759c72ae..35f3322116 100644 --- a/modules/ve-mw/ui/dialogs/ve.ui.MWMediaDialog.js +++ b/modules/ve-mw/ui/dialogs/ve.ui.MWMediaDialog.js @@ -508,9 +508,7 @@ ve.ui.MWMediaDialog.prototype.buildMediaInfoPanel = function ( imageinfo ) { { name: 'ImageDescription', value: ve.getProp( metadata, 'ImageDescription', 'value' ), - data: { - keepOriginal: true - }, + format: 'html', view: { type: 'description', primary: true, @@ -519,13 +517,15 @@ ve.ui.MWMediaDialog.prototype.buildMediaInfoPanel = function ( imageinfo ) { }, { name: '$fileDetails', - data: { skipProcessing: true }, + // Real value is set later + value: '', + format: 'html', view: { icon: 'image' } }, { name: 'LicenseShortName', value: ve.getProp( metadata, 'LicenseShortName', 'value' ), - data: {}, + format: 'html-remove-formatting', view: { href: ve.getProp( metadata, 'LicenseUrl', 'value' ), icon: this.getLicenseIcon( ve.getProp( metadata, 'LicenseShortName', 'value' ) ) @@ -534,54 +534,52 @@ ve.ui.MWMediaDialog.prototype.buildMediaInfoPanel = function ( imageinfo ) { { name: 'Artist', value: ve.getProp( metadata, 'Artist', 'value' ), - data: {}, + format: 'html-remove-formatting', view: { // "Artist" label - label: 'visualeditor-dialog-media-info-meta-artist', + labelMsg: 'visualeditor-dialog-media-info-meta-artist', icon: 'userAvatar' } }, { name: 'Credit', value: ve.getProp( metadata, 'Credit', 'value' ), - data: {}, + format: 'html-remove-formatting', view: { icon: 'userAvatar' } }, { name: 'user', value: imageinfo.user, - data: { skipProcessing: true }, + format: 'plaintext', view: { icon: 'userAvatar', // This is 'uploaded by' - label: 'visualeditor-dialog-media-info-artist' + labelMsg: 'visualeditor-dialog-media-info-artist' } }, { name: 'timestamp', value: imageinfo.timestamp, - data: { - ignoreCharLimit: true - }, + format: 'plaintext', view: { icon: 'clock', - label: 'visualeditor-dialog-media-info-uploaded', + labelMsg: 'visualeditor-dialog-media-info-uploaded', isDate: true } }, { name: 'DateTimeOriginal', value: ve.getProp( metadata, 'DateTimeOriginal', 'value' ), - data: {}, + format: 'html-remove-formatting', view: { icon: 'clock', - label: 'visualeditor-dialog-media-info-created' + labelMsg: 'visualeditor-dialog-media-info-created' } }, { name: 'moreinfo', value: ve.msg( 'visualeditor-dialog-media-info-moreinfo' ), - data: {}, + format: 'plaintext', view: { icon: 'info', href: imageinfo.descriptionurl @@ -609,14 +607,17 @@ ve.ui.MWMediaDialog.prototype.buildMediaInfoPanel = function ( imageinfo ) { // Clean data from the API responses for ( i = 0; i < apiDataKeysConfig.length; i++ ) { field = apiDataKeysConfig[ i ].name; - // Skip empty fields and those that are specifically configured to be skipped - if ( apiDataKeysConfig[ i ].data.skipProcessing ) { + if ( apiDataKeysConfig[ i ].format === 'html' ) { + apiData[ field ] = new OO.ui.HtmlSnippet( apiDataKeysConfig[ i ].value ); + + } else if ( apiDataKeysConfig[ i ].format === 'html-remove-formatting' ) { + apiData[ field ] = this.cleanAPIresponse( apiDataKeysConfig[ i ].value ); + + } else if ( apiDataKeysConfig[ i ].format === 'plaintext' ) { apiData[ field ] = apiDataKeysConfig[ i ].value; + } else { - // Store a clean information from the API. - if ( apiDataKeysConfig[ i ].value ) { - apiData[ field ] = this.cleanAPIresponse( apiDataKeysConfig[ i ].value, apiDataKeysConfig[ i ].data ); - } + throw new Error( 'Unexpected metadata field format' ); } } @@ -624,7 +625,7 @@ ve.ui.MWMediaDialog.prototype.buildMediaInfoPanel = function ( imageinfo ) { if ( imageinfo.mediatype === 'AUDIO' ) { // Label this file as an audio apiData.$fileDetails = $( '' ) - .append( ve.msg( 'visualeditor-dialog-media-info-audiofile' ) ); + .text( ve.msg( 'visualeditor-dialog-media-info-audiofile' ) ); } else { // Build the display for image size and type apiData.$fileDetails = $( '
' ) @@ -639,7 +640,7 @@ ve.ui.MWMediaDialog.prototype.buildMediaInfoPanel = function ( imageinfo ) { ), $( '' ) .addClass( 've-ui-mwMediaDialog-panel-imageinfo-separator' ) - .text( mw.msg( 'visualeditor-dialog-media-info-separator' ) ), + .text( ve.msg( 'visualeditor-dialog-media-info-separator' ) ), $( '' ).text( fileType ) ); } @@ -768,29 +769,20 @@ ve.ui.MWMediaDialog.prototype.fetchThumbnail = function ( imageName, dimensions /** * Clean the API responses and return it in plaintext. If needed, truncate. * - * @param {string} rawResponse Raw response from the API - * @param {Object} config Configuration options + * @param {string} html Raw response from the API * @return {string} Plaintext clean response */ -ve.ui.MWMediaDialog.prototype.cleanAPIresponse = function ( rawResponse, config ) { - var isTruncated, charLimit, - html = $.parseHTML( rawResponse ), - ellipsis = ve.msg( 'visualeditor-dialog-media-info-ellipsis' ), - originalText = $( '
' ).append( html ).text(); - - config = config || {}; - - charLimit = config.charLimit || 50; - isTruncated = originalText.length > charLimit; - - if ( config.keepOriginal ) { - return html; - } +ve.ui.MWMediaDialog.prototype.cleanAPIresponse = function ( html ) { + var text = $( $.parseHTML( html ) ).text(); // Check if the string should be truncated - return mw.html.escape( isTruncated && !config.ignoreCharLimit ? - originalText.slice( 0, charLimit ) + ellipsis : - originalText ); + var charLimit = 50; + if ( text.length > charLimit ) { + var ellipsis = ve.msg( 'visualeditor-dialog-media-info-ellipsis' ); + text = text.slice( 0, charLimit ) + ellipsis; + } + + return text; }; /** diff --git a/modules/ve-mw/ui/widgets/ve.ui.MWMediaInfoFieldWidget.js b/modules/ve-mw/ui/widgets/ve.ui.MWMediaInfoFieldWidget.js index 74b3201333..46cc7fd32e 100644 --- a/modules/ve-mw/ui/widgets/ve.ui.MWMediaInfoFieldWidget.js +++ b/modules/ve-mw/ui/widgets/ve.ui.MWMediaInfoFieldWidget.js @@ -16,10 +16,10 @@ * @mixins OO.ui.mixin.TitledElement * * @constructor - * @param {Object} content API response data from which to build the display + * @param {jQuery|string|OO.ui.HtmlSnippet} content API response data from which to build the display * @param {Object} [config] Configuration options * @cfg {string} [href] A url encapsulating the field text. If a label is attached it will include the label. - * @cfg {string} [label] A ve.msg() label string for the field. + * @cfg {string} [labelMsg] A ve.msg() label string for the field. * @cfg {boolean} [isDate=false] Field text is a date that will be converted to 'fromNow' string. * @cfg {string} [type='attribute'] Field type, either 'description' or 'attribute' * @cfg {string} [descriptionHeight='4em'] Height limit for description fields @@ -39,38 +39,44 @@ ve.ui.MWMediaInfoFieldWidget = function VeUiMWMediaInfoFieldWidget( content, con this.$text = $( '
' ) .addClass( 've-ui-mwMediaInfoFieldWidget-text' ); - this.$overlay = null; this.type = config.type || 'attribute'; // Initialization - if ( config.isDate && ( datetime = moment( content ) ).isValid() ) { - content = datetime.fromNow(); + if ( typeof content === 'string' ) { + if ( config.isDate && ( datetime = moment( content ) ).isValid() ) { + content = datetime.fromNow(); + } + + if ( config.labelMsg ) { + // Messages defined in ve.ui.MWMediaDialog#buildMediaInfoPanel + // eslint-disable-next-line mediawiki/msg-doc + content = ve.msg( config.labelMsg, content ); + } + + if ( config.href ) { + // This variable may contain either jQuery objects or strings + // eslint-disable-next-line no-jquery/variable-pattern + content = $( '' ) + .attr( 'href', + // For the cases where we get urls that are "local" + // without http(s) prefix, we will add that prefix + // ourselves + !config.href.match( /^(https?:)?\/\// ) ? + '//' + config.href : + config.href + ) + .text( content ); + } } - if ( config.label ) { - // Messages defined in ve.ui.MWMediaDialog#buildMediaInfoPanel - // eslint-disable-next-line mediawiki/msg-doc - content = ve.msg( config.label, content ); - } - - if ( config.href ) { - this.$text - .append( - $( '' ) - .attr( 'target', '_blank' ) - .attr( 'rel', 'mw:ExtLink' ) - .attr( 'href', - // For the cases where we get urls that are "local" - // without http(s) prefix, we will add that prefix - // ourselves - !config.href.match( /^(https?:)?\/\// ) ? - '//' + config.href : - config.href - ) - .append( content ) - ); - } else { + if ( typeof content === 'string' ) { + this.$text.text( content ); + } else if ( content instanceof OO.ui.HtmlSnippet ) { + this.$text.html( content.toString() ); + } else if ( content instanceof $ ) { this.$text.append( content ); + } else { + throw new Error( 'Unexpected metadata field content' ); } this.$element