Opened 12 years ago
Closed 12 years ago
#9093 closed defect (fixed)
[patch][ccla][IE]Two connects on the same event can cause problem if disconnecting the second one in the first function called
Reported by: | jfcunat | Owned by: | sjmiles |
---|---|---|---|
Priority: | high | Milestone: | 1.3.1 |
Component: | Events | Version: | 1.3.0 |
Keywords: | Cc: | [email protected]… | |
Blocked By: | Blocking: |
Description
On all IE, there is a problem when you do two dojo.connect on the same event. If you disconnect the second handle in the first function, it causes an error on IE
This did not occur in Dojo 1.2
A test file is provided. Just hover the indicated area and you'll se an error in IE but not in FF.
It seems related to Changeset [14604] that makes an iteration over a copy of the listener array instead of the original one.
As it solves another bug, what I propose is just to test the existance of the target function
I submitted a patch for that
Attachments (2)
Change History (8)
Changed 12 years ago by
Attachment: | test_disconnect.html added |
---|
Changed 12 years ago by
Attachment: | ieDispatcher.patch added |
---|
comment:1 Changed 12 years ago by
Component: | Core → Events |
---|---|
Owner: | changed from anonymous to sjmiles |
comment:2 Changed 12 years ago by
Milestone: | 1.3.1 → tbd |
---|
The patch solves the problem for connects to nodes on IE, but a variation of the problem exists for non-node connects.
x = { y:function(){ console.log("y"); } }; c1 = dojo.connect(x, "y", function() { dojo.disconnect(c2); console.log("c1"); }); c2 = dojo.connect(x, "y", function() { console.log("c2"); }); x.y(); x.y();
The above code produces output like so:
y c1 c2 y c1
Iow, the list of listeners is made immutable during event processing, so the removal of c2 doesn't take effect until the second call to x.y.
This effect has been known to cause confusion. And on IE (for connects to nodes) it will actually cause an exception as noted in this ticket.
So, even though the patch solves the exception problem, I'm not convinced it's a good idea to change the behavior in for this one case.
Suggest we consult the standards documents for what the behavior is suppose to be for add/removeEventLister and ensure that all connects/disconnects behave in the expected fashion. Dylan mentioned that somebody has been doing a rewrite of connect, but I haven't seen the code yet.
Fwiw, it should be possible to avoid the exception by doing the disconnect in a timeout block, although I understand the semantics are different. But right now, we just don't support a connection while executing the connected function.
I'm interested to hear more opinions on this topic.
comment:3 Changed 12 years ago by
After thinking about it, I think it's right to commit this patch after all.
There is still an inconsistancy in the way disconnects are handled between nodes and non-nodes, but at least the patch above brings IE in line with the other browsers.
After that, there will remain a bug in regular non-node connect, which is that removing an event listener during connect processing does not prevent it from being called. I call it a bug because of the standards info: https://developer.mozilla.org/En/DOM:element.removeEventListener
comment:4 Changed 12 years ago by
Milestone: | tbd → 1.3.1 |
---|
comment:6 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
fixed in 1.3 branch: [17273]
test file