jquery.byteLimit: Partial rewrite to fix logic errors
authorTimo Tijhof <ttijhof@wikimedia.org>
Sat, 23 Jun 2012 02:16:06 +0000 (04:16 +0200)
committerCatrope <roan.kattouw@gmail.com>
Wed, 4 Jul 2012 06:11:11 +0000 (23:11 -0700)
* Previously it was hooking into the keypress event, this meant
  that it had to do all sorts of calculation to figure out what
  the string length would be *after* the to-be-pressed key
  is allowed to be pressed. This sounds trivial, but is quite the
  contrary. First of all are control, alt, shift etc. those were
  covered.

  What wasn't covered and imho can't be realistically covered
  without bugs is: copy/paste, drag/drop, text selection and
  replacing or removing more at once. Or even a combination of the
  above. And then there is custom insert methods such as Narayam,
  where the character will not be literally inserted at all.

  Therefore I've changed it to instead listen to *after* the insert.
  and if necessary undo the last (one or more) characters until the
  byteLimit is satisfied. This works much more stable. Still very
  fast and not noticeable to the user.

  This also fixes the bug where when the limit is reached, you
  select all and type something else, you couldn't because it would
  prevent the keypress since the limit is reached.

* Updated documentation comments.

* Updated unit tests to simulate onkeyup instead of onkeypress.
 - hasLimit option in byteLimitTest() was redundant, using limit
   directly, instead of duplicating it twice.
 - Added more comments and better test names. Tests themselves
   have not changed.
 - Removed redundant tests at the bottom that no longer make sense
   in the new version (and were broken) since the property is
   now always unset instead of just if there is a callback.
   The native limit must not interfere.

Change-Id: I9ace3ab79f488d24ad13baace32118c151c06347

resources/jquery/jquery.byteLimit.js
tests/qunit/suites/resources/jquery/jquery.byteLength.test.js
tests/qunit/suites/resources/jquery/jquery.byteLimit.test.js

