From 724fbe82c15cfb8a7d79155bfa2d66e0abbc832a Mon Sep 17 00:00:00 2001 From: =?utf8?q?Bartosz=20Dziewo=C5=84ski?= Date: Mon, 10 Sep 2018 11:08:00 +0200 Subject: [PATCH] mw.jqueryMsg: Handle non-string parameters to functions that expect strings better Generally, non-string parameters should not be passed to functions that expect strings. But sometimes we don't document the types well and our translators do unexpected things. :) * If the function uses the parameter to look up some other value (e.g. {{GENDER:$1|...}}, {{PLURAL:$1|...}}), convert it to a string by removing all formatting (using textify()) and hope for the best. * If the function applies some textual transformation to the parameter (e.g. {{GRAMMAR:...|$1}}, {{formatnum:$1}}), do nothing and return the original parameter. Bug: T203892 Change-Id: I5c760bf666cc8dae4ce10e3319366e73e2d596e8 --- .../mediawiki.jqueryMsg.js | 34 +++++++++++++------ .../mediawiki/mediawiki.jqueryMsg.test.js | 8 +++-- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/resources/src/mediawiki.jqueryMsg/mediawiki.jqueryMsg.js b/resources/src/mediawiki.jqueryMsg/mediawiki.jqueryMsg.js index b3154e155f..04d1b4269e 100644 --- a/resources/src/mediawiki.jqueryMsg/mediawiki.jqueryMsg.js +++ b/resources/src/mediawiki.jqueryMsg/mediawiki.jqueryMsg.js @@ -1042,7 +1042,7 @@ * * @param {Array} nodes List of one element, integer, n >= 0 * @param {Array} replacements List of at least n strings - * @return {string} replacement + * @return {string|jQuery} replacement */ replace: function ( nodes, replacements ) { var index = parseInt( nodes[ 0 ], 10 ); @@ -1115,7 +1115,7 @@ * Handles an (already-validated) HTML element. * * @param {Array} nodes Nodes to process when creating element - * @return {jQuery|Array} jQuery node for valid HTML or array for disallowed element + * @return {jQuery} */ htmlelement: function ( nodes ) { var tagName, attributes, contents, $element; @@ -1173,13 +1173,13 @@ * So convert it back with the current language's convertNumber. * * @param {Array} nodes List of nodes, [ {string|number}, {string}, {string} ... ] - * @return {string} selected pluralized form according to current language + * @return {string|jQuery} selected pluralized form according to current language */ plural: function ( nodes ) { var forms, firstChild, firstChildText, explicitPluralFormNumber, formIndex, form, count, explicitPluralForms = {}; - count = parseFloat( this.language.convertNumber( nodes[ 0 ], true ) ); + count = parseFloat( this.language.convertNumber( textify( nodes[ 0 ] ), true ) ); forms = nodes.slice( 1 ); for ( formIndex = 0; formIndex < forms.length; formIndex++ ) { form = forms[ formIndex ]; @@ -1225,7 +1225,7 @@ * - a gender string ('male', 'female' or 'unknown') * * @param {Array} nodes List of nodes, [ {string|mw.user}, {string}, {string}, {string} ] - * @return {string} Selected gender form according to current language + * @return {string|jQuery} Selected gender form according to current language */ gender: function ( nodes ) { var gender, @@ -1241,7 +1241,7 @@ if ( maybeUser && maybeUser.options instanceof mw.Map ) { gender = maybeUser.options.get( 'gender' ); } else { - gender = maybeUser; + gender = textify( maybeUser ); } return this.language.gender( gender, forms ); @@ -1252,23 +1252,30 @@ * Invoked by putting `{{grammar:form|word}}` in a message * * @param {Array} nodes List of nodes [{Grammar case eg: genitive}, {string word}] - * @return {string} selected grammatical form according to current language + * @return {string|jQuery} selected grammatical form according to current language */ grammar: function ( nodes ) { var form = nodes[ 0 ], word = nodes[ 1 ]; - return word && form && this.language.convertGrammar( word, form ); + // These could be jQuery objects (passed as message parameters), + // in which case we can't transform them (like rawParams() in PHP). + if ( typeof form === 'string' && typeof word === 'string' ) { + return this.language.convertGrammar( word, form ); + } + return word; }, /** * Tranform parsed structure into a int: (interface language) message include * Invoked by putting `{{int:othermessage}}` into a message * + * TODO Syntax in the included message is not parsed, this seems like a bug? + * * @param {Array} nodes List of nodes * @return {string} Other message */ 'int': function ( nodes ) { - var msg = nodes[ 0 ]; + var msg = textify( nodes[ 0 ] ); return mw.jqueryMsg.getMessageFunction()( msg.charAt( 0 ).toLowerCase() + msg.slice( 1 ) ); }, @@ -1294,13 +1301,18 @@ * separator, according to the current language. * * @param {Array} nodes List of nodes - * @return {number|string} Formatted number + * @return {number|string|jQuery} Formatted number */ formatnum: function ( nodes ) { var isInteger = !!nodes[ 1 ] && nodes[ 1 ] === 'R', number = nodes[ 0 ]; - return this.language.convertNumber( number, isInteger ); + // These could be jQuery objects (passed as message parameters), + // in which case we can't transform them (like rawParams() in PHP). + if ( typeof number === 'string' || typeof number === 'number' ) { + return this.language.convertNumber( number, isInteger ); + } + return number; }, /** diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js index 2bbc2b57e7..ff83b980d5 100644 --- a/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js +++ b/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js @@ -1157,6 +1157,8 @@ QUnit.test( 'Non-string parameters to various functions', function ( assert ) { var i, cases; + // For jquery-param-int + mw.messages.set( 'x', 'y' ); // For jquery-param-grammar mw.language.setData( 'en', 'grammarTransformations', { test: [ @@ -1183,12 +1185,12 @@ { key: 'jquery-param-grammar', msg: '{{GRAMMAR:test|$1}}', - expected: '{{GRAMMAR:test|$1}}' + expected: 'x' }, { key: 'jquery-param-int', msg: '{{int:$1}}', - expected: '{{int:$1}}' + expected: 'y' }, { key: 'jquery-param-ns', @@ -1198,7 +1200,7 @@ { key: 'jquery-param-formatnum', msg: '{{formatnum:$1}}', - expected: '[object Object]' + expected: 'x' }, { key: 'jquery-param-case', -- 2.20.1