Moodle

This is the Reason why... IE7 hangs when a page has <OBJECT> tags!

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: 1.8.1
  • Fix Version/s: 1.8.1
  • Component/s: General
  • Labels:
    None
  • Affected Branches:
    MOODLE_18_STABLE
  • Fixed Branches:
    MOODLE_18_STABLE

Description

Sorry for a new ticket... but I am not sure which of the many others to attach it to...

Basically... all the pages that have object in them... should be okay... it is the UFO.JS file that is the problem...

====================

For those that are interested....

This is the Reason why... IE7 hangs when a page has <OBJECT> tags!

It is not the pages themselves... it is a javascript file... ufo.js

I have worked it all out... just needs the programmer people to sort it out...

I decided to do a line by line trace of the code... and narrowed it down as follows...

============

I took a HTML source dump of the output from policy.php (ie) made it into
test.html

I then deleted a line at a time... and tried it... see if it crashed... or
not... put the line back... and then moved onto the next line.

When the line below was deleted... it stops crashing...

<script type="text/javascript"
src="http://www.tbshs.herts.sch.uk/moodle/lib/ufo.js"></script>

I am now going function by function in this file... to see what is happening...

============

So I decided to do a really simple page... called... o.htm

============

<html>
<head>
<script type="text/javascript" src="http://www.tbshs.herts.sch.uk/moodle/lib/ufo.js"></script>
</head>
<body>

<p>Mooo</p>

<object id="sss" data="http://www.tbshs.herts.sch.uk/tbshsdis.htm" type="text/html" height=200 width=200>
Your browser does not appear to support the <code><object></code> element,
the document that is supposed to be here is <a href="tbshsdis.htm">tbshsdis.htm</a>.
</object>

<p>Mooo</p>

</body>
</html>
============

Even this simple page crashed when you moved onto another page... so... time to look at ufo.js

============

In moodle/lib/ufo.js the 2nd to last line is...

window.attachEvent("onunload", UFO.cleanupIELeaks);

this is the killer... take that out... it does not crash...

============

So I put it back in... and then amended the function to look like this...

cleanupIELeaks: function() {
var _o = document.getElementsByTagName("object");
var _l = _o.length
alert("Msg 1...");
for (var i = 0; i < _l; i++) {
alert("Msg 2...");
_o[i].style.display = "none";
for (var x in _o[i]) {
alert("Msg 3...");
if (typeof _o[i][x] == "function") { alert("Msg 4..."); _o[i][x] = null; }
}
}
}

============

When you leave/unload my "o.htm" page... it shows...

============

Msg 1...

Msg 2...

Msg 3...
Msg 3...
Msg 3...
Msg 3...
Msg 3...
Msg 3...
Msg 3...
FOREVER AND FOREVER...

The Inner loop is going infinite... and never gets to Msg 4...

============

Issue Links

Activity

Hide
Antony O'Sullivan added a comment -

Strange to think the thing that is causing the problems is... cleanupIELeaks !!!

============

Show
Antony O'Sullivan added a comment - Strange to think the thing that is causing the problems is... cleanupIELeaks !!! ============
Hide
Martin Dougiamas added a comment -

I think this is no longer a problem in the latest code ... assigning to Petr to confirm.

Show
Martin Dougiamas added a comment - I think this is no longer a problem in the latest code ... assigning to Petr to confirm.
Hide
Petr Škoda (skodak) added a comment -

thanks!!
I will work on it more today...

Show
Petr Škoda (skodak) added a comment - thanks!! I will work on it more today...
Hide
Antony O'Sullivan added a comment -

It is good to know when ufo.js is fixed... you will be able to put Themes and Policy forms back to newer improved look...

I had to have some rest... but I am going to put some debug code into UFO... and see what the varibles are set to... to see if I can come up with a fix for you...

Show
Antony O'Sullivan added a comment - It is good to know when ufo.js is fixed... you will be able to put Themes and Policy forms back to newer improved look... I had to have some rest... but I am going to put some debug code into UFO... and see what the varibles are set to... to see if I can come up with a fix for you...
Hide
Antony O'Sullivan added a comment - - edited

Hmmm http://www.tbshs.herts.sch.uk/o.htm

I have amend ... cleanupIELeaks ... it shows that the inner loop is huge... I get it to jump out at 160 ... to stop system crashing... I am trying to work out how to get the size of this object array at the moment.

cleanupIELeaks: function() {
var _o = document.getElementsByTagName("object");
var _l = _o.length
var cnt = 0;
var totcnt = 0;
var mystr = "";
//alert("MSG 1...");
for (var i = 0; i < _l; i++) {
alert("_o_i is " + _o[i]);
var moink = _o[i];
alert("type is : " + _o[i].type);
alert("typeof is : " + typeof _o[i] );
//alert("sizeof is : " + sizeof(_o[i]) );
//alert("MSG 2...");
_o[i].style.display = "none";
cnt = 0;
for (var x in _o[i]) {
//alert("MSG 3... " + x + " ...");
mystr = mystr + " " + cnt + "] " + x;
cnt++;
totcnt++;
if (totcnt > 180) { alert("_L is " + _l); alert(moink); alert(mystr); return(totcnt); }
if (typeof _o[i][x] == "function") { alert("MSG 4..."); _o[i][x] = null; }
}
}
//
alert("We escaped the loop...");
alert("_L is " + _l);
alert(moink);
alert(mystr);
return(totcnt);
}

