r91844 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r91843‎ | r91844 | r91845 >
Date:21:01, 10 July 2011
Author:krinkle
Status:resolved (Comments)
Tags:
Comment:
Add test suite for jquery.byteLimit:
- Tests to verify that the byteLength does not exceed the byteLimit when inserting more characters
- Tests to verify that it doesn't prevent too early (ie. if limit is 10 and we insert 20 characters, there should be 10 characters in the input field).

This test suite has exposed that the latter is currently broken. jquery.byteLimit is preventing characters to be added as soon as byteLength(currentValue) + 3 is more than the given limit.

(Follows-up r86698, r91148)
Modified paths:
  • /trunk/phase3/tests/qunit/index.html (modified) (history)
  • /trunk/phase3/tests/qunit/suites/resources/jquery/jquery.byteLimit.js (added) (history)

Diff [purge]

Index: trunk/phase3/tests/qunit/index.html
@@ -41,6 +41,7 @@
4242 <!-- MW: Non-default modules -->
4343 <script src="../../resources/jquery/jquery.autoEllipsis.js"></script>
4444 <script src="../../resources/jquery/jquery.byteLength.js"></script>
 45+ <script src="../../resources/jquery/jquery.byteLimit.js"></script>
4546 <script src="../../resources/jquery/jquery.colorUtil.js"></script>
4647 <script src="../../resources/jquery/jquery.tabIndex.js"></script>
4748 <script src="../../resources/jquery/jquery.tablesorter.js"></script>
@@ -64,6 +65,7 @@
6566
6667 <script src="suites/resources/jquery/jquery.autoEllipsis.js"></script>
6768 <script src="suites/resources/jquery/jquery.byteLength.js"></script>
 69+ <script src="suites/resources/jquery/jquery.byteLimit.js"></script>
