r92113 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r92112‎ | r92113 | r92114 >
Date:22:37, 13 July 2011
Author:krinkle
Status:resolved (Comments)
Tags:
Comment:
Unit tests for jquery.localize
(Follow-up r92069)

Also fixing a bug in jquery.localize. Previously it did mw.msg(key, [p1, p2, ...]); (passing an array as second argument), however mw.msg doesn't take an array as second argument, instead the second argument is the first in a list of optional variadic arguments. (like mw.msg(key, p1, p2, ..);

In cases were only 1 variable is passed, this didn't brake the test as [p1].toString() === p1. Fixing by using .apply instead and creating an array of arguments, starting with the key (unshifted) and if available adding parameters.

All these tests appear to be broken in Gecko-browsers because it uses a different attribute order (WebKit moslty in order of touch, Gecko mostly alphabetical, Trident/Presto engine different too). They are reported because the elements are compared as strings. I'll fix this in a better way when I think of one (soon!)
Modified paths:
  • /trunk/phase3/resources/jquery/jquery.localize.js (modified) (history)
  • /trunk/phase3/tests/qunit/index.html (modified) (history)
  • /trunk/phase3/tests/qunit/suites/resources/jquery/jquery.localize.js (added) (history)

Diff [purge]

Index: trunk/phase3/tests/qunit/index.html
——@@ -43,6 +43,7 @@
4444 <script src="../../resources/jquery/jquery.byteLength.js"></script>
4545 <script src="../../resources/jquery/jquery.byteLimit.js"></script>
4646 <script src="../../resources/jquery/jquery.colorUtil.js"></script>
 47+ <script src="../../resources/jquery/jquery.localize.js"></script>
4748 <script src="../../resources/jquery/jquery.tabIndex.js"></script>
4849 <script src="../../resources/jquery/jquery.tablesorter.js"></script>
4950 <script src="../../resources/mediawiki/mediawiki.Title.js"></script>
——@@ -67,6 +68,7 @@
6869 <script src="suites/resources/jquery/jquery.byteLength.js"></script>
6970 <script src="suites/resources/jquery/jquery.byteLimit.js"></script>
7071 <script src="suites/resources/jquery/jquery.colorUtil.js"></script>
 72+ <script src="suites/resources/jquery/jquery.localize.js"></script>
