PDA

View Full Version : [FIXED][3.0rc1.1] Cannot remove event listeners from Elements



cnelissen
8 May 2009, 9:29 AM
Seems there is a problem with element.removeListener() and element.removeAllListeners(). The following code works fine in Ext 2.2, but does not work with Ext3.0 RC1.1, or Ext Core 3.0. I have tried including ext-core.js as a standalone, including adapters/ext/ext-base.js along with ext-core.js, and adapters/ext/ext-base.js with ext-all.js. None of them produce the expected results...



function checkForm(form) {
// Define CSS classes
var errorClass = 'error-field';
var errorMsgClass = 'error-field-message';

// Define Error Messages
var requiredMsg = 'Required Field';
var invalidEmailMsg = 'Must be a Valid Email Address';

// Boolean error
var error = false;

// Remove any existing error classes and error messages
var errorFields = Ext.DomQuery.select('.' + errorClass, form);
for (var i = 0; i < errorFields.length; i++) {
var field = Ext.get(errorFields[i]);
field.removeClass(errorClass);
field.removeAllListeners(); // <--- Does not remove event listeners!
}

// Process required fields
var requiredFields = Ext.DomQuery.select('input.required, select.required, textarea.required', form);
for (var i = 0; i < requiredFields.length; i++) {
var field = Ext.get(requiredFields[i]);
if (!field.hasClass(errorClass)) {
if (field.getValue() == '') {
// Add the error class
field.addClass(errorClass);
field.addListener('focus', addErrorMsg, this, {errorMsg: requiredMsg});
field.addListener('blur', removeErrorMsg);
error = true;
}
}
}

// Process email fields
var emailFields = Ext.DomQuery.select('input.email-address, select.email-address, textarea.email-address', form);
for (var i = 0; i < emailFields.length; i++) {
var field = Ext.get(emailFields[i]);
if (!field.hasClass(errorClass)) {
if (!validEmailAddress(field.getValue())) {
// Add the error class
field.addClass(errorClass);
field.addListener('focus', addErrorMsg, this, {errorMsg: invalidEmailMsg});
field.addListener('blur', removeErrorMsg);
error = true;
}
}
}

// If there were any errors, return false
if (error == true) {
return false;
} else {
return true;
}

// Function to add an error message to a form field
function addErrorMsg(e, t, o) {
// Create an error message
var field = Ext.fly(t);
var el = Ext.get(Ext.DomHelper.insertAfter(t, {id: field.dom.id + '-msg', tag: 'div', cls: errorMsgClass, html: o.errorMsg}));
el.setLocation(field.getRight() + 10, field.getTop());
}

// Function to remove an error message from a form field
function removeErrorMsg(e, t, o) {
Ext.fly(t.id + '-msg').remove();
}

// Function to check if email is valid
function validEmailAddress(str) {
if (str.match(/^[\w-]+(\.[\w-]+)*@([\w-]+\.)+[a-zA-Z]{2,7}$/)) {
return true;
} else {
return false;
}
}
}


This function would validate a form similar to this:



<form id="form" action="/some/form/action.php" method="post" onsubmit="return checkForm(this);">
<p><strong>First Name</strong><br />
<input type="text" class="required" id="first_name" name="first_name" value="" /></p>

<p><strong>Last Name</strong><br />
<input type="text" id="last_name" name="last_name" value="" /></p>

<p><strong>Address</strong><br />
<input type="text" id="address" name="address" value="" /></p>

<p><strong>Phone</strong><br />
<input type="text" class="required" id="phone_number" name="phone_number" value="" /></p>

<p><strong>Email</strong><br />
<input type="text" class="required email-address" id="email_address" name="email_address" value="" /></p>

<p><input type="submit" value="&rsaquo; Submit" /></p>
</form>


Also to clarify, what exactly is the difference between the ext-core.js that is included with Ext3.0RC1.1 and the ext-core.js that ships with Ext Core 3.0? Aren't they supposed to be the same thing?

mystix
8 May 2009, 11:25 AM
hunt for evantName in ext-base.js and replace that with eventName (there should be only 1 occurrence of this typo).

i believe that should do the trick.

bug was spotted by @hendricd in the following thread:
http://extjs.com/forum/showthread.php?p=327225#post327225

cnelissen
8 May 2009, 12:27 PM
I found and fixed the typo in the following files:

Ext 3.0 RC1.1
/ext-core.js
/ext-core-debug.js
/adapter/ext/ext-base.js

Ext Core 3.0 Beta 1
/ext-core.js

However I am still having the same problem when testing using any one of those... Same code runs fine in 2.2.

aconran
11 May 2009, 10:21 PM
Clint -

Could you create a simple testcase that we could attempt to regenerate the same issue in SVN?

mystix
12 May 2009, 1:51 AM
simple test cases (both fail to remove document.body's click listener):

removeListener() failure:


Ext.onReady(function() {
Ext.getBody().on({
click: console.log
});

Ext.getBody().un('click', console.log);
});



removeAllListeners() failure:


Ext.onReady(function() {
Ext.getBody().on({
click: console.log
});

Ext.getBody().removeAllListeners();
});

tested against official 3.0rc1.1 download and latest 3.x SVN revision.

aconran
12 May 2009, 7:10 AM
Thanks for prepping the testcases for us Marc.

evant
12 May 2009, 6:02 PM
Was only able to confirm the removeAllListeners issue, removing specific listeners seems to be working fine.

Fixed the removeAllListeners issue in SVN.

mystix
12 May 2009, 6:39 PM
tested with the latest build from SVN, and confirmed that removeAllListeners() now works as expected.

removeListener() (i.e. un()), however, still fails to work in the following test case:


<html>
<head>
<title>removeListener() / un() failure</title>

<link rel="stylesheet" href="../../resources/css/ext-all.css" />
<style>
body { height:100%;width:100%; }
</style>

<script src="../../adapter/ext/ext-base.js"></script>
<script src="../../ext-all-debug.js"></script>
<script>
Ext.onReady(function() {
Ext.getBody().on({
click: console.log
});

Ext.getBody().un('click', console.log);
});
</script>
</head>

<body scroll="no">
</body>

</html>

evant
12 May 2009, 6:47 PM
That works for me, make sure you've got the latest version of core:

1) Checkout ext-core
2) Checkout ext-3.0
3) Build

