jquery.byteLimit: Rewrite (fix bug 38158, bug 38163)
authorTimo Tijhof <ttijhof@wikimedia.org>
Fri, 14 Sep 2012 02:09:47 +0000 (04:09 +0200)
committerTimo Tijhof <ttijhof@wikimedia.org>
Tue, 2 Oct 2012 02:54:48 +0000 (04:54 +0200)
It used to be fairly simply and that seemed good enough. Listen
on keypress, get current value, add character from key code in
event object, and calculate the byteLength. If it is too long,
preventDefault().

However there were so many edge cases (too many of them to be
considered edge cases) where this failed whereas the native
maxLength handling was fine.

For example:
 - Cut and paste
 - Selecting text and replacing or removing it (by any of the
   methods on this list)
 - Custom javascript input methods (e.g. Narayam)
 - Spelling suggestions and corrections
 - Drag and drop
 - etc.

Now it acts on changes after the fact.

I considered building in a timeout loop in addition to this so
that when javascript gadgets change the input value it would
detect it, however I'd like to hold that off for now. We can
add that later if needed. For now scripts are expected to fire
the 'change' event if they change it, which seems reasonable.

The only challenge that it creates, however, is the need for
basic history reconstruction. Figure out what was already there
and what is new, and cut down the new part until the total sum
satisfies the limit.

This is the same behavior that WebKit browsers (and others) have.
e.g. of "barbaz" is pasted between "f" and "o" in "foo" with a
maxLength of 5, then the resulting string will be "fbaoo".

Also fixed bug 38158 while at it. Filed an upstream bug against
webkit/Chrome but was wont fixed. Either way, we'd have to be
compatible for a little while with old WebKit builds out there
(iOS 3, Safari 5, etc.).
http://code.google.com/p/chromium/issues/detail?id=136004

