From: Krinkle <krinkle@users.mediawiki.org>
Date: Sun, 18 Mar 2012 22:13:55 +0000 (+0000)
Subject: [jquery.byteLimit] Set vars in return this.each loop
X-Git-Tag: 1.31.0-rc.0~24210
X-Git-Url: https://git.cyclocoop.org/%27.WWW_URL.%27admin/?a=commitdiff_plain;h=c2f213a4045153cc703d3b6dd3dfdca2f64c4a9c;p=lhc%2Fweb%2Fwiklou.git

[jquery.byteLimit] Set vars in return this.each loop
* 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.
---

diff --git a/RELEASE-NOTES-1.19 b/RELEASE-NOTES-1.19
index 3f3deca342..a5d58c81f7 100644
--- a/RELEASE-NOTES-1.19
+++ b/RELEASE-NOTES-1.19
@@ -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
diff --git a/resources/jquery/jquery.byteLimit.js b/resources/jquery/jquery.byteLimit.js
index 10411924c9..3c0d7e5738 100644
--- a/resources/jquery/jquery.byteLimit.js
+++ b/resources/jquery/jquery.byteLimit.js
@@ -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,
@@ -16,12 +17,12 @@
 	 * 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 ) ) {
@@ -29,59 +30,67 @@
 			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 ) );
diff --git a/tests/qunit/suites/resources/jquery/jquery.byteLimit.test.js b/tests/qunit/suites/resources/jquery/jquery.byteLimit.test.js
index 69f97883a5..21effdb6d1 100644
--- a/tests/qunit/suites/resources/jquery/jquery.byteLimit.test.js
+++ b/tests/qunit/suites/resources/jquery/jquery.byteLimit.test.js
@@ -202,4 +202,51 @@
 		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 ) );