Show
Antony O'Sullivan added a comment - - edited Hmmm http://www.tbshs.herts.sch.uk/o.htm I have amend ... cleanupIELeaks ... it shows that the inner loop is huge... I get it to jump out at 160 ... to stop system crashing... I am trying to work out how to get the size of this object array at the moment. cleanupIELeaks: function() { var _o = document.getElementsByTagName("object"); var _l = _o.length var cnt = 0; var totcnt = 0; var mystr = ""; //alert("MSG 1..."); for (var i = 0; i < _l; i++) { alert("_o_i is " + _o[i]); var moink = _o[i]; alert("type is : " + _o[i].type); alert("typeof is : " + typeof _o[i] ); //alert("sizeof is : " + sizeof(_o[i]) ); //alert("MSG 2..."); _o[i].style.display = "none"; cnt = 0; for (var x in _o[i]) { //alert("MSG 3... " + x + " ..."); mystr = mystr + " " + cnt + "] " + x; cnt++; totcnt++; if (totcnt > 180) { alert("_L is " + _l); alert(moink); alert(mystr); return(totcnt); } if (typeof _o[i][x] == "function") { alert("MSG 4..."); _o[i][x] = null; } } } // alert("We escaped the loop..."); alert("_L is " + _l); alert(moink); alert(mystr); return(totcnt); }
Hide
Petr Škoda (skodak) added a comment -

I have committed a temporary workaround into ufo and reverted previous changes in policy and theme selector.
It uses a counter and breaks from the loop after 1000 iterations, I guess we might report it upstream.

Could you please test my workaround inside the leak workaround, I would like to close it before the release, which is expected tomorrow

thanks for the diagnosing of this trouble!!!

Show
Petr Škoda (skodak) added a comment - I have committed a temporary workaround into ufo and reverted previous changes in policy and theme selector. It uses a counter and breaks from the loop after 1000 iterations, I guess we might report it upstream. Could you please test my workaround inside the leak workaround, I would like to close it before the release, which is expected tomorrow thanks for the diagnosing of this trouble!!!
Hide
Antony O'Sullivan added a comment -

Yup new UFO works... I put in an alert... and on my system it gets upto the 1000 loops... and has to use the break to escape!

Tested it on policy and themes... they now do not crash...

So you can commit that one...

I will keep seeing if there is a better solution in the long run...

where did the original idea for "cleanupIELeaks" come from? Since it crashed both IE6 and IE7... it seems strange that it got in.

I reckon most people did not report it... though being a problem on all IE6 and IE7... because once it crashed... when they reloaded the pages... the changes had already been made to the database... so they were not presented with the policy again... so had no real reason to get annoyed by what they see as a single crash...

Show
Antony O'Sullivan added a comment - Yup new UFO works... I put in an alert... and on my system it gets upto the 1000 loops... and has to use the break to escape! Tested it on policy and themes... they now do not crash... So you can commit that one... I will keep seeing if there is a better solution in the long run... where did the original idea for "cleanupIELeaks" come from? Since it crashed both IE6 and IE7... it seems strange that it got in. I reckon most people did not report it... though being a problem on all IE6 and IE7... because once it crashed... when they reloaded the pages... the changes had already been made to the database... so they were not presented with the policy again... so had no real reason to get annoyed by what they see as a single crash...
Hide
Petr Škoda (skodak) added a comment -

thanks for testing

you can try to report it upstream, I know very little about this script myself. I guess they should be able to reproduce this quite easily, maybe nobody expected us to be embedding html files with object (for accessibility reasons) instead of iframe.

closing for now

Show
Petr Škoda (skodak) added a comment - thanks for testing you can try to report it upstream, I know very little about this script myself. I guess they should be able to reproduce this quite easily, maybe nobody expected us to be embedding html files with object (for accessibility reasons) instead of iframe. closing for now
Hide
Antony O'Sullivan added a comment -

Upstream... Can do... but... are you meaning moodle 1.9 or Microsoft...

Show
Antony O'Sullivan added a comment - Upstream... Can do... but... are you meaning moodle 1.9 or Microsoft...
Hide
Antony O'Sullivan added a comment - - edited

Do a google search for... cleanupIELeaks it appears everywhere!!!

UFO is from http://www.bobbyvandersluis.com/ufo/

I think if I write to the authors... and tell them about our use of object and the problem with UFO... they might suggest that we go back to iframe!

To see how far the loop went... I took out the break and put in window.status = j;

And the loop got upto 50,000 before I stopped it!

Show
Antony O'Sullivan added a comment - - edited Do a google search for... cleanupIELeaks it appears everywhere!!! UFO is from http://www.bobbyvandersluis.com/ufo/ I think if I write to the authors... and tell them about our use of object and the problem with UFO... they might suggest that we go back to iframe! To see how far the loop went... I took out the break and put in window.status = j; And the loop got upto 50,000 before I stopped it!
Hide
Antony O'Sullivan added a comment -

-

I am hoping to get hold of the UFO author and explain the necessary changes... some point next week.

Show
Antony O'Sullivan added a comment - - I am hoping to get hold of the UFO author and explain the necessary changes... some point next week.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: