IE8: DOM 0 event handler called twice when a separate handler is attached via jQuery

[Pointy]

If a <select> element is coded with its own "onchange" attribute, and is also referenced by a jQuery expression to bind "change" to some other code, then the handler bound via the "onchange" attribute is called twice: once before the jQuery-bound handler, and once afterwards.

This happens in IE8 but not Firefox or Chrome.

Example code: ​http://gutfullofbeer.net/onchange.html

[snover]

#7000 is a duplicate of this bug.

[snover]

#6310 is a duplicate of this bug.

[snover]

​live test case

[snover]

#6109 is a duplicate of this ticket.

[adel.m.salah@…]

A possible workaround is using :

$('#file').bind($.browser.msie? 'propertychange': 'change', handler);

instead of

$('#file').change(handler);

[jitter]

#7674 is a duplicate of this ticket.

[dennisjq]

I think this issue is more serious than it first appears. If you have a live 'change' event tied to anything else on the DOM the select tag's onchange event still fires twice.

See the modified example here: ​http://jsfiddle.net/ETzwU/1/

The .live('change') call matches *nothing* on the page, yet the select tag's onchange event is still fired twice.

[Brandon]

Note: This issue occurs also in IE7 and IE6 and applies to <input type='text' /> (and probably all INPUT types).

And I agree with dennisjq that it is more serious than "low". Using live change events on input text fields in conjunction with ASP.NET textboxes that are set to AutoPostback=true attempt to perform 2 partial postbacks when the user types in a value.

[brandon@…]

I've written a more complete jsFiddle example which shows the tricky nature of the bug: ​http://jsfiddle.net/2ARLa/3/ When it comes up, just spend a bit modifying the text in the 2 textboxes then hit continue and it will show QUnit test results of:

programmatic changes on the textbox and how change events are handled vs user-interaction changes on the textbox and how change events are handled.

The bug only occurs on IE, and *only* when the user interactively changes the value. It *works correctly when the value is changed via code* so it will not show up in a normal regression test.

I spent the day on this since it was impacting my project and I've coded a fix for 1.4.2. Once I port it to 1.4.4, I'll post the fix here and explain it.

[brandon@…]

Ok,

Here's the fix for 1.4.4 and how it works:

The trick is that on IE, if the value changed due to user interaction, IE will fire the inline onchange event automatically and jQuery should *not* fire it. But if the value changed due to programming, or the change event was triggered via code, then IE will *not* fire the onchange event and jQuery should do it (as it currently does).

I never could come up with an entirely satisfactory way to determine when the value was changed by the user that would always work. I finally settled on a sort of hack:

1 - When a change event handler is registered, I also register a handler for the IE "onpropertychange" event. event.handle() has special case code that watches for "propertychange" events on the "value" property. If it sees one of these, it examines the current call stack. If the call stack is very short, then this means the propertychange event must have come from the browser in response to a user action. If the stack is not short, then the event must have been fired in response to programmer action (code setting the .value property of the input). This code sets a flag on the element indicating whether or not the element was modified by the user. This codes sits inside event.handle() instead of inside the propertychange event handler to keep the stack trace code as simple and robust to changes as possible.

2 - Inside event.trigger(), just before calling the inline onchange event handler, there is code which checks for this user edited flag. If it is set, then it will not call the onchange event handler, but will instead just clear the flag for next time.

Here is a unified diff of the required changes to jquery-1.4.4.js.

---

--- E:\dev\bpa\src\BpaWeb\script\jquery-1.4.4.js	2010-12-30 08:11:07.000000000 -0600
+++ E:\dev\bpa\src\BpaWeb\script\jquery-1.4.4-bpafixes.js	2010-12-30 08:35:51.000000000 -0600
@@ -10,13 +10,20 @@
  * http://sizzlejs.com/
  * Copyright 2010, The Dojo Foundation
  * Released under the MIT, BSD, and GPL Licenses.
  *
  * Date: Thu Nov 11 19:04:53 2010 -0500
  */