mystix
12 May 2009, 7:00 PM
checked out all the latest files.

ext-core-debug works also fails:


<html>
<head>

<style>
body { height:100%;width:100%; }
</style>

<script src="../../ext-core-debug.js"></script>

<script>
Ext.onReady(function() {
Ext.getBody().on({
click: console.log
});

Ext.getBody().un('click', console.log);
})
</script>
</head>

<body scroll="no">
</body>

</html>

but ext-base + ext-all-debug fails.


the reason why i think you're seeing it work is because the <body>'s too short
-- you'll need to include the following style:


<style>
body { height:100%;width:100%; }
</style>

mystix
12 May 2009, 7:19 PM
cleared the cache, updated from both svn branches, rebuilt the source, tried again with ext-core and ext-base + ext-all-debug, and still no dice.

am i missing anything else?

evant
12 May 2009, 7:47 PM
You're right Marc, I needed the 100%. I'll look into it, thanks.

mystix
12 May 2009, 8:00 PM
great. thought i / my machine was going bonkers.

anyways, dug into it before you replied and came up with the following fix:


removeListener : function(element, eventName, fn, scope){
var el = Ext.getDom(element),
id = Ext.id(el),
wrap;

Ext.each((elHash[id] || {})[eventName], function (v,i,a) {
// if (Ext.isArray(v) && v[0] == fn && (!scope || v[2] == scope)) {
E.un(el, eventName, wrap = v[1]);
a.splice(i,1);
return false;
// }
});

// jQuery workaround that should be removed from Ext Core
if(eventName == "mousewheel" && el.addEventListener && wrap){
el.removeEventListener("DOMMouseScroll", wrap, false);
}

if(eventName == "mousedown" && el == DOC && wrap){ // fix stopped mousedowns on the document
Ext.EventManager.stoppedMouseDownEvent.removeListener(wrap);
}
}

no idea why that check in red is there since Ext.EventManager.removeAll() doesn't have it.

scratch that. buggy fix removes all listeners for an event...

mystix
12 May 2009, 8:10 PM
wow, tested with the latest 2.x build and the bug i mentioned is also present there. :-?


as a side note, if i register 2 identical event handlers for the click event, i.e.


Ext.getBody().on('click', console.log);
Ext.getBody().on('click', console.log);


then doing


Ext.getBody().un('click', console.log);

only unregisters the first event handler with a matching event signature.

mystix
12 May 2009, 8:33 PM
found 2 problems with the Ext.EventManager.removeListener():

if called without a scope, it is always set to the calling Ext.Element
console.log == console.log always returns false. calling it on other "normal" functions returns the correct result

mystix
12 May 2009, 8:39 PM
ok try this fix:



removeListener : function(element, eventName, fn, scope){
var el = Ext.getDom(element),
id = Ext.id(el),
wrap;

Ext.each((elHash[id] || {})[eventName], function (v,i,a) {
if (Ext.isArray(v) &&
(v[0] == fn || (Ext.isFunction(fn) && (v[0] + '' == fn + ''))) &&
(!scope || v[2] == scope || el == Ext.getDom(scope))
) {
E.un(el, eventName, wrap = v[1]);
a.splice(i,1);
return false;
}
});

// jQuery workaround that should be removed from Ext Core
if(eventName == "mousewheel" && el.addEventListener && wrap){
el.removeEventListener("DOMMouseScroll", wrap, false);
}

if(eventName == "mousedown" && el == DOC && wrap){ // fix stopped mousedowns on the document
Ext.EventManager.stoppedMouseDownEvent.removeListener(wrap);
}
},

mystix
12 May 2009, 8:58 PM
though that fixes the issue, i'd recommend taking another look at EventManager's (private) listen(), addListener() and removeListener() methods -- i smell a fish.

evant
12 May 2009, 9:54 PM
Hm, I still can't reproduce this:



Ext.onReady(function(){
var fn = function(){
console.log('a');
Ext.getBody().un('click', fn, /*x*/);
};
var x = {};
Ext.getBody().on('click', fn, /*x*/);
});


Either with or without scope, it still unbinds.

mystix
12 May 2009, 10:47 PM
Odd... So it works for all vanilla functions except console.log?
(i'm referring to the specific test case I posted above)

Condor
12 May 2009, 10:54 PM
Evidently console.log doesn't return a unique method instance, because:

This fails:

Ext.getBody().on('click', console.log).un('click', console.log);
But this succeeds:

var fn = console.log;
Ext.getBody().on('click', fn).un('click', fn);

mystix
12 May 2009, 11:03 PM
no wonder this returns false:


console.log == console.log;

console.log is probably a factory function.

thanks for the heads up @condor.


@evant: i guess this case is closed then (unless i / you come up with some other failing test cases).

evant
12 May 2009, 11:32 PM
Roger that, post here if you find anything else.