Skip to content
This repository
Browse code

unbind document handlers to prevent memory leaks in textinput

  • Loading branch information...
commit 3a23ee166a98c68df5c23db21d5591e351dd9aba 1 parent 0325c03
Michael Ridland authored johnbender committed
1  docs/_assets/css/jqm-docs.css
@@ -27,6 +27,7 @@ body { background: #dddddd; }
27 27
 	top: .75em;
28 28
 	width: 78px;
29 29
 	z-index: 9;
  30
+	font-size:.8em;
30 31
 	-webkit-transform: rotate(45deg);
31 32
 	-moz-transform: rotate(45deg);
32 33
 	-o-transform: rotate(45deg);
63  js/widgets/forms/slider.js
@@ -152,11 +152,11 @@ $.widget( "mobile.slider", $.mobile.widget, {
152 152
 				self.refresh( val(), true );
153 153
 			});
154 154
 
155  
-		// prevent screen drag when slider activated
156  
-		$( document ).bind( "vmousemove", function( event ) {
  155
+		this._preventDocumentDrag = function( event ) {
157 156
 			// NOTE: we don't do this in refresh because we still want to
158 157
 			//       support programmatic alteration of disabled inputs
159 158
 			if ( self.dragging && !self.options.disabled ) {
  159
+
160 160
 				// self.mouseMoved must be updated before refresh() because it will be used in the control "change" event
161 161
 				self.mouseMoved = true;
162 162
 
@@ -171,7 +171,9 @@ $.widget( "mobile.slider", $.mobile.widget, {
171 171
 				self.userModified = self.beforeStart !== control[0].selectedIndex;
172 172
 				return false;
173 173
 			}
174  
-		});
  174
+		}
  175
+
  176
+		$( document ).bind( "vmousemove", this._preventDocumentDrag );
175 177
 
176 178
 		// it appears the clicking the up and down buttons in chrome on
177 179
 		// range/number inputs doesn't trigger a change until the field is
@@ -199,42 +201,36 @@ $.widget( "mobile.slider", $.mobile.widget, {
199 201
 		})
200 202
 		.bind( "vclick", false );
201 203
 
202  
-		slider.add( document )
203  
-			.bind( "vmouseup", function() {
204  
-				if ( self.dragging ) {
205  
-
206  
-					self.dragging = false;
  204
+		this._sliderMouseUp = function() {
  205
+			if ( self.dragging ) {
  206
+				self.dragging = false;
207 207
 
208  
-					if ( cType === "select") {
209  
-
210  
-						// make the handle move with a smooth transition
211  
-						handle.addClass( "ui-slider-handle-snapping" );
212  
-
213  
-						if ( self.mouseMoved ) {
214  
-
215  
-							// this is a drag, change the value only if user dragged enough
216  
-							if ( self.userModified ) {
217  
-								self.refresh( self.beforeStart === 0 ? 1 : 0 );
218  
-							}
219  
-							else {
220  
-								self.refresh( self.beforeStart );
221  
-							}
  208
+				if ( cType === "select") {
  209
+					// make the handle move with a smooth transition
  210
+					handle.addClass( "ui-slider-handle-snapping" );
222 211
 
  212
+					if ( self.mouseMoved ) {
  213
+						// this is a drag, change the value only if user dragged enough
  214
+						if ( self.userModified ) {
  215
+						    self.refresh( self.beforeStart === 0 ? 1 : 0 );
223 216
 						}
224 217
 						else {
225  
-							// this is just a click, change the value
226  
-							self.refresh( self.beforeStart === 0 ? 1 : 0 );
  218
+						    self.refresh( self.beforeStart );
227 219
 						}
228  
-
229 220
 					}
230  
-
231  
-					self.mouseMoved = false;
232  
-
233  
-					self._trigger( "stop" );
234  
-					return false;
  221
+					else {
  222
+						// this is just a click, change the value
  223
+						self.refresh( self.beforeStart === 0 ? 1 : 0 );
  224
+					}
235 225
 				}
236  
-			});
237 226
 
  227
+				self.mouseMoved = false;
  228
+				self._trigger( "stop" );
  229
+				return false;
  230
+			}
  231
+		};
  232
+
  233
+		slider.add( document ).bind( "vmouseup", this._sliderMouseUp );
238 234
 		slider.insertAfter( control );
239 235
 
240 236
 		// Only add focus class to toggle switch, sliders get it automatically from ui-btn
@@ -327,6 +323,11 @@ $.widget( "mobile.slider", $.mobile.widget, {
327 323
 			parseFloat( this.element.val() ) : this.element[0].selectedIndex;
328 324
 	},
329 325
 
  326
+	_destroy: function() {
  327
+		$( document ).unbind( "vmousemove", this._preventDocumentDrag );
  328
+		$( document ).unbind( "vmouseup", this._sliderMouseUp );
  329
+	},
  330
+
330 331
 	refresh: function( val, isfromControl, preventInputUpdate ) {
331 332
 
332 333
 		// NOTE: we don't return here because we want to support programmatic
37  js/widgets/forms/textinput.js
@@ -21,7 +21,8 @@ $.widget( "mobile.textinput", $.mobile.widget, {
21 21
 
22 22
 	_create: function() {
23 23
 
24  
-		var input = this.element,
  24
+		var self = this,
  25
+			input = this.element,
25 26
 			o = this.options,
26 27
 			theme = o.theme || $.mobile.getInheritedTheme( this.element, "c" ),
27 28
 			themeclass  = " ui-body-" + theme,
@@ -106,30 +107,31 @@ $.widget( "mobile.textinput", $.mobile.widget, {
106 107
 		if ( input.is( "textarea" ) ) {
107 108
 			var extraLineHeight = 15,
108 109
 				keyupTimeoutBuffer = 100,
109  
-				keyup = function() {
110  
-					var scrollHeight = input[ 0 ].scrollHeight,
111  
-						clientHeight = input[ 0 ].clientHeight;
112  
-
113  
-					if ( clientHeight < scrollHeight ) {
114  
-						input.height(scrollHeight + extraLineHeight);
115  
-					}
116  
-				},
117 110
 				keyupTimeout;
118 111
 
  112
+			this._keyup = function() {
  113
+				var scrollHeight = input[ 0 ].scrollHeight,
  114
+					clientHeight = input[ 0 ].clientHeight;
  115
+
  116
+				if ( clientHeight < scrollHeight ) {
  117
+					input.height(scrollHeight + extraLineHeight);
  118
+				}
  119
+			};
  120
+
119 121
 			input.keyup(function() {
120 122
 				clearTimeout( keyupTimeout );
121  
-				keyupTimeout = setTimeout( keyup, keyupTimeoutBuffer );
  123
+				keyupTimeout = setTimeout( self._keyup, keyupTimeoutBuffer );
122 124
 			});
123 125
 
124 126
 			// binding to pagechange here ensures that for pages loaded via
125 127
 			// ajax the height is recalculated without user input
126  
-			$( document ).one( "pagechange", keyup );
  128
+			$( document ).one( "pagechange", this._keyup );
127 129
 
128 130
 			// Issue 509: the browser is not providing scrollHeight properly until the styles load
129 131
 			if ( $.trim( input.val() ) ) {
130 132
 				// bind to the window load to make sure the height is calculated based on BOTH
131 133
 				// the DOM and CSS
132  
-				$( window ).load( keyup );
  134
+				$( window ).load( this._keyup );
133 135
 			}
134 136
 		}
135 137
 		if ( input.attr( "disabled" ) ) {
@@ -137,17 +139,19 @@ $.widget( "mobile.textinput", $.mobile.widget, {
137 139
 		}
138 140
 	},
139 141
 
140  
-	disable: function() {
  142
+	_destroy: function() {
  143
+		$( window ).unbind( "load", this._keyup );
  144
+	},
141 145
 
  146
+	disable: function() {
142 147
 		var $el;
143 148
 		if ( this.element.attr( "disabled", true ).is( "[type='search'], :jqmData(type='search')" ) ) {
144 149
 			$el = this.element.parent();
145 150
 		} else {
146 151
 			$el = this.element;
147  
-		} 
  152
+		}
148 153
 		$el.addClass( "ui-disabled" );
149 154
 		return this._setOption( "disabled", true );
150  
-			
151 155
 	},
152 156
 
153 157
 	enable: function() {
@@ -157,10 +161,9 @@ $.widget( "mobile.textinput", $.mobile.widget, {
157 161
 			$el = this.element.parent();
158 162
 		} else {
159 163
 			$el = this.element;
160  
-		} 
  164
+		}
161 165
 		$el.removeClass( "ui-disabled" );
162 166
 		return this._setOption( "disabled", false );
163  
-			
164 167
 	}
165 168
 });
166 169
 
5  tests/unit/slider/index.html
@@ -106,6 +106,11 @@ <h2 id="qunit-userAgent"></h2>
106 106
 		<label for="mouseup-refresh">Input slider:</label>
107 107
 		<input type="range" name="slider" id="mouseup-refresh" value="25" min="0" max="100"/>
108 108
 	</div>
  109
+
  110
+  <div data-role="fieldcontain">
  111
+    <label for="remove-events-slider">Input slider:</label>
  112
+    <input type="range" name="remove-events-slider" id="remove-events-slider" value="25" min="0" max="100"/>
  113
+  </div>
109 114
 </div>
110 115
 
111 116
 <div id="enhancetest">
1  tests/unit/slider/slider_core.js
@@ -62,7 +62,6 @@
62 62
 		expect( 1 );
63 63
 		var slider = $( "#mouseup-refresh" );
64 64
 
65  
-
66 65
 		slider.val( parseInt(slider.val(), 10) +  10 );
67 66
 		slider.change(function() {
68 67
 			ok( true, "slider changed" );
22  tests/unit/slider/slider_events.js
@@ -386,15 +386,33 @@
386 386
 			},
387 387
 
388 388
 			"slidestart", function( timeout ) {
389  
-				ok( !timeout, "slidermovestart fired" );
  389
+				ok( !timeout, "slidestart fired" );
390 390
 				slider.trigger( "mouseup" );
391 391
 			},
392 392
 
393 393
 			"slidestop", function( timeout ) {
394  
-				ok( !timeout, "slidermovestop fired" );
  394
+				ok( !timeout, "slidestop fired" );
395 395
 				start();
396 396
 			}
397 397
 		], 500);
398 398
 	});
399 399
 
  400
+	test( "slider should detach event", function() {
  401
+		var slider = $( "#remove-events-slider" ),
  402
+			doc = $( document ),
  403
+			vmouseupLength,
  404
+			vmousemoveLength;
  405
+
  406
+		function getDocumentEventsLength( name ){
  407
+			return (doc.data( 'events' )[name] || []).length;
  408
+		}
  409
+
  410
+		vmouseupLength = getDocumentEventsLength( "vmouseup" );
  411
+		vmousemoveLength = getDocumentEventsLength( "vmousemove" );
  412
+
  413
+		slider.remove();
  414
+		
  415
+		equal(getDocumentEventsLength( "vmouseup" ), (vmouseupLength - 1), 'vmouseup event was removed');
  416
+		equal(getDocumentEventsLength( "vmousemove" ), (vmousemoveLength - 1), 'vmousemove event was removed');
  417
+	});
400 418
 })(jQuery);
2  tests/unit/textinput/index.html
@@ -54,6 +54,8 @@ <h2 id="qunit-userAgent"></h2>
54 54
   </textarea>
55 55
   <a href="external.html" id="external">external</a>
56 56
 
  57
+  <textarea id="destroycorrectly">Test Value</textarea>
  58
+
57 59
   <input type="search" id="search-input">
58 60
 </div>
59 61
 </body>
12  tests/unit/textinput/textinput_core.js
@@ -4,6 +4,18 @@
4 4
 (function($){
5 5
 	module( "jquery.mobile.forms.textinput.js" );
6 6
 
  7
+	test( "input is cleaned up on destroy", function(){
  8
+		var input = $( "#destroycorrectly" ),
  9
+			win = $( window ),
  10
+			loadLen;
  11
+
  12
+		loadLen = win.data("events").load.length;
  13
+
  14
+		input.remove();
  15
+
  16
+		equal(win.data("events").load.length, (loadLen-1), "window load event was not removed");
  17
+	});
  18
+
7 19
 	test( "inputs without type specified are enhanced", function(){
8 20
 		ok( $( "#typeless-input" ).hasClass( "ui-input-text" ) );
9 21
 	});

0 notes on commit 3a23ee1

Please sign in to comment.
Something went wrong with that request. Please try again.