From 5faaa93a4a916e00fdf7113aea4f970d7dc541fe Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Sun, 20 Jan 2013 02:57:21 +0100 Subject: [PATCH] mediawiki.jqueryMsg: Fix regression and add tests. From If060b75f. Thanks to jshint and us using Checkstyle reports in Jenkins (instead of a boolean good/bad). We also see warnings that are excluded from the regular count (but can still be useful). https://integration.mediawiki.org/ci/job/mediawiki-core-lint/4617/checkstyleResult/NORMAL/ > Implied global 'arg' The html escaping didn't do anything because the condition was always false (there is no variable there by name of 'arg'). It didn't fail the tests because If060b75f didn't add any tests. I added proper tests now for "foo [$1 bar]" where $1 is a url containing characters ("&" specifically) that must not be double escaped. The test immediately failed. Addressed the bug in concat instead of replace. append() is a very powerful jQuery method. It can append: * HTMLElement objects * node lists * jQuery objects * Arrays with any of the above * text strings * html strings "html strings" is likely decided by some kind of regex looking for html-ish characters. Which is exactly what we don't want, we want it to consider strings always as text, so we make a text node instead. And removed the useless if/else block in replace() as it was always going for the else condition (and should). Effectively undoing the fix from If060b75f that was supposedly fixing this bug but didn't. Change-Id: Ifbeae7e9bd003a33f353d42caffc3ae978c3dc56 --- resources/mediawiki/mediawiki.jqueryMsg.js | 17 ++--- .../mediawiki/mediawiki.jqueryMsg.test.js | 66 +++++++++++++++++-- 2 files changed, 66 insertions(+), 17 deletions(-) diff --git a/resources/mediawiki/mediawiki.jqueryMsg.js b/resources/mediawiki/mediawiki.jqueryMsg.js index c599f4bc40..8ab4a7e531 100644 --- a/resources/mediawiki/mediawiki.jqueryMsg.js +++ b/resources/mediawiki/mediawiki.jqueryMsg.js @@ -100,6 +100,8 @@ */ return function () { var $target = this.empty(); + // TODO: Simply $target.append( failableParserFn( arguments ).contents() ) + // or Simply $target.append( failableParserFn( arguments ) ) $.each( failableParserFn( arguments ).contents(), function ( i, node ) { $target.append( node ); } ); @@ -593,8 +595,9 @@ $span.append( childNode ); } ); } else { - // strings, integers, anything else - $span.append( node ); + // Let jQuery append nodes, arrays of nodes and jQuery objects + // other things (strings, numbers, ..) are appended as text nodes (not as HTML strings) + $span.append( $.type( node ) === 'object' ? node : document.createTextNode( node ) ); } } ); return $span; @@ -605,7 +608,7 @@ * Note that we expect the parsed parameter to be zero-based. i.e. $1 should have become [ 0 ]. * if the specified parameter is not found return the same string * (e.g. "$99" -> parameter 98 -> not found -> return "$99" ) - * TODO throw error if nodes.length > 1 ? + * TODO: Throw error if nodes.length > 1 ? * @param {Array} of one element, integer, n >= 0 * @return {String} replacement */ @@ -613,13 +616,7 @@ var index = parseInt( nodes[0], 10 ); if ( index < replacements.length ) { - if ( typeof arg === 'string' ) { - // replacement is a string, escape it - return mw.html.escape( replacements[index] ); - } else { - // replacement is no string, don't touch! - return replacements[index]; - } + return replacements[index]; } else { // index not found, fallback to displaying variable return '$' + ( index + 1 ); diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js index 0ec584fc4a..b90644ea1f 100644 --- a/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js +++ b/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js @@ -49,6 +49,59 @@ function getMwLanguage( langCode, cb ) { }); } +QUnit.test( 'Replace', 9, function ( assert ) { + var parser = mw.jqueryMsg.getMessageFunction(); + + mw.messages.set( 'simple', 'Foo $1 baz $2' ); + + assert.equal( parser( 'simple' ), 'Foo $1 baz $2', 'Replacements with no substitutes' ); + assert.equal( parser( 'simple', 'bar' ), 'Foo bar baz $2', 'Replacements with less substitutes' ); + assert.equal( parser( 'simple', 'bar', 'quux' ), 'Foo bar baz quux', 'Replacements with all substitutes' ); + + mw.messages.set( 'plain-input', 'x$1y<z' ); + + assert.equal( + parser( 'plain-input', 'bar' ), + '<foo foo="foo">xbary&lt;</foo>z', + 'Input is not considered html' + ); + + mw.messages.set( 'plain-replace', 'Foo $1' ); + + assert.equal( + parser( 'plain-replace', '>' ), + 'Foo <bar bar="bar">&gt;</bar>', + 'Replacement is not considered html' + ); + + mw.messages.set( 'object-replace', 'Foo $1' ); + + assert.equal( + parser( 'object-replace', $( '
>
' ) ), + 'Foo
>
', + 'jQuery objects are preserved as raw html' + ); + + assert.equal( + parser( 'object-replace', $( '
>
' ).get( 0 ) ), + 'Foo
>
', + 'HTMLElement objects are preserved as raw html' + ); + + assert.equal( + parser( 'object-replace', $( '
>
' ).toArray() ), + 'Foo
>
', + 'HTMLElement[] arrays are preserved as raw html' + ); + + mw.messages.set( 'wikilink-replace', 'Foo [$1 bar]' ); + assert.equal( + parser( 'wikilink-replace', 'http://example.org/?x=y&z' ), + 'Foo bar', + 'Href is not double-escaped in wikilink function' + ); +} ); + QUnit.test( 'Plural', 3, function ( assert ) { var parser = mw.jqueryMsg.getMessageFunction(); @@ -58,7 +111,6 @@ QUnit.test( 'Plural', 3, function ( assert ) { assert.equal( parser( 'plural-msg', 2 ), 'Found 2 items', 'Plural test for english' ); } ); - QUnit.test( 'Gender', 11, function ( assert ) { // TODO: These tests should be for mw.msg once mw.msg integrated with mw.jqueryMsg // TODO: English may not be the best language for these tests. Use a language like Arabic or Russian @@ -144,7 +196,7 @@ QUnit.test( 'Grammar', 2, function ( assert ) { assert.equal( parser( 'grammar-msg-wrong-syntax' ), 'Przeszukaj ' , 'Grammar Test with wrong grammar template syntax' ); } ); -QUnit.test( 'Output matches PHP parser', mw.libs.phpParserData.tests.length, function ( assert ) { +QUnit.test( 'Match PHP parser', mw.libs.phpParserData.tests.length, function ( assert ) { mw.messages.set( mw.libs.phpParserData.messages ); $.each( mw.libs.phpParserData.tests, function ( i, test ) { QUnit.stop(); @@ -165,7 +217,7 @@ QUnit.test( 'Output matches PHP parser', mw.libs.phpParserData.tests.length, fun } ); }); -QUnit.test( 'Links', 6, function ( assert ) { +QUnit.test( 'Wikilink', 6, function ( assert ) { var parser = mw.jqueryMsg.getMessageFunction(), expectedListUsers, expectedDisambiguationsText, @@ -191,12 +243,12 @@ QUnit.test( 'Links', 6, function ( assert ) { 'Piped wikilink' ); - expectedDisambiguationsText = 'The following pages contain at least one link to a disambiguation page.\nThey may have to link to a more appropriate page instead.
\nA page is treated as a disambiguation page if it uses a template that is linked from ' + + expectedDisambiguationsText = 'The following pages contain at least one link to a disambiguation page.\nThey may have to link to a more appropriate page instead.\nA page is treated as a disambiguation page if it uses a template that is linked from ' + $( '' ).attr( { title: 'MediaWiki:Disambiguationspage', href: mw.util.wikiGetlink( 'MediaWiki:Disambiguationspage' ) } ).text( 'MediaWiki:Disambiguationspage' ).getOuterHtml() + '.'; - mw.messages.set( 'disambiguations-text', 'The following pages contain at least one link to a disambiguation page.\nThey may have to link to a more appropriate page instead.
\nA page is treated as a disambiguation page if it uses a template that is linked from [[MediaWiki:Disambiguationspage]].' ); + mw.messages.set( 'disambiguations-text', 'The following pages contain at least one link to a disambiguation page.\nThey may have to link to a more appropriate page instead.\nA page is treated as a disambiguation page if it uses a template that is linked from [[MediaWiki:Disambiguationspage]].' ); assert.equal( parser( 'disambiguations-text' ), expectedDisambiguationsText, @@ -243,7 +295,7 @@ QUnit.test( 'Links', 6, function ( assert ) { ); }); -QUnit.test( 'Int magic word', 4, function ( assert ) { +QUnit.test( 'Int', 4, function ( assert ) { var parser = mw.jqueryMsg.getMessageFunction(), newarticletextSource = 'You have followed a link to a page that does not exist yet. To create the page, start typing in the box below (see the [[{{Int:Helppage}}|help page]] for more info). If you are here by mistake, click your browser\'s back button.', expectedNewarticletext; @@ -292,7 +344,7 @@ QUnit.test( 'Int magic word', 4, function ( assert ) { // Tests that getMessageFunction is used for messages with curly braces or square brackets, // but not otherwise. -QUnit.test( 'Calls to mw.msg', 8, function ( assert ) { +QUnit.test( 'mw.msg()', 8, function ( assert ) { // Should be var map, oldGMF, outerCalled, innerCalled; -- 2.20.1