-(function( window, undefined ) {
+/* ***********************
+*
+* Includes the following fixes patched in by BPA Team:
+*
+* BPA-7595 - a bugfix to the live change event that was causing the onchange to fire 2x for the target element
+*/
+(function (window, undefined)
+{
 
 // Use the correct document accordingly with window argument (sandbox)
 var document = window.document;
 var jQuery = (function() {
 
 // Define a local copy of jQuery
@@ -2161,23 +2168,31 @@
 		if ( handle ) {
 			handle.apply( elem, data );
 		}
 
 		var parent = elem.parentNode || elem.ownerDocument;
 
-		// Trigger an inline bound script
-		try {
-			if ( !(elem && elem.nodeName && jQuery.noData[elem.nodeName.toLowerCase()]) ) {
-				if ( elem[ "on" + type ] && elem[ "on" + type ].apply( elem, data ) === false ) {
-					event.result = false;
-					event.preventDefault();
+		// ***** BEGIN BPA-7595 *****
+		// Do *not* trigger inline bound script if this is the change event and the user edited us directly.  This is because IE will have already called the onchange
+		if (elem && type == "change" && jQuery.data(elem, "_edited_by_user"))
+		{
+			jQuery.data (elem, "_edited_by_user", false);
+		} else {
+			// Trigger an inline bound script
+			try {
+				if ( !(elem && elem.nodeName && jQuery.noData[elem.nodeName.toLowerCase()]) ) {
+					if ( elem[ "on" + type ] && elem[ "on" + type ].apply( elem, data ) === false ) {
+						event.result = false;
+						event.preventDefault();
+					}
 				}
-			}
 
-		// prevent IE from throwing an error for some elements with some event types, see #3533
-		} catch (inlineError) {}
+			// prevent IE from throwing an error for some elements with some event types, see #3533
+			} catch (inlineError) {}
+		}
+		// ***** END BPA-7595 *****
 
 		if ( !event.isPropagationStopped() && parent ) {
 			jQuery.event.trigger( event, data, parent, true );
 
 		} else if ( !event.isDefaultPrevented() ) {
 			var old,
@@ -2239,13 +2254,23 @@
 		if ( typeof events === "function" ) {
 			events = events.events;
 		}
 
 		handlers = (events || {})[ event.type ];
 
-		if ( events && handlers ) {
+		// ***** BEGIN BPA-7595 *****
+		if (event.type == "propertychange" && event.originalEvent && event.originalEvent.propertyName == "value")
+		{
+			// See if we are deep in a callstack (indicates the value was changed and this event triggered programatically and thus IE will *not* be calling onchange directly)
+			// or if we are near the top of the callstack (indicates this property was changed due to user action and thus IE will be calling onchange directly)
+			jQuery.data(this, "_edited_by_user", !arguments.callee.caller || !arguments.callee.caller.caller);
+		}
+		// ***** END BPA-7595 *****
+
+		if (events && handlers)
+		{
 			// Clone the handlers to prevent manipulation
 			handlers = handlers.slice(0);
 
 			for ( var j = 0, l = handlers.length; j < l; j++ ) {
 				var handleObj = handlers[ j ];
 
@@ -2601,12 +2626,16 @@
 			return jQuery.event.trigger( e, arguments[1], elem );
 		}
 	};
 
 	jQuery.event.special.change = {
 		filters: {
+			// ***** BEGIN BPA-7595 *****
+			propertychange: function (e) { /* no-op.  Just registering this event handler causes code in jQuery.event.handle() to examine all propertychange events for this input field. */ },
+			// ***** END BPA-7595 *****
+
 			focusout: testChange, 
 
 			beforedeactivate: testChange,
 
 			click: function( e ) {
 				var elem = e.target, type = elem.type;

[brandon@…]

I've submitted the patch to GITHUB: ​https://github.com/jquery/jquery/pull/156

[nelis]

This should have a much higher priority. A select's onchange shouldn't be called twice. This way upgrading jQuery is a painful process.

Workaround: teardown this functionality for selects

var special = jQuery.event.special['change'] || {};
$('select').each(function() {
    special.teardown.call(this);
});

[nikolai.essel@…]

We have the problem in IE7-9 with jQuery 1.4.2 & 1.5 too:

A select box has a change event binded via HTML. We add an additional handler via jQuery. If the select box entry are changed by user, the HTML-EventHandler is called twice because of the jQuery "testChange.call( this, e );" click handler.

Testable at ​http://jsfiddle.net/q6yzC/ :

A counter is loaded with 0. The default handler is incrementing the counter. The dynamic handler does nothing. When changing the selectbox from "val2" to "val1", the counter is incremented twice -> counter status can be shown by clicking the button

[brandon@…]

Note the patch I pasted above wasn't quite right. But the ​pull request I linked *is* correct.

I've not yet tried to apply it to 1.5 yet but will in the next few weeks.

[anonymous]

Priority of this ticket should'nt be low.

[john]

We're discussing the patch over in the pull request.

[john]

(In #6705) I think this is actually very similar to #6593. We're working to try and fix the problem over there for the change event.

[dmethvin]

I have a fix working on my 1.7 branch, will test with this once it's landed later this month:

​http://jsfiddle.net/dmethvin/q6yzC/11/

[dmethvin]

(In #6705) Confirmed: ​http://jsfiddle.net/dmethvin/gsJyE/

This appears to be fixed in my 1.7 branch, I'll confirm once it lands.

[Dave Methvin]

Fix #6593. Don't trigger change event twice when simulating it on IE.

Changeset: 619a89d5ff6f5bcf678bdb88c3b47dea96b06ea7

[Dave Methvin]

Update "Fix #6593. Don't trigger change event twice when simulating it on IE."

This reverts commit 3d0de29d5615c1b1d74c72e6272484961a4ba243.

Changeset: 4030de9519e91fdc247af3c508a8ede277242c3f

[Dave Methvin]

Revert "Fix #6593. Don't trigger change event twice when simulating it on IE." Add inline handler monitor to delegatetest.html so we can see it fail.

This reverts commit e77686495b6e34938575c9c0fe978cb4a0be6f05.

Changeset: 6ece8f138f0c89d72fd597d95aa87304cdbe4d1d

[dmethvin]

Unfortunately this did not (yet) get fixed, each potential solution caused its own, more significant, problems. I'll note that inline handlers are generally a wontfix item, see ​http://docs.jquery.com/Won't_Fix , but I'll leave this open for a while in hopes I can find a better solution.

[Dave Methvin]

Fix #6593. Don't let onchange trigger twice for elements in IE.

This is a major revamp of the approach we use for IE change events. Instead of trying to track and simulate, we lazy-attach real change events to inputs and have only one workaround for check/radio. Somewhat more resource intensive but closes several sticky bugs. The onchange is still triggered for check/radio on blur but no double-trigger on any element occurs.

Changeset: 3bd7bed340f9077d39734ffce366ef2caeb9ce35

[mike@…]

Thanks for finally fixing this. This was perfectly fine in jquery 1.3 and broken between 1.4 and 1.6.4 - which is a VERY long time to be broken.

This sort of big is extremely hard for your average developer to track down and we tend to rely on jquery being fairly solid. It is supposed to reduce browser incompatibilites but in this case it actually created browser incompatibility due to unnecessary dicking around with events dispatch.

One of the original tenets of jquery was leaving the native objects an global namespace alone (eg not adding a foreach method to "object"). I believe this ought to also apply to being extremely careful not to screw up native events. First, do no harm.

John et al - please give these thoughts some consideration.

[mike@…]

Ps. I did not see this bug in lost of bugs fixed in 1.7 but it was a bug in 1.6.4 and was fixed in 1.7. Will this still be covered in your regression tests? Thanks.

[anonymous]

is this bug fixed?If yes tell me which version??