[jquery.byteLimit] Set vars in return this.each loop
authorKrinkle <krinkle@users.mediawiki.org>
Sun, 18 Mar 2012 22:13:55 +0000 (22:13 +0000)
committerKrinkle <krinkle@users.mediawiki.org>
Sun, 18 Mar 2012 22:13:55 +0000 (22:13 +0000)
* Set vars in return this.each loop. This is the defacto standard plugin structure
  but somehow it slipped through this one (it's a 2 line wrapper, easy to miss).
* Added unit test (which failed before this commit)
* Fixes:
-- (bug 35294) jquery.byteLimit shouldn't set element specific variables outside the "return this.each" loop.

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

index 3f3deca..a5d58c8 100644 (file)
@@ -281,6 +281,8 @@ production.
 * (bug 12262) Indents and lists are now aligned
 * (bug 34972) An error occurred while changing your watchlist settings for 
   [[Special:WhatLinksHere/Example]]
+* (bug 35294) jquery.byteLimit shouldn't set element specific variables outside
+  the "return this.each" loop.
 
 === API changes in 1.19 ===
 * Made action=edit less likely to return "unknownerror", by returning the actual error
index 1041192..3c0d7e5 100644 (file)
@@ -1,9 +1,10 @@
 /**
- * jQuery byteLimit
+ * jQuery byteLimit plugin
  *
- * @author Jan Paul Posma
+ * @author Jan Paul Posma, 2011
+ * @author Timo Tijhof, 2011-2012
  */
-( function( $ ) {
+( function ( $, undefined ) {
 
        /**
         * Enforces a byte limit to a textbox, so that UTF-8 entries are counted as well, when, for example,
         * or both. Order of arguments is important!
         *
         * @context {jQuery} Instance of jQuery for one or more input elements
-        * @param limit {Number} (optional) Limit to enforce, fallsback to maxLength-attribute,
+        * @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 fn {Function} [optional] Function to call on the input string before assessing the length
         * @return {jQuery} The context
         */
-       $.fn.byteLimit = function( limit, fn ) {
+       $.fn.byteLimit = function ( limit, fn ) {
                // If the first argument is the function,
                // set fn to the first argument's value and ignore the second argument.
                if ( $.isFunction( limit ) ) {
                        limit = undefined;
                }
 
-               // Default limit to current attribute value
-               if ( limit === undefined ) {
-                       limit = this.prop( 'maxLength' );
-               }
-
-               // 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 ) {
-                       this.prop( 'maxLength', limit );
-               } else {
-                       this.removeProp( 'maxLength' );
-               }
-
-               // Nothing passed and/or empty attribute, return without binding an event.
-               if ( limit === undefined ) {
-                       return this;
-               }
-
-               // Save function for reference
-               this.data( 'byteLimit-callback', fn );
+               // The following is specific to each element in the collection
+               return this.each( function ( i, el ) {
+                       var $el, elLimit;
 
-               // We've got something, go for it:
-               return this.keypress( function( e ) {
-                       // 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.
+                       $el = $( el );
 
-                       // 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 )
-                       {
-                               return true; //a special key (backspace, etc) so don't interfere.
+                       // Default limit to current attribute 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;
+       
+                       // 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 );
+                       } else {
+                               $el.removeProp( 'maxLength' );
                        }
-
-                       var     val = fn !== undefined ? fn( $( this ).val() ): $( this ).val(),
-                               len = $.byteLength( val ),
+       
+                       // Nothing passed and/or empty attribute, return without binding an event.
+                       if ( elLimit === undefined ) {
+                               return;
+                       }
+       
+                       // 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 ) );
-
-                       if ( ( len + charLen ) > limit ) {
-                               e.preventDefault();
-                       }
+       
+                               if ( ( len + charLen ) > elLimit ) {
+                                       e.preventDefault();
+                               }
+                       });
                });
        };
-
-} )( jQuery );
+}( jQuery ) );
index 69f9788..21effdb 100644 (file)
                expected: 'User:Sample'
        });
 
-}( jQuery ) );
\ No newline at end of file
+       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 ) );