Opened 12 years ago

Closed 11 years ago

#3242 closed defect (fixed)

IE: functions registered with dojo.addOnUnload are called or not called (race condition)

Reported by: guest Owned by: James Burke
Priority: high Milestone: 1.2
Component: Core Version: 0.9
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by Sam Foster)

Dojo version: 0.9 built from current sources, revision 8875
Browser: IE6, IE7;
does not occur with FireFox? 2.0

Problem: Functions, registered with dojo.addOnUnload() are sometimes not called in IE.

My investigation showed, that there is a race condition, caused by the following fragment of hostenv_browser.js

222 	        // IE WebControl hosted in an application can fire "beforeunload" and "unload"
223 	        // events when control visibility changes, causing Dojo to unload too soon. The
224 	        // following code fixes the problem
225 	        // Reference: http://support.microsoft.com/default.aspx?scid=kb;en-us;199155
226 	        if(dojo.isIE){
227 	                dojo._handleNodeEvent(window, "beforeunload", function(){
228 	                        dojo._unloading = true;
229 	                        window.setTimeout(function() {
230 	                                dojo._unloading = false;
231 	                        }, 0);
232 	                });
233 	        }
234 	
235 	        dojo._handleNodeEvent(window, "unload", function(){
236 	                if((!dojo.isIE)||(dojo.isIE && dojo._unloading)){
237 	                        dojo.unloaded();
238 	                }
239 	        });

The call on line 237 is skipped if the function 229-231 happens to be called too early.

This IE-specific behavior was introduced by changeset [6092].

I have a testcase, that will be attached below.

Best regards,
Konstantin Kolinko

Attachments (6)

first.html (545 bytes) - added by guest 12 years ago.
first page of the testcase
second.html (944 bytes) - added by guest 12 years ago.
second page of the testcase
onloadonUnload.html (1.1 KB) - added by Sam Foster 11 years ago.
test page that alerts for onload/onunload
PowerpointSlidewithBrowserComponent.ppt (71.5 KB) - added by Sam Foster 11 years ago.
test_unload.html (933 bytes) - added by Sam Foster 11 years ago.
test page that runs stuff via dojo.addOnLoad, and stops running via addOnUnload
3242_unloaded_20080429.patch (1.6 KB) - added by Sam Foster 11 years ago.
[PATCH] [CLA] in hostenv_browser.js, remove IE branch of onbeforeunload dojo.unloaded registration

Download all attachments as: .zip

Change History (15)

Changed 12 years ago by guest

Attachment: first.html added

first page of the testcase

Changed 12 years ago by guest

Attachment: second.html added

second page of the testcase

comment:1 Changed 12 years ago by guest

Steps to reproduce:

  1. Deploy first.html, second.html on a web server.
  2. Launch browser, and open first.html
  3. Press the "Start" link on the page.
  4. The second.html is opened in a separate window and is reloading itself once a second, incrementing a number in its url. Every time when load or unload of second.html happens, a message is printed in Firebug console on the first page.
  1. When done, close second.html then first.html.

Expected behaviour (FF):
The following messages are printed:

1) loaded. Loaded count: 1
1) unloaded. Loaded count: 0
2) loaded. Loaded count: 1
2) unloaded. Loaded count: 0
3) loaded. Loaded count: 1
3) unloaded. Loaded count: 0
4) loaded. Loaded count: 1
4) unloaded. Loaded count: 0
...

Actual behaviour (IE):

1) loaded. Loaded count: 1
1) unloaded. Loaded count: 0
2) loaded. Loaded count: 1
3) loaded. Loaded count: 2
3) unloaded. Loaded count: 1
4) loaded. Loaded count: 2
5) loaded. Loaded count: 3
5) unloaded. Loaded count: 2
6) loaded. Loaded count: 3
7) loaded. Loaded count: 4
8) loaded. Loaded count: 5

Note, that "unloaded" is sometimes called, but in overall the loaded vs. unloaded counter gradually increases.

Note:
I encountered this behavior when trying to test for memory leaks with sIEve. It has a mode when the loaded page is repeatedly reloaded (Auto-Refresh).

Konstantin Kolinko

comment:2 Changed 12 years ago by guest

The MSDN page 199155, referenced in hostenv_browser.js sources behind [6092], states that the IE bug is in IE 4.0, 4.01, 5.0.

Is it still an issue with IE 6, IE 7 versions? Is that workaround code still needed?

comment:3 Changed 12 years ago by Adam Peller

Owner: changed from anonymous to alex

comment:4 Changed 12 years ago by dylan

Milestone: 1.2

Changed 11 years ago by Sam Foster

Attachment: onloadonUnload.html added

test page that alerts for onload/onunload

comment:5 Changed 11 years ago by Sam Foster

Description: modified (diff)

To investigate the original issue here (onunload firing when the IE browser is an embedded component and has its Visibility toggled) I've uploaded a test html page with alerts that show onload/onunload and their dojo equivalents, and a powerpoint (Office 2003) that has an embedded browser component that reproduces the issue. To use the PPT: view the slideshow (in Windows only I'd guess), make sure macros are enabled, enter the url of your test page (e.g drop onloadonUnload.html on your local server) into the text box, and click the "Load Page" button. Then click the "Hide browser" button to set Visibility false on that control. "Show browser" sets it back to true.

Changed 11 years ago by Sam Foster

Changed 11 years ago by Sam Foster

Attachment: test_unload.html added

test page that runs stuff via dojo.addOnLoad, and stops running via addOnUnload

comment:6 Changed 11 years ago by Sam Foster

Ok, the powerpoint tester is now SelfSigned? (trust me and enable macros when you open it). the test_unload.html page has an addOnLoad function that starts a setInterval. It also registers a function via addOnUnload to stop that setInterval. So, if the unload functions are called, you'll see the log output under "Status:" stop. This is exactly what happens if you load that test_unload.html page into the PPT tester, and click "Hide browser", then "Show browser". As promised by MS, the window.unload event fires, the dojo unload functions are called, but the IE branch that's supposed to catch and avoid a scenario where dojo runs the unload functions but the page isnt actually unloaded - doesnt work.

If other people can confirm this is a valid test, then it would seem the troublesome setTimeout workaround in hostenv_browser.js is needlessly causing problems as the problem its supposed to be fixing remains. If the original ticket is still around, it should be re-opened regardless, and unless a better solution is imminent, we could close this ticket by simply removing that IE code branch.

Changed 11 years ago by Sam Foster

[PATCH] [CLA] in hostenv_browser.js, remove IE branch of onbeforeunload dojo.unloaded registration

comment:7 Changed 11 years ago by Sam Foster

Owner: changed from alex to Sam Foster
Status: newassigned

3242_unloaded_20080429.patch is a proposal to "fix" this issue. actually its more a rollback. It just removes the registration of dojo.unloaded with the onbeforeunload event from the IE-only code branch, and removes the not-working workaround for the embedded IE scenario

comment:8 Changed 11 years ago by James Burke

Owner: changed from Sam Foster to James Burke
Status: assignednew

I will take this ticket and apply the patch to trunk. Holler if you do not like it.

comment:9 Changed 11 years ago by James Burke

Resolution: fixed
Status: newclosed

(In [13555]) Fixes #3242. Applies patch from Sam Foster. Removing weird IE WebControl? logic that caused some race conditions with unload firing in IE. Updated test file to reflect test results.

Note: See TracTickets for help on using tickets.