Refactor out "Stop running this script?" error in jquery

Question

I've been tweaking this and that and the other in an attempt to get the "Stop running this script?" error to go away in IE7 - the browser all users are required to use at this time :P I've tried several improvement attempts; all of them have caused the script to stop working, instead of running long. I've tried using setTimeout(), with no luck. It's possible I didn't do it correctly. Can anyone suggest ways to improve this to make it more efficient (and get that dang long running script message to go away)?

Here's the code:

The HTML:

<div class="changeView" style="float:right;">Show All...</div>
<div id="accordion" style="width: 99%;">
    <% foreach (var obj in Model.Objects) { %>
        <h3><a href="#"><span class="title"><%:obj.Id%></span><span class="status" style="font-size:75%"> - <%:obj.Status%></span></a></h3>
        <div id="<%:obj.Id %>">
            <div class="loading"><img src="<%=Url.Content("~/Content/Images/ajaxLoader.gif") %>" alt="loading..." /></div>
        </div>
    <% } %>
</div>

Then we have an onclick function to start it off...

$(function () {
    $(".changeView").click(function () {
        var divText = $(this).html();
        var src = '<%=Url.Content("~/Content/Images/ajax-loader.gif")%>';

        if (divText == "Show All...") {
            $(this).html("Show Filtered...");
            $('#accordion').accordion('destroy');
            $('#accordion').empty();
            $('#accordion').addClass("loading");
            $('#accordion').append('Loading Information...<img src="' + src + '" alt="loading..." />');
            changePartialView("all");
        }
        else {
            $(this).html("Show All...");
            $('#accordion').accordion('destroy');
            $('#accordion').empty();
            $('#accordion').addClass("loading");
            $('#accordion').append('Loading Information...<img src="' + src + '" alt="loading..." />');
            changePartialView("filter");
        }
    });
});

Next the changeView function is called:

//change view and reinit accordion
function changePartialView(viewType) {
    $.ajax({
        type: "POST",
        url: "<%:Model.BaseUrl%>" + "ToggleView",
        data: "Type=<%:Model.Target%>&Id=<%:Model.Id%>&view=" + viewType,
        success: function (result) {
            $('#accordion').empty();
            $('#accordion').removeClass();
            for (var index = 0; index < result.Html.length; index++) {
                $('#accordion').append(result.Html[index]);
            }
            var $acc = $("#accordion").accordion({
                collapsible: true,
                active: false,
                autoHeight: false,
                change: function (event, ui) {
                    var index = $acc.accordion("option", "active");
                    if (index >= 0) {
                        var clickedId = ui.newHeader.find("a").find(".title").text();
                        getRequirements(clickedId);
                    }
                    else {
                        // all panels are closed
                    }
                }
            });
        },
        error: function (xhr, err) {
            alert("readyState: " + xhr.readyState + "\nstatus: " + xhr.status);
            alert("responseText: " + xhr.responseText);
            alert("Error in ajax: " + result);
        }
    });
}

Note: The result.Html returns a generic List of formatted HTML, one for each panel of the accordion. With the exception of the long running script error message, everyone works pretty sweet.

Clarification of returned value: The result.Html consists of about 200-250 instances of these strings:

"<h3><a href=\"#\"><span class=\"title\">" + obj.Id +
"</span><span class=\"status\" style=\"font-size:75%\"> - " + obj.Status + count +
"</span></a></h3><div id=\"" + obj.Id + "\"><div class=\"loading\"><img src=\"" +
Url.Content("~/Content/Images/ajaxLoader.gif") + "\" alt=\"loading...\" /></div></div>")

The first thing I'd try would be to create a new <div> element before the loop that appends the ajax results, append the results to that instead of the real DOM, and then append the <div> when finished. — Pointy
– Pointy, Commented Aug 9, 2012 at 21:57
what is result.Html? I'm wondering if you are appending a long string, character by character... — MrOBrian
– MrOBrian, Commented Aug 9, 2012 at 21:59
@MrOBrian - see the note at the end, it explains what's being returned. — morganpdx
– morganpdx, Commented Aug 9, 2012 at 22:05
The for loop in your success callback is the only place I see that the browser could get stuck. If you are returning the data as JSON, IE7 might not understand it correctly. Try doing alert(result.Html.length) and see what IE7 says as compared to other browsers. — MrOBrian
– MrOBrian, Commented Aug 9, 2012 at 22:14

evan · Accepted Answer · 2012-08-09 21:57:58Z

1

        for (var index = 0; index < result.Html.length; index++) {
            $('#accordion').append(result.Html[index]);
        }

Appending a lot of nodes one-at-a-time into the DOM is slow, one way you might be able to speed this up is to insert them all into an unattached node and then move them all at once when you're done:

        var holder = $('<div></div>');
        for (var index = 0; index < result.Html.length; index++) {
            holder.append(result.Html[index]);
        }
        $('#accordion').append(holder.children());

answered Aug 9, 2012 at 21:57

evan

4,3081 gold badge20 silver badges18 bronze badges

Sign up to request clarification or add additional context in comments.

7 Comments

morganpdx Over a year ago

I changed it as you suggested, but I still get the message. I also factored out all the accordion selectors into a single call in both the click function and the change view function (var $acc = $("#accordion");) and then used the variable for all subsequent calls. Thought it might help.

evan Over a year ago

you're not giving us a lot to work with, to be honest. That error just means something's taking a really long time. It could be anything. (You don't have $.ajax configured to be synchronous, do you?) Try profiling your script with Chrome's web developer tools. coding.smashingmagazine.com/2012/06/12/…

evan Over a year ago

another thing you could try is to just assemble the HTML on the server and then insert it with $('#accordion').html(assembledHTML)

morganpdx Over a year ago

Getting any third party tools around here is like pulling teeth. It would take a couple weeks at best to get it. Really conducive to cutting edge development :P As for $ajax being configured to be synchronous, pretty sure I haven't...

morganpdx Over a year ago

I tried that too. Figured having an array of sorts would give me places to insert a setTimeout, but I couldn't get that to work either.

|

Madd0g · Accepted Answer · 2012-08-09 22:24:07Z

0

Change the server to return data instead of lots of HTML. Use a client-side templating solution. Then, once you have an array, you can just update the display asynchronously (like with the setTimeout you mentioned)

You only have two dynamic things in that big HTML string, pretty wasteful.

Or return less items?

answered Aug 9, 2012 at 22:24

Madd0g

4,0315 gold badges41 silver badges60 bronze badges

6 Comments

morganpdx Over a year ago

Yeah, that's how I started. I was returning just the data and building the Html in the page for each panel, but I was still getting the script error.

Madd0g Over a year ago

that's where the setTimeout comes in, if you don't do it in one long-running operation, there would be no alert. Just work on a subset of the array (like 30 at a time), then use setTimeout to re-run the function.

morganpdx Over a year ago

Can you toss up an example? I tried doing that and it just ended up breaking the accordion.

Madd0g Over a year ago

I tried to make it simple - jsfiddle.net/4UHk7 (yeah, I should probably mention I don't know how the accordion works - this is just the crude logic)

morganpdx Over a year ago

Thanks, I'll try that. What exactly does the delay param do?

|

Collectives™ on Stack Overflow

Refactor out "Stop running this script?" error in jquery

2 Answers 2

7 Comments

6 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

7 Comments

6 Comments

Your Answer

Sign up or log in

Post as a guest

Related