Opened 13 years ago
Closed 12 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 )
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)
Change History (15)
Changed 13 years ago by
Attachment: | first.html added |
---|
comment:1 Changed 13 years ago by
Steps to reproduce:
- Deploy first.html, second.html on a web server.
- Launch browser, and open first.html
- Press the "Start" link on the page.
- 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.
- 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 13 years ago by
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
Owner: | changed from anonymous to alex |
---|
comment:4 Changed 12 years ago by
Milestone: | → 1.2 |
---|
Changed 12 years ago by
Attachment: | onloadonUnload.html added |
---|
test page that alerts for onload/onunload
comment:5 Changed 12 years ago by
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 12 years ago by
Attachment: | PowerpointSlidewithBrowserComponent.ppt added |
---|
Changed 12 years ago by
Attachment: | test_unload.html added |
---|
test page that runs stuff via dojo.addOnLoad, and stops running via addOnUnload
comment:6 Changed 12 years ago by
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 12 years ago by
Attachment: | 3242_unloaded_20080429.patch added |
---|
[PATCH] [CLA] in hostenv_browser.js, remove IE branch of onbeforeunload dojo.unloaded registration
comment:7 Changed 12 years ago by
Owner: | changed from alex to Sam Foster |
---|---|
Status: | new → assigned |
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 12 years ago by
Owner: | changed from Sam Foster to James Burke |
---|---|
Status: | assigned → new |
I will take this ticket and apply the patch to trunk. Holler if you do not like it.
comment:9 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
(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.
first page of the testcase