Opened 8 years ago

Closed 8 years ago

#14157 closed defect (fixed)

[regression] cross frame connects don't work on IE

Reported by: bill Owned by: Kris Zyp
Priority: high Milestone: 1.7.1
Component: Events Version: 1.7.0
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by bill)

On IE (at least IE6 and IE7), the event system sets up an indirection where emiter.listeners[] contains inidices into an _dojoIEListeners_ array. When a page loads multiple copies of dojo, there are multiple _dojoIEListeners_ arrays, but for any specific node, all the inidices in emiter.listeners[] must refer to the same _dojoIEListeners_ array in order for the right callbacks to be called.

This isn't working correctly, specifically when there are two frames that load dojo, and frame #1 listens to an event on a node in frame #2. I suspect the problem happens even without multiple frames, just whenever two copies of dojo call on(node, type, ...) for the same event on the same node.

To demonstrate the problem, see the attached testcase. When run on IE6 or IE7, it ends up calling the focus handler twice. Load outer.html in IE6 or IE7 and then refresh the page to see the error (which appears as two alerts, showing the focus handler is getting run twice).

The call that confuses the event code is in dojo/robotx.js:

dojo.connect(dojo.body(), 'onunload', function(){
		dojo.global = window;
		dojo.doc = document;
		dojo.disconnect(unloadConnect);
	});

Although "dojo" refers to the dojo of the outer frame, dojo.body() actually points to the inner frame. Thus, it updates emiter.listeners as:

emiter.listeners.push(handle = (_dojoIEListeners_.push(listener) - 1));

But since it's using _dojoIEListeners_ from the outer frame, emiter.listeners ends up as [0, 1, 2, 3, 4, 5, 6, 6]. The final 6 is an index into the other _dojoIEListeners_ array.

This bug is the cause of #14073.

Attachments (3)

14157test2.patch (2.5 KB) - added by bill 8 years ago.
load outer.html in IE6, then press refresh. see alerts that "focus" handler incorrectly called twice
outer.html (580 bytes) - added by bill 8 years ago.
combined w/inner.html this reproduces the problem more simply. click on the button and both alerts should show.
inner.html (430 bytes) - added by bill 8 years ago.
updated to fix error in test case. use this w/outer.html to reproduce bug

Download all attachments as: .zip

Change History (12)

Changed 8 years ago by bill

Attachment: 14157test2.patch added

load outer.html in IE6, then press refresh. see alerts that "focus" handler incorrectly called twice

comment:1 Changed 8 years ago by bill

Description: modified (diff)

Changed 8 years ago by bill

Attachment: outer.html added

combined w/inner.html this reproduces the problem more simply. click on the button and both alerts should show.

comment:2 Changed 8 years ago by bill

Description: modified (diff)
Summary: cross frame events broken on IEcross frame connects don't work on IE

Added simpler testcase, just load outer.html in IE6 and then click the button. It should show two alerts but only shows one. Apparently this has always been broken (or at least it was broken in 1.6), so maybe it's the code in dojo/robotx.js that needs to change?

comment:3 Changed 8 years ago by bill

Description: modified (diff)
Summary: cross frame connects don't work on IE[regression] cross frame connects don't work on IE
Version: 1.7.0b11.7.0

comment:4 Changed 8 years ago by bill

Milestone: tbd1.7.1

Marking milestone=1.7.1 for possible inclusion in 1.7/ branch, although will likely be bumped to 1.7.2 if it isn't fixed in the next 24 hours.

comment:5 Changed 8 years ago by Kris Zyp

In [27321]:

Ensure that listeners are added to the correct dojoIEListener, refs #14157 !strict

comment:6 Changed 8 years ago by Kris Zyp

Do you want to check this before I commit it to the 1.7 branch?

Also note that the test seemed to be incorrect. The inner.html was attempting to connect to the node before the DOM was ready, so dojo.byId("clickme") was actually returning undefined, and never getting connected. The dojo.connect should be wrapped in a dojo.ready in inner.html to properly test.

comment:7 Changed 8 years ago by bill

Sure, thanks for the fix, I'll try it today. And you are right about the bug in my simplified test case.

Last edited 8 years ago by bill (previous) (diff)

Changed 8 years ago by bill

Attachment: inner.html added

updated to fix error in test case. use this w/outer.html to reproduce bug

comment:8 Changed 8 years ago by bill

It definitely fixes the example I attached, and IE6 seems to pass regression tests (with a few pre-existing problems). Looks good to me.

Last edited 8 years ago by bill (previous) (diff)

comment:9 Changed 8 years ago by Kris Zyp

Resolution: fixed
Status: newclosed

In [27329]:

Ensure that listeners are added to the correct dojoIEListener, fixes #14157 !strict

Note: See TracTickets for help on using tickets.