7173 <script src="suites/resources/jquery/jquery.tabIndex.js"></script>
7274 <script src="suites/resources/jquery/jquery.tablesorter.test.js" charset="UTF-8"></script>
7375 <script src="suites/resources/mediawiki/mediawiki.Title.js"></script>
Index: trunk/phase3/tests/qunit/suites/resources/jquery/jquery.localize.js
——@@ -0,0 +1,134 @@
 2+module( 'jquery.localize.js' );
 3+
 4+test( '-- Initial check', function() {
 5+ expect(1);
 6+ ok( $.fn.localize, 'jQuery.fn.localize defined' );
 7+} );
 8+
 9+test( 'Handle basic replacements', function() {
 10+ expect(4);
 11+
 12+ var html, $lc, expected;
 13+ mw.messages.set( 'basic', 'Basic stuff' );
 14+
 15+ // Tag: html:msg
 16+ html = '<div><span class="foobar"><html:msg key="basic"></span></div>';
 17+ $lc = $( html ).localize();
 18+ expected = '<span class="foobar">Basic stuff</span>';
 19+
 20+ strictEqual( $lc.html(), expected, 'Tag: html:msg' );
 21+
 22+ // Tag: msg (deprecated)
 23+ html = '<div><span class="foobar"><msg key="basic"></span></div>';
 24+ $lc = $( html ).localize();
 25+ expected = '<span class="foobar">Basic stuff</span>';
 26+
 27+ strictEqual( $lc.html(), expected, 'Tag: msg' );
 28+
 29+ // Attribute: title-msg
 30+ html = '<div><span class="foobar" title-msg="basic"></span></div>';
 31+ $lc = $( html ).localize();
 32+ expected = '<span class="foobar" title="Basic stuff"></span>';
 33+
 34+ strictEqual( $lc.html(), expected, 'Attribute: title-msg' );
 35+
 36+ // Attribute: alt-msg
 37+ html = '<div><span class="foobar" alt-msg="basic"></span></div>';
 38+ $lc = $( html ).localize();
 39+ expected = '<span class="foobar" alt="Basic stuff"></span>';
 40+
 41+ strictEqual( $lc.html(), expected, 'Attribute: alt-msg' );
 42+} );
 43+
 44+test( 'Proper escaping', function() {
 45+ expect(2);
 46+
 47+ var html, $lc, expected;
 48+ mw.messages.set( 'properfoo', '<proper esc="test">' );
 49+
 50+ // This is handled by jQuery inside $.fn.localize, just a simple sanity checked
 51+ // making sure it is actually using text() and attr() (or something with the same effect)
 52+
 53+ // Text escaping
 54+ html = '<div><span class="foobar"><html:msg key="properfoo"></span></div>';
 55+ $lc = $( html ).localize();
 56+ expected = '<span class="foobar">&lt;proper esc="test"&gt;</span>';
 57+
 58+ strictEqual( $lc.html(), expected, 'Content is inserted as text, not as html.' );
 59+
 60+ // Attribute escaping
 61+ html = '<div><span class="foobar" title-msg="properbar"></span></div>';
 62+ $lc = $( html ).localize();
 63+ expected = '<span class="foobar" title="&lt;properbar&gt;"></span>';
 64+
 65+ strictEqual( $lc.html(), expected, 'Attributes are not inserted raw.' );
 66+
 67+
 68+} );
 69+
 70+test( 'Options', function() {
 71+ expect(4);
 72+
 73+ mw.messages.set( {
 74+ 'foo-lorem': 'Lorem',
 75+ 'foo-ipsum': 'Ipsum',
 76+ 'foo-bar-title': 'Read more about bars',
 77+ 'foo-bar-label': 'The Bars',
 78+ 'foo-bazz-title': 'Read more about bazz at $1 (last modified: $2)',
 79+ 'foo-bazz-label': 'The Bazz ($1)',
 80+ 'foo-welcome': 'Welcome to $1! (last visit: $2)'
 81+ } );
 82+ var html, $lc, expected, x, sitename = 'Wikipedia';
 83+
 84+ // Message key prefix
 85+ html = '<div><span title-msg="lorem"><html:msg key="ipsum" /></span></div>';
 86+ $lc = $( html ).localize( {
 87+ prefix: 'foo-'
 88+ } );
 89+ expected = '<span title="Lorem">Ipsum</span>';
 90+
 91+ strictEqual( $lc.html(), expected, 'Message key prefix' );
 92+
 93+ // Variable keys mapping
 94+ x = 'bar';
 95+ html = '<div><span title-msg="title"><html:msg key="label" /></span></div>';
 96+ $lc = $( html ).localize( {
 97+ keys: {
 98+ 'title': 'foo-' + x + '-title',
 99+ 'label': 'foo-' + x + '-label'
 100+ }
 101+ } );
 102+ expected = '<span title="Read more about bars">The Bars</span>';
 103+
 104+ strictEqual( $lc.html(), expected, 'Message prefix' );
 105+
 106+ // Passing parameteters to mw.msg
 107+ html = '<div><span><html:msg key="foo-welcome" /></span></div>';
 108+ $lc = $( html ).localize( {
 109+ params: {
 110+ 'foo-welcome': [sitename, 'yesterday']
 111+ }
 112+ } );
 113+ expected = '<span>Welcome to Wikipedia! (last visit: yesterday)</span>';
 114+
 115+ strictEqual( $lc.html(), expected, 'Message prefix' );
 116+
 117+ // Combination of options prefix, params and keys
 118+ x = 'bazz';
 119+ html = '<div><span title-msg="title"><html:msg key="label" /></span></div>';
 120+ $lc = $( html ).localize( {
 121+ prefix: 'foo-',
 122+ keys: {
 123+ 'title': x + '-title',
 124+ 'label': x + '-label'
 125+ },
 126+ params: {
 127+ 'title': [sitename, '3 minutes ago'],
 128+ 'label': [sitename, '3 minutes ago']
 129+
 130+ }
 131+ } );
 132+ expected = '<span title="Read more about bazz at Wikipedia (last modified: 3 minutes ago)">The Bazz (Wikipedia)</span>';
 133+
 134+ strictEqual( $lc.html(), expected, 'Message prefix' );
 135+} );
Property changes on: trunk/phase3/tests/qunit/suites/resources/jquery/jquery.localize.js
___________________________________________________________________
Added: svn:eol-style
1136 + native
Index: trunk/phase3/resources/jquery/jquery.localize.js
——@@ -34,7 +34,10 @@
3535 $.fn.localize = function( options ) {
3636 options = $.extend( { 'prefix': '', 'keys': {}, 'params': {} }, options );
3737 function msg( key ) {
38 - return mw.msg( options.prefix + ( key in options.keys ? options.keys[key] : key ), ( key in options.params ? options.params[key] : [] ) )
 38+ var args = key in options.params ? options.params[key] : [];
 39+ // Format: mw.msg( key [, p1, p2, ...] )
 40+ args.unshift( options.prefix + ( key in options.keys ? options.keys[key] : key ) );
 41+ return mw.msg.apply( mw, args );
3942 };
4043 return $(this)
4144 .find( 'msg,html\\:msg' )

Follow-up revisions

RevisionCommit summaryAuthorDate
r92116New jquery.getAttrs module...krinkle22:46, 13 July 2011
r92125Fix broken test introduced in r92113 for Gecko-browsers....krinkle23:31, 13 July 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r92069jquery.localize:...krinkle17:57, 13 July 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   17:25, 14 July 2011

test failures on gecko, IE, opera

#Comment by Krinkle (talk | contribs)   20:21, 14 July 2011

Gecko (and opera) fixed in r92125 as seen here , between 213 and 214.

#Comment by Brion VIBBER (talk | contribs)   00:47, 15 July 2011

For IE tests see my comment on r80116 -- the selector doesn't seem to work on IE 8 (maybe this regresed from older jquery versions in use at the time?)

#Comment by Krinkle (talk | contribs)   20:57, 27 July 2011

The follow up commits by brion and me to jquery.localize and the unit test suite fixed all issues. It's passing 100% as of r92758 (TestSwarm results). Marking resolved.

Status & tagging log