index d8f4bfc..e073b71 100644 (file)
@@ -7,20 +7,23 @@
 ( function ( $, undefined ) {
 
        /**
-        * Enforces a byte limit to a textbox, so that UTF-8 entries are counted as well, when, for example,
-        * a database field has a byte limit rather than a character limit.
-        * Plugin rationale: Browser has native maxlength for number of characters, this plugin exists to
-        * limit number of bytes instead.
+        * Enforces a byte limit on an input field, so that UTF-8 entries are counted
+        * as well, when, for example, a database field has a byte limit rather than
+        * a character limit.
+        * Plugin rationale: Browser has native maxlength for number of characters,
+        * this plugin exists to limit number of bytes instead.
         *
-        * Can be called with a custom limit (to use that limit instead of the maxlength attribute value),
-        * a filter function (in case the limit should apply to something other than the exact input value),
-        * or both. Order of arguments is important!
+        * Can be called with a custom limit (to use that limit instead of the
+        * maxlength attribute value), a filter function (in case the limit should
+        * apply to something other than the exact input value), or both.
+        * Order of parameters is important!
         *
-        * @context {jQuery} Instance of jQuery for one or more input elements
-        * @param limit {Number} [optional] Limit to enforce, fallsback to maxLength-attribute,
-        * called with fetched value as argument.
-        * @param fn {Function} [optional] Function to call on the input string before assessing the length
-        * @return {jQuery} The context
+        * @context {jQuery} Instance of jQuery for one or more input elements.
+        * @param limit {Number} [optional] Limit to enforce, fallsback to
+        * maxLength-attribute, called with fetched value as argument.
+        * @param fn {Function} [optional] Function to call on the input string
+        *  before assessing the length.
+        * @return {jQuery} The context.
         */
        $.fn.byteLimit = function ( limit, fn ) {
                // If the first argument is the function,
                                return;
                        }
 
-                       // Update/set attribute value, but only if there is no callback set.
                        // If there's a callback set, it's possible that the limit being enforced
-                       // is too low (ie. if the callback would return "Foo" for "User:Foo").
+                       // is lower or higher (ie. if the callback would return "Foo" for "User:Foo").
                        // Usually this isn't a problem since browsers ignore maxLength when setting
-                       // the value property through JavaScript, but Safari 4 violates that rule, so
-                       // we have to remove or not set the property if we have a callback.
-                       if ( fn === undefined ) {
-                               $el.prop( 'maxLength', elLimit );
-                       } else {
-                               $el.removeProp( 'maxLength' );
-                       }
+                       // the value property through JavaScript, but Safari 4 violates that rule,
+                       // and makes sense to generally make sure the native browser limit doesn't
+                       // interfere
+                       $el.removeProp( 'maxLength' );
        
                        // Save function for reference
-                       $el.data( 'byteLimit-callback', fn );
-       
-                       // We've got something, go for it:
-                       $el.keypress( function ( e ) {
-                               var val, len, charLen;
-                               // First check to see if this is actually a character key
-                               // being pressed.
-                               // Based on key-event info from http://unixpapa.com/js/key.html
-                               // jQuery should also normalize e.which to be consistent cross-browser,
-                               // however the same check is still needed regardless of jQuery.
-       
-                               // Note: At the moment, for some older opera versions (~< 10.5)
-                               // some special keys won't be recognized (aka left arrow key).
-                               // Backspace will be, so not big issue.
-       
-                               if ( e.which === 0 || e.charCode === 0 || e.which === 8 ||
-                                       e.ctrlKey || e.altKey || e.metaKey )
-                               {
-                                       // A special key (backspace, etc) so don't interfere
-                                       return true;
-                               }
-       
-                               val = fn !== undefined ? fn( $( this ).val() ): $( this ).val();
-                               len = $.byteLength( val );
-                               // Note that keypress returns a character code point, not a keycode.
-                               // However, this may not be super reliable depending on how keys come in...
-                               charLen = $.byteLength( String.fromCharCode( e.which ) );
+                       $el.data( 'byteLimitCallback', fn );
        
-                               if ( ( len + charLen ) > elLimit ) {
-                                       e.preventDefault();
+                       // Using keyup instead of keypress so that we don't have to account
+                       // for the infinite number of methods for character insertion (typing,
+                       // holding down for multiple characters, special characters inserted
+                       // with shift/alt, backspace, drag/drop, cut/copy/paste, selecting
+                       // text and replacing).
+                       // Also using onchange. Usually only triggered when field loses focus,
+                       // (incl. before submission), which seems redundant. But this is used
+                       // to allow other JavaScript libraries (e.g. for custom input methods of
+                       // special characters) which tend to trigger onchange as convienience for
+                       // plugins like these.
+                       $el.on( 'keyup change', function ( e ) {
+                               var len,
+                                       $el = $( this ),
+                                       curVal = $el.val(),
+                                       val = curVal;
+
+                               // Run any value modifier (e.g. a function to apply the limit to
+                               // "Foo" in value "User:Foo").
+                               while ( $.byteLength( fn ? fn( val ) : val ) > elLimit ) {
+                                       val = val.substr( 0, val.length - 1 );
+                               };
+
+                               if ( val !== curVal ) {
+                                       $el.val( val );
                                }
                        });
                });
index 15fac69..60d0092 100644 (file)
@@ -5,7 +5,7 @@ test( '-- Initial check', function() {
        ok( $.byteLength, 'jQuery.byteLength defined' );
 } );
 
-test( 'Simple text', function() {
+test( 'Simple text', function () {
        expect(5);
 
        var     azLc = 'abcdefghijklmnopqrstuvwxyz',
@@ -22,7 +22,7 @@ test( 'Simple text', function() {
 
 } );
 
-test( 'Special text', window.foo = function() {
+test( 'Special text', function () {
        expect(5);
 
        // http://en.wikipedia.org/wiki/UTF-8
index 2cb94d1..9a039d6 100644 (file)
@@ -1,75 +1,72 @@
 ( function ( $ ) {
-       var simpleSample, U_20AC, mbSample;
+       var sample20simple, sample1mb, sample22mixed;
 
        module( 'jquery.byteLimit', QUnit.newMwEnvironment() );
 
        // Simple sample (20 chars, 20 bytes)
-       simpleSample = '12345678901234567890';
+       sample20simple = '1234567890abcdefghij';
 
-       // 3 bytes (euro-symbol)
-       U_20AC = '\u20AC';
+       // Euro-symbol (1 char, 3 bytes)
+       sample1mb = '\u20AC';
 
        // Multi-byte sample (22 chars, 26 bytes)
-       mbSample = '1234567890' + U_20AC + '1234567890' + U_20AC;
+       sample22mixed = '1234567890' + sample1mb + 'abcdefghij' + sample1mb;
 
-       // Basic sendkey-implementation
-       function addChars( $input, charstr ) {
-               var len, i, prevVal, code, event;
-               len = charstr.length;
+       /**
+        * Basic emulation of character-by-charater insertion
+        * and triggering of keyup event after each character.
+        */
+       function simulateKeyUps( $input, charstr ) {
+               var i, code, event, liveVal,
+                       len = charstr.length;
                for ( i = 0; i < len; i += 1 ) {
-                       // Keep track of the previous value
-                       prevVal = $input.val();
+                       // Always use the live value as base
+                       liveVal = $input.val();
 
-                       // Get the key code
+                       // Get the key code for the to-be-inserted character
                        code = charstr.charCodeAt( i );
 
-                       // Trigger event and undo if prevented
-                       event = new jQuery.Event( 'keypress', {
-                               which: code,
+                       $input.val( liveVal + charstr.charAt( i ) );
+
+                       // Trigger keyup event
+                       event = new jQuery.Event( 'keyup', {
                                keyCode: code,
                                charCode: code
                        } );
                        $input.trigger( event );
-                       if ( !event.isDefaultPrevented() ) {
-                               $input.val( prevVal + charstr.charAt( i ) );
-                       }
                }
        }
 
        /**
         * Test factory for $.fn.byteLimit
         *
-        * @param $input {jQuery} jQuery object in an input element
-        * @param hasLimit {Boolean} Wether a limit should apply at all
-        * @param limit {Number} Limit (if used) otherwise undefined
-        * The limit should be less than 20 (the sample data's length)
+        * @param Object options
         */
        function byteLimitTest( options ) {
                var opt = $.extend({
                        description: '',
                        $input: null,
                        sample: '',
-                       hasLimit: false,
-                       expected: '',
-                       limit: null
+                       limit: false,
+                       expected: ''
                }, options);
 
                test( opt.description, function () {
-                       var rawVal, fn, newVal;
+                       var rawVal, fn, useVal;
 
                        opt.$input.appendTo( '#qunit-fixture' );
 
-                       // Simulate pressing keys for each of the sample characters
-                       addChars( opt.$input, opt.sample );
+                       simulateKeyUps( opt.$input, opt.sample );
+
                        rawVal = opt.$input.val();
-                       fn = opt.$input.data( 'byteLimit-callback' );
-                       newVal = $.isFunction( fn ) ? fn( rawVal ) : rawVal;
+                       fn = opt.$input.data( 'byteLimitCallback' );
+                       useVal = $.isFunction( fn ) ? fn( rawVal ) : rawVal;
 
-                       if ( opt.hasLimit ) {
-                               expect(3);
+                       if ( opt.limit ) {
+                               expect( 3 );
 
                                QUnit.ltOrEq(
-                                       $.byteLength( newVal ),
+                                       $.byteLength( useVal ),
                                        opt.limit,
                                        'Prevent keypresses after byteLimit was reached, length never exceeded the limit'
                                );
                                equal( rawVal, opt.expected, 'New value matches the expected string' );
 
                        } else {
-                               expect(2);
-                               equal( newVal, opt.expected, 'New value matches the expected string' );
+                               expect( 2 );
+                               equal( useVal, opt.expected, 'New value matches the expected string' );
                                equal(
-                                       $.byteLength( newVal ),
+                                       $.byteLength( useVal ),
                                        $.byteLength( opt.expected ),
                                        'Unlimited scenarios are not affected, expected length reached'
                                );
                description: 'Plain text input',
                $input: $( '<input>' )
                        .attr( 'type', 'text' ),
-               sample: simpleSample,
-               hasLimit: false,
-               expected: simpleSample
+               sample: sample20simple,
+               expected: sample20simple
        });
 
        byteLimitTest({
-               description: 'Plain text input. Calling byteLimit with no parameters and no maxLength property (bug 36310)',
+               description: 'No .byteLimit() parameters and no maxLength property - should not throw exceptions (bug 36310)',
                $input: $( '<input>' )
                        .attr( 'type', 'text' )
                        .byteLimit(),
-               sample: simpleSample,
-               hasLimit: false,
-               expected: simpleSample
+               sample: sample20simple,
+               expected: sample20simple
        });
 
        byteLimitTest({
-               description: 'Limit using the maxlength attribute',
+               description: 'maxLength property',
                $input: $( '<input>' )
                        .attr( 'type', 'text' )
                        .prop( 'maxLength', '10' )
                        .byteLimit(),
-               sample: simpleSample,
-               hasLimit: true,
+               sample: sample20simple,
                limit: 10,
                expected: '1234567890'
        });
 
        byteLimitTest({
-               description: 'Limit using a custom value',
+               description: '.byteLimit( limit )',
                $input: $( '<input>' )
                        .attr( 'type', 'text' )
                        .byteLimit( 10 ),
-               sample: simpleSample,
-               hasLimit: true,
+               sample: sample20simple,
                limit: 10,
                expected: '1234567890'
        });
 
        byteLimitTest({
-               description: 'Limit using a custom value, overriding maxlength attribute',
+               description: 'Limit passed to .byteLimit() takes precedence over maxLength property',
                $input: $( '<input>' )
                        .attr( 'type', 'text' )
                        .prop( 'maxLength', '10' )
                        .byteLimit( 15 ),
-               sample: simpleSample,
-               hasLimit: true,
+               sample: sample20simple,
                limit: 15,
-               expected: '123456789012345'
+               expected: '1234567890abcde'
        });
 
        byteLimitTest({
-               description: 'Limit using a custom value (multibyte)',
+               description: '.byteLimit( limit ) - mixed - cut in simplepart',
                $input: $( '<input>' )
                        .attr( 'type', 'text' )
                        .byteLimit( 14 ),
-               sample: mbSample,
-               hasLimit: true,
+               sample: sample22mixed,
                limit: 14,
-               expected: '1234567890' + U_20AC + '1'
+               expected: '1234567890' + sample1mb + 'a'
        });
 
        byteLimitTest({
-               description: 'Limit using a custom value (multibyte) overlapping a byte',
+               description: '.byteLimit( limit ) - mixed - cut in multibyte',
                $input: $( '<input>' )
                        .attr( 'type', 'text' )
                        .byteLimit( 12 ),
-               sample: mbSample,
-               hasLimit: true,
+               sample: sample22mixed,
                limit: 12,
-               expected: '1234567890' + '12'
+               // The 3-byte symbol after 0 would have exceeded the 12 byte limit.
+               // Later when the simulation resumed typing, the two simple characters
+               // were allowed.
+               expected: '1234567890' + 'ab'
        });
 
        byteLimitTest({
-               description: 'Pass the limit and a callback as input filter',
+               description: '.byteLimit( limit, fn ) callback can alter the value to be checked against',
                $input: $( '<input>' )
                        .attr( 'type', 'text' )
                        .byteLimit( 6, function ( val ) {
-                               // Invalid title
+                               // Only construct mw.Title if the string is non-empty,
+                               // since mw.Title throws an exception on invalid titles.
                                if ( val === '' ) {
                                        return '';
                                }
 
-                               // Return without namespace prefix
+                               // Example: Use value without namespace prefix.
                                return new mw.Title( String( val ) ).getMain();
                        } ),
-               sample: 'User:Sample',
-               hasLimit: true,
-               limit: 6, // 'Sample' length
-               expected: 'User:Sample'
+               sample: 'User:John',
+               // Limit is 6, text "User:Sample" would normally be too long,
+               // but in this case we test the callback's ability to only
+               // apply the limit to part of the input. The part "John" in this
+               // case is within the limit.
+               limit: 6,
+               // The callback only affects the comparison, the actual input field
+               // should still contain the full value.
+               expected: 'User:John'
        });
 
        byteLimitTest({
-               description: 'Limit using the maxlength attribute and pass a callback as input filter',
+               description: '.byteLimit( fn ) combined with maxLength property',
                $input: $( '<input>' )
                        .attr( 'type', 'text' )
                        .prop( 'maxLength', '6' )
                                return new mw.Title( String( val ) ).getMain();
                        } ),
                sample: 'User:Sample',
-               hasLimit: true,
                limit: 6, // 'Sample' length
                expected: 'User:Sample'
        });
 
-       test( 'Confirm properties and attributes set', function () {
-               var $el, $elA, $elB;
-
-               expect(5);
-
-               $el = $( '<input>' )
-                       .attr( 'type', 'text' )
-                       .prop( 'maxLength', '7' )
-                       .appendTo( '#qunit-fixture' )
-                       .byteLimit();
-
-               strictEqual( $el.prop( 'maxLength' ), 7, 'Pre-set maxLength property unchanged' );
-
-               $el = $( '<input>' )
-                       .attr( 'type', 'text' )
-                       .prop( 'maxLength', '7' )
-                       .appendTo( '#qunit-fixture' )
-                       .byteLimit( 12 );
-
-               strictEqual( $el.prop( 'maxLength' ), 12, 'maxLength property updated if value was passed to $.fn.byteLimit' );
-
-               $elA = $( '<input>' )
-                       .addClass( 'mw-test-byteLimit-foo' )
-                       .attr( 'type', 'text' )
-                       .prop( 'maxLength', '7' )
-                       .appendTo( '#qunit-fixture' );
-
-               $elB = $( '<input>' )
-                       .addClass( 'mw-test-byteLimit-foo' )
-                       .attr( 'type', 'text' )
-                       .prop( 'maxLength', '12' )
-                       .appendTo( '#qunit-fixture' );
-
-               $el = $( '.mw-test-byteLimit-foo' );
-
-               strictEqual( $el.length, 2, 'Verify that there are no other elements clashing with this test suite' );
-
-               $el.byteLimit();
-
-               // Before bug 35294 was fixed, both $elA and $elB had maxLength set to 7,
-               // because $.fn.byteLimit sets:
-               // `limit = limitArg || this.prop( 'maxLength' ); this.prop( 'maxLength', limit )`
-               // and did so outside the each() loop.
-               strictEqual( $elA.prop( 'maxLength' ), 7, 'maxLength was not incorrectly set on #1 when calling byteLimit on multiple elements (bug 35294)' );
-               strictEqual( $elB.prop( 'maxLength' ), 12, 'maxLength was not incorrectly set on #2 when calling byteLimit on multiple elements (bug 35294)' );
-       });
-
 }( jQuery ) );