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

diff --git a/resources/mediawiki/mediawiki.jqueryMsg.js b/resources/mediawiki/mediawiki.jqueryMsg.js
index c599f4b..8ab4a7e 100644 (file)
--- 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 0ec584f..b90644e 100644 (file)
--- 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', '<foo foo="foo">x$1y&lt;</foo>z' );
+
+       assert.equal(
+               parser( 'plain-input', 'bar' ),
+               '&lt;foo foo="foo"&gt;xbary&amp;lt;&lt;/foo&gt;z',
+               'Input is not considered html'
+       );
+
+       mw.messages.set( 'plain-replace', 'Foo $1' );
+
+       assert.equal(
+               parser( 'plain-replace', '<bar bar="bar">&gt;</bar>' ),
+               'Foo &lt;bar bar="bar"&gt;&amp;gt;&lt;/bar&gt;',
+               'Replacement is not considered html'
+       );
+
+       mw.messages.set( 'object-replace', 'Foo $1' );
+
+       assert.equal(
+               parser( 'object-replace', $( '<div class="bar">&gt;</div>' ) ),
+               'Foo <div class="bar">&gt;</div>',
+               'jQuery objects are preserved as raw html'
+       );
+
+       assert.equal(
+               parser( 'object-replace', $( '<div class="bar">&gt;</div>' ).get( 0 ) ),
+               'Foo <div class="bar">&gt;</div>',
+               'HTMLElement objects are preserved as raw html'
+       );
+
+       assert.equal(
+               parser( 'object-replace', $( '<div class="bar">&gt;</div>' ).toArray() ),
+               'Foo <div class="bar">&gt;</div>',
+               '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 <a href="http://example.org/?x=y&amp;z">bar</a>',
+               '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.<br>\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 ' +
                $( '<a>' ).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.<br />\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;