6870 <script src="suites/resources/jquery/jquery.colorUtil.js"></script>
6971 <script src="suites/resources/jquery/jquery.tabIndex.js"></script>
7072 <script src="suites/resources/jquery/jquery.tablesorter.test.js" charset="UTF-8"></script>
Index: trunk/phase3/tests/qunit/suites/resources/jquery/jquery.byteLimit.js
@@ -0,0 +1,155 @@
 2+module( 'jquery.byteLimit.js' );
 3+
 4+test( '-- Initial check', function() {
 5+ expect(1);
 6+ ok( $.fn.byteLimit, 'jQuery.fn.byteLimit defined' );
 7+} );
 8+
 9+// Basic sendkey-implementation
 10+$.addChars = function( $input, charstr ) {
 11+ var len = charstr.length;
 12+ for ( var i = 0; i < len; i++ ) {
 13+ // Keep track of the previous value
 14+ var prevVal = $input.val();
 15+
 16+ // Get the key code
 17+ var code = charstr.charCodeAt(i);
 18+
 19+ // Trigger event and undo if prevented
 20+ var event = new jQuery.Event( 'keypress', { keyCode: code, which: code, charCode: code } );
 21+ $input.trigger( event );
 22+ if ( !event.isDefaultPrevented() ) {
 23+ $input.val( prevVal + charstr[i] );
 24+ }
 25+ }
 26+};
 27+var blti = 0;
 28+/**
 29+ * Test factory for $.fn.byteLimit
 30+ *
 31+ * @param $input {jQuery} jQuery object in an input element
 32+ * @param useLimit {Boolean} Wether a limit should apply at all
 33+ * @param limit {Number} Limit (if used) otherwise undefined
 34+ * The limit should be less than 20 (the sample data's length)
 35+ */
 36+var byteLimitTest = function( options ) {
 37+ var opt = $.extend({
 38+ description: '',
 39+ $input: null,
 40+ sample: '',
 41+ useLimit: false,
 42+ expected: 0,
 43+ limit: null
 44+ }, options);
 45+ var i = blti++;
 46+
 47+ test( opt.description, function() {
 48+
 49+ opt.$input.appendTo( 'body' );
 50+
 51+ // Simulate pressing keys for each of the sample characters
 52+ $.addChars( opt.$input, opt.sample );
 53+ var newVal = opt.$input.val();
 54+
 55+ if ( opt.useLimit ) {
 56+ expect(2);
 57+
 58+ ltOrEq( $.byteLength( newVal ), opt.limit, 'Prevent keypresses after byteLimit was reached, length never exceeded the limit' );
 59+ equal( $.byteLength( newVal ), opt.expected, 'Not preventing keypresses too early, length has reached the expected length' );
 60+
 61+ } else {
 62+ expect(1);
 63+ equal( $.byteLength( newVal ), opt.expected, 'Unlimited scenarios are not affected, expected length reached' );
 64+ }
 65+
 66+ opt.$input.remove();
 67+ } );
 68+};
 69+
 70+var
 71+ // Simple sample (20 chars, 20 bytes)
 72+ simpleSample = '12345678901234567890',
 73+
 74+ // 3 bytes (euro-symbol)
 75+ U_20AC = '\u20AC',
 76+
 77+ // Multi-byte sample (22 chars, 26 bytes)
 78+ mbSample = '1234567890' + U_20AC + '1234567890' + U_20AC;
 79+
 80+byteLimitTest({
 81+ description: 'Plain text input',
 82+ $input: $( '<input>' )
 83+ .attr( {
 84+ 'type': 'text'
 85+ }),
 86+ sample: simpleSample,
 87+ useLimit: false,
 88+ expected: $.byteLength( simpleSample )
 89+});
 90+
 91+byteLimitTest({
 92+ description: 'Limit using the maxlength attribute',
 93+ $input: $( '<input>' )
 94+ .attr( {
 95+ 'type': 'text',
 96+ 'maxlength': '10'
 97+ })
 98+ .byteLimit(),
 99+ sample: simpleSample,
 100+ useLimit: true,
 101+ limit: 10,
 102+ expected: 10
 103+});
 104+
 105+byteLimitTest({
 106+ description: 'Limit using a custom value',
 107+ $input: $( '<input>' )
 108+ .attr( {
 109+ 'type': 'text'
 110+ })
 111+ .byteLimit( 10 ),
 112+ sample: simpleSample,
 113+ useLimit: true,
 114+ limit: 10,
 115+ expected: 10
 116+});
 117+
 118+byteLimitTest({
 119+ description: 'Limit using a custom value, overriding maxlength attribute',
 120+ $input: $( '<input>' )
 121+ .attr( {
 122+ 'type': 'text',
 123+ 'maxLength': '10'
 124+ })
 125+ .byteLimit( 15 ),
 126+ sample: simpleSample,
 127+ useLimit: true,
 128+ limit: 15,
 129+ expected: 15
 130+});
 131+
 132+byteLimitTest({
 133+ description: 'Limit using a custom value (multibyte)',
 134+ $input: $( '<input>' )
 135+ .attr( {
 136+ 'type': 'text'
 137+ })
 138+ .byteLimit( 14 ),
 139+ sample: mbSample,
 140+ useLimit: true,
 141+ limit: 14,
 142+ expected: 14 // (10 x 1-byte char) + (1 x 3-byte char) + (1 x 1-byte char)
 143+});
 144+
 145+byteLimitTest({
 146+ description: 'Limit using a custom value (multibyte) overlapping a byte',
 147+ $input: $( '<input>' )
 148+ .attr( {
 149+ 'type': 'text'
 150+ })
 151+ .byteLimit( 12 ),
 152+ sample: mbSample,
 153+ useLimit: true,
 154+ limit: 12,
 155+ expected: 10 // 10 x 1-byte char. The next 3-byte char exceeds limit of 12
 156+});
Property changes on: trunk/phase3/tests/qunit/suites/resources/jquery/jquery.byteLimit.js
___________________________________________________________________
Added: svn:eol-style
1157 + native

Follow-up revisions

RevisionCommit summaryAuthorDate
r91891* (bug 29804) Fix byte-length-limited fields to check the length of to-be-add...brion17:53, 11 July 2011
r91894Followup r91844: fix jquery.byteLimit test cases on IE6, IE7...brion18:14, 11 July 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r86698(bug 28650) Refactor dynamic byte-based maxlength of edit summary into a jQue...catrope10:58, 22 April 2011
r91148byteLimit/byteLength improvements:...krinkle01:06, 30 June 2011

Comments

#Comment by Krinkle (talk | contribs)   21:02, 10 July 2011

Breakage recorded as bugzilla:29804 (jquery.byteLimit should allow the limit to be reached).

Note that this revision didn't introduce the bug, it merely exposed it through a unit test.

#Comment by Brion VIBBER (talk | contribs)   17:58, 11 July 2011

r91891 fixes the test cases, including a correction for the last test which actually ends up with a length of 12 as there are two more ASCII chars to be inserted.

Note that I'm leaving that bug open as there seem to be some general issues, and further testing is required to see how things actually work, especially with non-BOM characters and funky input method stuff.

#Comment by Brion VIBBER (talk | contribs)   18:16, 11 July 2011

r91894 fixes another bug in the test cases; using the array operator to get a char from a string (str[i]) doesn't work in IE6 or IE 7, which caused the simulated keypress stuff to fail to update the field properly. Switching to str.charAt(i) fixes this right up!

Status & tagging log