Use removeAttribute('maxlength') to let the browser internally
reset the maxLength property. Resetting it directly (to undefined
or with `delete el.maxLength') does not work and casts it to 0,
which blocks the input field entirely.

Updated unit tests, and also removed the "type=text" code
afterwards which that doesn't work in IE (Internet Explorer does
not support dynamic changing of input types), needs to be in the
HTML directly. So far it was passing in IE because 'text' is the
default. Tests now use attr('maxlength') everywhere for
consistency, though properties work the same. Browsers
synchronize these things (just like the "class" attribute and
"className" property).

References:
* Fixes bug 38158: Unexpected 0 maxLength.
* Fixes bug 38163: Incorrect limiting of input when using input
  methods other than basic per-char typing.
* Follows-up: 39cb0c19
* Supersedes: I70d09aae

Change-Id: I9c204243d0bdf7158aa86a62b591ce717a36fe27

RELEASE-NOTES-1.20
resources/jquery/jquery.byteLimit.js
tests/qunit/suites/resources/jquery/jquery.byteLimit.test.js

index 3e68abd..bf90c16 100644 (file)
@@ -249,6 +249,9 @@ upgrade PHP if you have not done so prior to upgrading MediaWiki.
 * (bug 39941) Add missing stylesheets to the installer pages
 * In HTML5 mode, allow new input element types values (such as color, range..)
 * (bug 36151) mw.Title: Don't limit extension in title parsing.
+* (bug 38158) jquery.byteLimit sometimes causes an unexpected 0 maxLength being enforced.
+* (bug 38163) jquery.byteLimit incorrectly limits input when using methods other than
+  basic per-char typing.
 
 === API changes in 1.20 ===
 * (bug 34316) Add ability to retrieve maximum upload size from MediaWiki API.
index 484651e..75dc2b9 100644 (file)
@@ -1,5 +1,5 @@
 /**
- * jQuery byteLimit plugin
+ * jQuery byteLimit plugin.
  *
  * @author Jan Paul Posma, 2011
  * @author Timo Tijhof, 2011-2012
 ( function ( $ ) {
 
        /**
-        * 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.
+        * Utility function to trim down a string, based on byteLimit
+        * and given a safe start position. It supports insertion anywhere
+        * in the string, so "foo" to "fobaro" if limit is 4 will result in
+        * "fobo", not "foba". Basically emulating the native maxlength by
+        * reconstructing where the insertion occured.
         *
-        * 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!
+        * @param {string} safeVal Known value that was previously returned by this
+        * function, if none, pass empty string.
+        * @param {string} newVal New value that may have to be trimmed down.
+        * @param {number} byteLimit Number of bytes the value may be in size.
+        * @param {Function} fn [optional] See $.fn.byteLimit.
+        * @return {Object} Object with:
+        *  - {string} newVal
+        *  - {boolean} trimmed
+        */
+       function trimValForByteLength( safeVal, newVal, byteLimit, fn ) {
+               var startMatches, endMatches, matchesLen, inpParts,
+                       oldVal = safeVal;
+
+               // Run the hook if one was provided, but only on the length
+               // assessment. The value itself is not to be affected by the hook.
+               if ( $.byteLength( fn ? fn( newVal ) : newVal ) <= byteLimit ) {
+                       // Limit was not reached, just remember the new value
+                       // and let the user continue.
+                       return {
+                               newVal: newVal,
+                               trimmed: false
+                       };
+               }
+
+               // Current input is longer than the active limit.
+               // Figure out what was added and limit the addition.
+               startMatches = 0;
+               endMatches = 0;
+
+               // It is important that we keep the search within the range of
+               // the shortest string's length.
+               // Imagine a user adds text that matches the end of the old value
+               // (e.g. "foo" -> "foofoo"). startMatches would be 3, but without
+               // limiting both searches to the shortest length, endMatches would
+               // also be 3.
+               matchesLen = Math.min( newVal.length, oldVal.length );
+
+               // Count same characters from the left, first.
+               // (if "foo" -> "foofoo", assume addition was at the end).
+               while (
+                       startMatches < matchesLen &&
+                       oldVal.charAt( startMatches ) === newVal.charAt( startMatches )
+               ) {
+                       startMatches += 1;
+               }
+
+               while (
+                       endMatches < ( matchesLen - startMatches ) &&
+                       oldVal.charAt( oldVal.length - 1 - endMatches ) === newVal.charAt( newVal.length - 1 - endMatches )
+               ) {
+                       endMatches += 1;
+               }
+
+               inpParts = [
+                       // Same start
+                       newVal.substring( 0, startMatches ),
+                       // Inserted content
+                       newVal.substring( startMatches, newVal.length - endMatches ),
+                       // Same end
+                       newVal.substring( newVal.length - endMatches )
+               ];
+
+               // Chop off characters from the end of the "inserted content" string
+               // until the limit is statisfied.
+               if ( fn ) {
+                       while ( $.byteLength( fn( inpParts.join( '' ) ) ) > byteLimit ) {
+                               inpParts[1] = inpParts[1].slice( 0, -1 );
+                       }
+               } else {
+                       while ( $.byteLength( inpParts.join( '' ) ) > byteLimit ) {
+                               inpParts[1] = inpParts[1].slice( 0, -1 );
+                       }
+               }
+
+               newVal = inpParts.join( '' );
+
+               return {
+                       newVal: newVal,
+                       trimmed: true
+               };
+       }
+
+       var eventKeys = [
+               'keyup.byteLimit',
+               'keydown.byteLimit',
+               'change.byteLimit',
+               'mouseup.byteLimit',
+               'cut.byteLimit',
+               'paste.byteLimit',
+               'focus.byteLimit',
+               'blur.byteLimit'
+       ].join( ' ' );
+
+       /**
+        * 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 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
+        * @param {Number} limit [optional] Limit to enforce, fallsback to maxLength-attribute,
+        *  called with fetched value as argument.
+        * @param {Function} fn [optional] Function to call on the string before assessing the length.
         * @return {jQuery} The context
         */
        $.fn.byteLimit = function ( limit, fn ) {
                if ( $.isFunction( limit ) ) {
                        fn = limit;
                        limit = undefined;
+               // Either way, verify it is a function so we don't have to call
+               // isFunction again after this.
+               } else if ( !fn || !$.isFunction( fn ) ) {
+                       fn = undefined;
                }
 
-               // The following is specific to each element in the collection
+               // The following is specific to each element in the collection.
                return this.each( function ( i, el ) {
-                       var $el, elLimit;
+                       var $el, elLimit, prevSafeVal;
 
                        $el = $( el );
 
-                       // Default limit to current attribute value
+                       // If no limit was passed to byteLimit(), use the maxlength value.
                        // Can't re-use 'limit' variable because it's in the higher scope
-                       // that affects the next each() iteration as well.
-                       elLimit = limit === undefined ? $el.prop( 'maxLength' ) : limit;
-       
+                       // that would affect the next each() iteration as well.
+                       // Note that we use attribute to read the value instead of property,
+                       // because in Chrome the maxLength property by default returns the
+                       // highest supported value (no indication that it is being enforced
+                       // by choice). We don't want to bind all of this for some ridiculously
+                       // high default number, unless it was explicitly set in the HTML.
+                       // Also cast to a (primitive) number (most commonly because the maxlength
+                       // attribute contains a string, but theoretically the limit parameter
+                       // could be something else as well).
+                       elLimit = Number( limit === undefined ? $el.attr( 'maxlength' ) : limit );
+
                        // If there is no (valid) limit passed or found in the property,
                        // skip this. The < 0 check is required for Firefox, which returns
                        // -1  (instead of undefined) for maxLength if it is not set.
                                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").
-                       // 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 );
+                       if ( fn ) {
+                               // Save function for reference
+                               $el.data( 'byteLimit.callback', fn );
+                       }
+
+                       // Remove old event handlers (if there are any)
+                       $el.off( '.byteLimit' );
+
+                       if ( fn ) {
+                               // Disable the native maxLength (if there is any), because it interferes
+                               // with the (differently calculated) byte limit.
+                               // Aside from being differently calculated (average chars with byteLimit
+                               // is lower), we also support a callback which can make it to allow longer
+                               // values (e.g. count "Foo" from "User:Foo").
+                               // maxLength is a strange property. Removing or setting the property to
+                               // undefined directly doesn't work. Instead, it can only be unset internally
+                               // by the browser when removing the associated attribute (Firefox/Chrome).
+                               // http://code.google.com/p/chromium/issues/detail?id=136004
+                               $el.removeAttr( 'maxlength' );
+
                        } else {
-                               $el.removeProp( 'maxLength' );
+                               // If we don't have a callback the bytelimit can only be lower than the charlimit
+                               // (that is, there are no characters less than 1 byte in size). So lets (re-)enforce
+                               // the native limit for efficiency when possible (it will make the while-loop below
+                               // faster by there being less left to interate over).
+                               $el.attr( 'maxlength', elLimit );
                        }
-       
-                       // Save function for reference
-                       $el.data( 'byteLimitCallback', 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 ) );
-       
-                               if ( ( len + charLen ) > elLimit ) {
-                                       e.preventDefault();
+
+
+                       // Safe base value, used to determine the path between the previous state
+                       // and the state that triggered the event handler below - and enforce the
+                       // limit approppiately (e.g. don't chop from the end if text was inserted
+                       // at the beginning of the string).
+                       prevSafeVal = '';
+
+                       // We need to listen to after the change has already happened because we've
+                       // learned that trying to guess the new value and canceling the event
+                       // accordingly doesn't work because the new value is not always as simple as:
+                       // oldValue + String.fromCharCode( e.which ); because of cut, paste, select-drag
+                       // replacements, and custom input methods and what not.
+                       // Even though we only trim input after it was changed (never prevent it), we do
+                       // listen on events that input text, because there are cases where the text has
+                       // changed while text is being entered and keyup/change will not be fired yet
+                       // (such as holding down a single key, fires keydown, and after each keydown,
+                       // we can trim the previous one).
+                       // See http://www.w3.org/TR/DOM-Level-3-Events/#events-keyboard-event-order for
+                       // the order and characteristics of the key events.
+                       $el.on( eventKeys, function () {
+                               var res = trimValForByteLength(
+                                       prevSafeVal,
+                                       this.value,
+                                       elLimit,
+                                       fn
+                               );
+
+                               // Only set value property if it was trimmed, because whenever the
+                               // value property is set, the browser needs to re-initiate the text context,
+                               // which moves the cursor at the end the input, moving it away from wherever it was.
+                               // This is a side-effect of limiting after the fact.
+                               if ( res.trimmed === true ) {
+                                       this.value = res.newVal;
+                                       prevSafeVal = res.newVal;
                                }
-                       });
-               });
+                       } );
+               } );
        };
 }( jQuery ) );
index 23a93a7..4f86eb9 100644 (file)
@@ -1,4 +1,4 @@
-( function ( $ ) {
+( function ( $, mw ) {
        var simpleSample, U_20AC, mbSample;
 
        QUnit.module( 'jquery.byteLimit', QUnit.newMwEnvironment() );
 
        // Basic sendkey-implementation
        function addChars( $input, charstr ) {
-               var len, i, prevVal, code, event;
-               len = charstr.length;
-               for ( i = 0; i < len; i += 1 ) {
-                       // Keep track of the previous value
-                       prevVal = $input.val();
-
-                       // Get the key code
-                       code = charstr.charCodeAt( i );
-
-                       // Trigger event and undo if prevented
-                       event = new jQuery.Event( 'keypress', {
-                               which: code,
-                               keyCode: code,
-                               charCode: code
-                       } );
-                       $input.trigger( event );
-                       if ( !event.isDefaultPrevented() ) {
-                               $input.val( prevVal + charstr.charAt( i ) );
-                       }
+               var c, len;
+               for ( c = 0, len = charstr.length; c < len; c += 1 ) {
+                       $input
+                               .val( function ( i, val ) {
+                                       // Add character to the value
+                                       return val + charstr.charAt( c );
+                               } )
+                               .trigger( 'change' );
                }
        }
 
                        limit: null
                }, options);
 
-               QUnit.test( opt.description, function ( assert ) {
-                       var rawVal, fn, newVal;
+               QUnit.asyncTest( opt.description, opt.hasLimit ? 3 : 2, function ( assert ) {
+               setTimeout( function () {
+                       var rawVal, fn, effectiveVal;
 
                        opt.$input.appendTo( '#qunit-fixture' );
 
                        // Simulate pressing keys for each of the sample characters
                        addChars( opt.$input, opt.sample );
+
                        rawVal = opt.$input.val();
-                       fn = opt.$input.data( 'byteLimit-callback' );
-                       newVal = $.isFunction( fn ) ? fn( rawVal ) : rawVal;
+                       fn = opt.$input.data( 'byteLimit.callback' );
+                       effectiveVal = fn ? fn( rawVal ) : rawVal;
 
                        if ( opt.hasLimit ) {
-                               QUnit.expect(3);
-
                                assert.ltOrEq(
-                                       $.byteLength( newVal ),
+                                       $.byteLength( effectiveVal ),
                                        opt.limit,
                                        'Prevent keypresses after byteLimit was reached, length never exceeded the limit'
                                );
                                assert.equal( rawVal, opt.expected, 'New value matches the expected string' );
 
                        } else {
-                               QUnit.expect(2);
-                               assert.equal( newVal, opt.expected, 'New value matches the expected string' );
                                assert.equal(
-                                       $.byteLength( newVal ),
+                                       $.byteLength( effectiveVal ),
                                        $.byteLength( opt.expected ),
                                        'Unlimited scenarios are not affected, expected length reached'
                                );
+                               assert.equal( rawVal, opt.expected, 'New value matches the expected string' );
                        }
+                       QUnit.start();
+               }, 10 );
                } );
        }
 
        byteLimitTest({
                description: 'Plain text input',
-               $input: $( '<input>' )
-                       .attr( 'type', 'text' ),
+               $input: $( '<input type="text"/>' ),
                sample: simpleSample,
                hasLimit: false,
                expected: simpleSample
        });
 
        byteLimitTest({
-               description: 'Plain text input. Calling byteLimit with no parameters and no maxLength property (bug 36310)',
-               $input: $( '<input>' )
-                       .attr( 'type', 'text' )
+               description: 'Plain text input. Calling byteLimit with no parameters and no maxlength attribute (bug 36310)',
+               $input: $( '<input type="text"/>' )
                        .byteLimit(),
                sample: simpleSample,
                hasLimit: false,
 
        byteLimitTest({
                description: 'Limit using the maxlength attribute',
-               $input: $( '<input>' )
-                       .attr( 'type', 'text' )
-                       .prop( 'maxLength', '10' )
+               $input: $( '<input type="text"/>' )
+                       .attr( 'maxlength', '10' )
                        .byteLimit(),
                sample: simpleSample,
                hasLimit: true,
 
        byteLimitTest({
                description: 'Limit using a custom value',
-               $input: $( '<input>' )
-                       .attr( 'type', 'text' )
+               $input: $( '<input type="text"/>' )
                        .byteLimit( 10 ),
                sample: simpleSample,
                hasLimit: true,
 
        byteLimitTest({
                description: 'Limit using a custom value, overriding maxlength attribute',
-               $input: $( '<input>' )
-                       .attr( 'type', 'text' )
-                       .prop( 'maxLength', '10' )
+               $input: $( '<input type="text"/>' )
+                       .attr( 'maxlength', '10' )
                        .byteLimit( 15 ),
                sample: simpleSample,
                hasLimit: true,
 
        byteLimitTest({
                description: 'Limit using a custom value (multibyte)',
-               $input: $( '<input>' )
-                       .attr( 'type', 'text' )
+               $input: $( '<input type="text"/>' )
                        .byteLimit( 14 ),
                sample: mbSample,
                hasLimit: true,
 
        byteLimitTest({
                description: 'Limit using a custom value (multibyte) overlapping a byte',
-               $input: $( '<input>' )
-                       .attr( 'type', 'text' )
+               $input: $( '<input type="text"/>' )
                        .byteLimit( 12 ),
                sample: mbSample,
                hasLimit: true,
 
        byteLimitTest({
                description: 'Pass the limit and a callback as input filter',
-               $input: $( '<input>' )
-                       .attr( 'type', 'text' )
+               $input: $( '<input type="text"/>' )
                        .byteLimit( 6, function ( val ) {
                                // Invalid title
                                if ( val === '' ) {
 
        byteLimitTest({
                description: 'Limit using the maxlength attribute and pass a callback as input filter',
-               $input: $( '<input>' )
-                       .attr( 'type', 'text' )
-                       .prop( 'maxLength', '6' )
+               $input: $( '<input type="text"/>' )
+                       .attr( 'maxlength', '6' )
                        .byteLimit( function ( val ) {
                                // Invalid title
                                if ( val === '' ) {
                expected: 'User:Sample'
        });
 
-       QUnit.test( 'Confirm properties and attributes set', 5, function ( assert ) {
+       QUnit.test( 'Confirm properties and attributes set', 4, function ( assert ) {
                var $el, $elA, $elB;
 
-               $el = $( '<input>' )
-                       .attr( 'type', 'text' )
-                       .prop( 'maxLength', '7' )
+               $el = $( '<input type="text"/>' )
+                       .attr( 'maxlength', '7' )
                        .appendTo( '#qunit-fixture' )
                        .byteLimit();
 
-               assert.strictEqual( $el.prop( 'maxLength' ), 7, 'Pre-set maxLength property unchanged' );
+               assert.strictEqual( $el.attr( 'maxlength' ), '7', 'maxlength attribute unchanged for simple limit' );
 
-               $el = $( '<input>' )
-                       .attr( 'type', 'text' )
-                       .prop( 'maxLength', '7' )
+               $el = $( '<input type="text"/>' )
+                       .attr( 'maxlength', '7' )
                        .appendTo( '#qunit-fixture' )
                        .byteLimit( 12 );
 
-               assert.strictEqual( $el.prop( 'maxLength' ), 12, 'maxLength property updated if value was passed to $.fn.byteLimit' );
+               assert.strictEqual( $el.attr( 'maxlength' ), '12', 'maxlength attribute updated for custom limit' );
 
-               $elA = $( '<input>' )
+               $el = $( '<input type="text"/>' )
+                       .attr( 'maxlength', '7' )
+                       .appendTo( '#qunit-fixture' )
+                       .byteLimit( 12, function ( val ) {
+                               return val;
+                       } );
+
+               assert.strictEqual( $el.attr( 'maxlength' ), undefined, 'maxlength attribute removed for limit with callback' );
+
+               $elA = $( '<input type="text"/>' )
                        .addClass( 'mw-test-byteLimit-foo' )
-                       .attr( 'type', 'text' )
-                       .prop( 'maxLength', '7' )
+                       .attr( 'maxlength', '7' )
                        .appendTo( '#qunit-fixture' );
 
-               $elB = $( '<input>' )
+               $elB = $( '<input type="text"/>' )
                        .addClass( 'mw-test-byteLimit-foo' )
-                       .attr( 'type', 'text' )
-                       .prop( 'maxLength', '12' )
+                       .attr( 'maxlength', '12' )
                        .appendTo( '#qunit-fixture' );
 
                $el = $( '.mw-test-byteLimit-foo' );
                assert.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.
-               assert.strictEqual( $elA.prop( 'maxLength' ), 7, 'maxLength was not incorrectly set on #1 when calling byteLimit on multiple elements (bug 35294)' );
-               assert.strictEqual( $elB.prop( 'maxLength' ), 12, 'maxLength was not incorrectly set on #2 when calling byteLimit on multiple elements (bug 35294)' );
        });
 
-}( jQuery ) );
+}( jQuery, mediaWiki ) );