search

leonidas / transparency

Transparency is a semantic template engine for the browser. It maps JSON objects to DOM elements by id, class and data-bind attributes.
http://leonidas.github.com/transparency/
MIT License
969 stars 112 forks source link

Out of stack space with IE8 #78

Closed juhamust closed 11 years ago

juhamust commented 12 years ago

Not sure about the validity of this, but IE8 gives a Out of Stack space error (because of getting into recursive loop) from following:

transparency.js: 376 
    if ((_ref1 = (_base = Array.prototype).indexOf) == null) {
      _base.indexOf = function(obj) {
        return $.inArray(obj, this);
      };
    }

Where obj is string "on" and this is:

[] {
    indexOf : function(obj) {         return $.inArray(obj, this);       }
}

With IE9, FF and Chrome, the issue cannot be reproduced.

pyykkis commented 12 years ago

That sounds like a bug. Will check.

juhamust commented 12 years ago

I'll try to provide a script for reproducing the issue.

Managed to get same/similar issue only with using jQuery + jQuery UI. Thus, the root cause may also be in jQuery version (1.4.3 in this case)

pyykkis commented 12 years ago

Thanks, that would be awesome! hmm...this might actually be related to http://stackoverflow.com/questions/9039463/split-string-into-array-with-javascript-in-ie8

juhamust commented 12 years ago

The outcome depends on jQuery version:

<html>
<head>
    <script type="text/javascript" src="http://code.jquery.com/jquery-1.4.4.js"></script>
    <!--script type="text/javascript" src="http://code.jquery.com/jquery-1.7.2.js"></script-->
    <script type="text/javascript" src="transparency-master.js"></script>
    <title></title>
</head>
<body id="body">
    <div id="output"></div>
    <script type="text/javascript">
        $(document).ready(function(){
            Transparency.register($);
            var output = $.inArray(2, [1,2,3]) !== -1 ? 'works' : 'broken';
            Transparency.render(document.getElementById('body'), {output: output});
        });
    </script>
</body>
</html>

With IE8 (or IE9 in IE8 mode):

pyykkis commented 12 years ago

Wow. Thanks, that's a great catch. There's at least two ways to fix this. Either document that jQuery 1.7.2 is or newer is required, or copy the working solution from the jQuery to transparency.js.

Do you need to support jQuery 1.4.x?

If not, I'd prefer the former in order to keep things simple. Ultimately I'm planning to drop oldIE support altogether as soon as IE8 usage drops to marginal.

juhamust commented 12 years ago

I'm using Transparency with Trac 0.12, that comes with jQuery 1.4.4 - but upgrading jQuery seems doable.

Btw. I did not check the compatibility with other jQuery releases than 1.7.2. Therefore, it may work just fine with 1.5.x or 1.6.x (http://en.wikipedia.org/wiki/JQuery#Release_history). It just happens, Trac 1.0 comes with 1.7.2 :)

pyykkis commented 11 years ago

Thanks for extensive investigation. I finally fixed the issue (no more $.isArray dependency): https://github.com/leonidas/transparency/blob/v0.7.10/src/transparency.coffee#L271