mediawiki.jqueryMsg: Fix double-escaped attributes in the parser
authorMatthias Mullie <git@mullie.eu>
Sat, 26 May 2012 00:42:47 +0000 (17:42 -0700)
committermlitn <git@mullie.eu>
Wed, 27 Jun 2012 13:39:50 +0000 (15:39 +0200)
E.g.: message: "Your feedback has been [$1 posted here]."
In mediawiki.jqueryMsg.js, L31, the parser would (rightfully) escape
XML entities in the parameter (a link) being assigned through $1

When actually adding the (already escaped) parameter as href-
attribute of the link-to-be-built, it'll automatically have its HTML
entities escaped once again (thus double-escaping &'s)

This patch will identify regular links and links with replacement-
url's in a different way; now the escaping can be moved to when the
variables are actually being replaced (for normal parameter
replacement) and can be processed differently (= not escaping) when
dealing with a link.

Tests:
 - Removed useless ok() tests. I accidentally introduced that pattern
   last year for no good reason. Slowly getting it out again. It
   either can't return false, or it tests unrelated things that have
   their own tests.
 - Simplified examples to reduce repetition of unrelated strings.
 - Improved assertion titles to better describe what is being tested
   to help find actual problems.
   (e.g. "Neutral from mw.user object" instead of
   "Gender neutral or unknown").

Change-Id: If060b75f5575f4c7c3a25e5280ae697171120e84

resources/mediawiki/mediawiki.jqueryMsg.js
tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js

index 043ebce..f690a1d 100644 (file)
                return function( args ) {
                        var key = args[0];
                        var argsArray = $.isArray( args[1] ) ? args[1] : $.makeArray( args ).slice( 1 ); 
-                       var escapedArgsArray = $.map( argsArray, function( arg ) { 
-                               return typeof arg === 'string' ? mw.html.escape( arg ) : arg;
-                       } );
                        try {
-                               return parser.parse( key, escapedArgsArray );
+                               return parser.parse( key, argsArray );
                        } catch ( e ) {
-                               return $( '<span></span>' ).append( key + ': ' + e.message );
+                               return $( '<span>' ).append( key + ': ' + e.message );
                        }
                };
        }
                },
 
                /**
-                * Fetch the message string associated with a key, return parsed structure. Memoized.
+                * Fetch the message string associated with a key, return parsed structure. Memoized.
                 * Note that we pass '[' + key + ']' back for a missing message here. 
-                * @param {String} key
+                * @param {String} key
                 * @return {String|Array} string of '[key]' if message missing, simple string if possible, array of arrays if needs parsing
                 */
                getAst: function( key ) {
 
 
                        var regularLiteral = makeRegexParser( /^[^{}[\]$\\]/ );
-                       var regularLiteralWithoutBar = makeRegexParser(/^[^{}[\]$\\|]/);
-                       var regularLiteralWithoutSpace = makeRegexParser(/^[^{}[\]$\s]/);
+                       var regularLiteralWithoutBar = makeRegexParser(/^[^{}[\]$\\|]/);
+                       var regularLiteralWithoutSpace = makeRegexParser(/^[^{}[\]$\s]/);
 
                        var backslash = makeStringParser( "\\" );
                        var anyCharacter = makeRegexParser( /^./ );
                                return result;
                        }
 
+                       // this is the same as the above extlink, except that the url is being passed on as a parameter
+                       function extLinkParam() {
+                               var result = sequence( [
+                                       openExtlink,
+                                       dollar,
+                                       digits,
+                                       whitespace,
+                                       expression,
+                                       closeExtlink
+                               ] );
+                               if ( result === null ) {
+                                       return null;
+                               }
+                               return [ 'LINKPARAM', parseInt( result[2], 10 ) - 1, result[4] ];
+                       }
+
                        var openLink = makeStringParser( '[[' );
                        var closeLink = makeStringParser( ']]' );
 
                        }
 
                        var nonWhitespaceExpression = choice( [
-                               template,        
+                               template,
                                link,
+                               extLinkParam,
                                extlink,
                                replacement,
                                literalWithoutSpace
                        ] );
 
                        var paramExpression = choice( [
-                               template,        
+                               template,
                                link,
+                               extLinkParam,
                                extlink,
                                replacement,
                                literalWithoutBar
                        var expression = choice( [ 
                                template,
                                link,
+                               extLinkParam,
                                extlink,
                                replacement,
                                literal 
                },
 
                /**
-                * Return replacement of correct index, or string if unavailable.
+                * Return escaped replacement of correct index, or string if unavailable.
                 * 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" )
                 */
                replace: function( nodes, replacements ) {
                        var index = parseInt( nodes[0], 10 );
-                       return index < replacements.length ? replacements[index] : '$' + ( index + 1 ); 
+                       
+                       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];
+                               }
+                       } else {
+                               // index not found, fallback to displaying variable
+                               return '$' + ( index + 1 );
+                       }
                },
 
                /** 
                        return $el;
                },
 
+               /**
+                * This is basically use a combination of replace + link (link with parameter
+                * as url), but we don't want to run the regular replace here-on: inserting a
+                * url as href-attribute of a link will automatically escape it already, so
+                * we don't want replace to (manually) escape it as well.
+                * TODO throw error if nodes.length > 1 ?
+                * @param {Array} of one element, integer, n >= 0
+                * @return {String} replacement
+                */
+               linkparam: function( nodes, replacements ) {
+                       var replacement,
+                               index = parseInt( nodes[0], 10 );
+                       if ( index < replacements.length) {
+                               replacement = replacements[index];
+                       } else {
+                               replacement = '$' + ( index + 1 );
+                       }
+                       return this.link( [ replacement, nodes[1] ] );
+               },
+
                /**
                 * Transform parsed structure into pluralization
                 * n.b. The first node may be a non-integer (for instance, a string representing an Arabic number).
index 2abe016..481a5bb 100644 (file)
 module( 'mediawiki.jqueryMsg' );
 
-test( '-- Initial check', function() {
+test( '-- Initial check', function () {
        expect( 1 );
        ok( mw.jqueryMsg, 'mw.jqueryMsg defined' );
 } );
 
-test( 'mw.jqueryMsg Plural', function() {
-       expect( 5 );
+test( 'mw.jqueryMsg Plural', function () {
+       expect( 3 );
        var parser = mw.jqueryMsg.getMessageFunction();
-       ok( parser, 'Parser Function initialized' );
-       ok( mw.messages.set( 'plural-msg', 'Found $1 {{PLURAL:$1|item|items}}' ), 'mw.messages.set: Register' );
-       equal( parser( 'plural-msg', 0 ) , 'Found 0 items', 'Plural test for english with zero as count' );
-       equal( parser( 'plural-msg', 1 ) , 'Found 1 item', 'Singular test for english' );
-       equal( parser( 'plural-msg', 2 ) , 'Found 2 items', 'Plural test for english' );
+
+       mw.messages.set( 'plural-msg', 'Found $1 {{PLURAL:$1|item|items}}' );
+       equal( parser( 'plural-msg', 0 ), 'Found 0 items', 'Plural test for english with zero as count' );
+       equal( parser( 'plural-msg', 1 ), 'Found 1 item', 'Singular test for english' );
+       equal( parser( 'plural-msg', 2 ), 'Found 2 items', 'Plural test for english' );
 } );
 
 
-test( 'mw.jqueryMsg Gender', function() {
-       expect( 16 );
-       //TODO: These tests should be for mw.msg once mw.msg integrated with mw.jqueryMsg
-       var user = mw.user;
+test( 'mw.jqueryMsg Gender', function () {
+       expect( 11 );
+       // 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
+       var user = mw.user,
+               parser = mw.jqueryMsg.getMessageFunction();
+
+       // The values here are not significant,
+       // what matters is which of the values is choosen by the parser
+       mw.messages.set( 'gender-msg', '$1: {{GENDER:$2|blue|pink|green}}' );
+
        user.options.set( 'gender', 'male' );
-       var parser = mw.jqueryMsg.getMessageFunction();
-       ok( parser, 'Parser Function initialized' );
-       //TODO: English may not be the best language for these tests. Use a language like Arabic or Russian
-       ok( mw.messages.set( 'gender-msg', '$1 reverted {{GENDER:$2|his|her|their}} last edit' ), 'mw.messages.set: Register' );
-       equal( parser( 'gender-msg', 'Bob', 'male' ) , 'Bob reverted his last edit', 'Gender masculine' );
-       equal( parser( 'gender-msg', 'Bob', user ) , 'Bob reverted his last edit', 'Gender masculine' );
+       equal(
+               parser( 'gender-msg', 'Bob', 'male' ),
+               'Bob: blue',
+               'Masculine from string "male"'
+       );
+       equal(
+               parser( 'gender-msg', 'Bob', user ),
+               'Bob: blue',
+               'Masculine from mw.user object'
+       );
+
        user.options.set( 'gender', 'unknown' );
-       equal( parser( 'gender-msg', 'They', user ) , 'They reverted their last edit', 'Gender neutral or unknown' );
-       equal( parser( 'gender-msg', 'Alice', 'female' ) , 'Alice reverted her last edit', 'Gender feminine' );
-       equal( parser( 'gender-msg', 'User' ) , 'User reverted their last edit', 'Gender neutral' );
-       equal( parser( 'gender-msg', 'User', 'unknown' ) , 'User reverted their last edit', 'Gender neutral' );
-       ok( mw.messages.set( 'gender-msg-one-form', '{{GENDER:$1|User}} reverted last $2 {{PLURAL:$2|edit|edits}}' ), 'mw.messages.set: Register' );
-       equal( parser( 'gender-msg-one-form', 'male', 10 ) , 'User reverted last 10 edits', 'Gender neutral and plural form' );
-       equal( parser( 'gender-msg-one-form', 'female', 1 ) , 'User reverted last 1 edit', 'Gender neutral and singular form' );
-       ok( mw.messages.set( 'gender-msg-lowercase', '{{gender:$1|he|she}} is awesome' ), 'mw.messages.set: Register' );
-       equal( parser( 'gender-msg-lowercase', 'male' ) , 'he is awesome', 'Gender masculine' );
-       equal( parser( 'gender-msg-lowercase', 'female' ) , 'she is awesome', 'Gender feminine' );
-       ok( mw.messages.set( 'gender-msg-wrong', '{{gender}} is awesome' ), 'mw.messages.set: Register' );
-       equal( parser( 'gender-msg-wrong', 'female' ) , ' is awesome', 'Wrong syntax used, but ignore the {{gender}}' );
+       equal(
+               parser( 'gender-msg', 'Foo', user ),
+               'Foo: green',
+               'Neutral from mw.user object' );
+       equal(
+               parser( 'gender-msg', 'Alice', 'female' ),
+               'Alice: pink',
+               'Feminine from string "female"' );
+       equal(
+               parser( 'gender-msg', 'User' ),
+               'User: green',
+               'Neutral when no parameter given' );
+       equal(
+               parser( 'gender-msg', 'User', 'unknown' ),
+               'User: green',
+               'Neutral from string "unknown"'
+       );
+
+       mw.messages.set( 'gender-msg-one-form', '{{GENDER:$1|User}}: $2 {{PLURAL:$2|edit|edits}}' );
+
+       equal(
+               parser( 'gender-msg-one-form', 'male', 10 ),
+               'User: 10 edits',
+               'Gender neutral and plural form'
+       );
+       equal(
+               parser( 'gender-msg-one-form', 'female', 1 ),
+               'User: 1 edit',
+               'Gender neutral and singular form'
+       );
+
+       mw.messages.set( 'gender-msg-lowercase', '{{gender:$1|he|she}} is awesome' );
+       equal(
+               parser( 'gender-msg-lowercase', 'male' ),
+               'he is awesome',
+               'Gender masculine'
+       );
+       equal(
+               parser( 'gender-msg-lowercase', 'female' ),
+               'she is awesome',
+               'Gender feminine'
+       );
+
+       mw.messages.set( 'gender-msg-wrong', '{{gender}} test' );
+       equal(
+               parser( 'gender-msg-wrong', 'female' ),
+               ' test',
+               'Invalid syntax should result in {{gender}} simply being stripped away'
+       );
 } );
 
 
-test( 'mw.jqueryMsg Grammar', function() {
-       expect( 5 );
+test( 'mw.jqueryMsg Grammar', function () {
+       expect( 2 );
        var parser = mw.jqueryMsg.getMessageFunction();
-       ok( parser, 'Parser Function initialized' );
-       // Hope the grammar form grammar_case_foo is not valid in any language
-       ok( mw.messages.set( 'grammar-msg', 'Przeszukaj {{GRAMMAR:grammar_case_foo|{{SITENAME}}}}' ), 'mw.messages.set: Register' );
-       equal( parser( 'grammar-msg' ) , 'Przeszukaj ' + mw.config.get( 'wgSiteName' ) , 'Grammar Test with sitename' );
-       ok( mw.messages.set( 'grammar-msg-wrong-syntax', 'Przeszukaj {{GRAMMAR:grammar_case_xyz}}' ), 'mw.messages.set: Register' );
-       equal( parser( 'grammar-msg-wrong-syntax' ) , 'Przeszukaj ' , 'Grammar Test with wrong grammar template syntax' );
+
+       // Assume the grammar form grammar_case_foo is not valid in any language
+       mw.messages.set( 'grammar-msg', 'Przeszukaj {{GRAMMAR:grammar_case_foo|{{SITENAME}}}}' );
+       equal( parser( 'grammar-msg' ), 'Przeszukaj ' + mw.config.get( 'wgSiteName' ), 'Grammar Test with sitename' );
+
+       mw.messages.set( 'grammar-msg-wrong-syntax', 'Przeszukaj {{GRAMMAR:grammar_case_xyz}}' );
+       equal( parser( 'grammar-msg-wrong-syntax' ), 'Przeszukaj ' , 'Grammar Test with wrong grammar template syntax' );
 } );