window.scroll function freezes firefox

Question

I'm working on a page with a fixed menu that picks up after the user has scrolled a certain distance from the top, and as they scroll down the page, different links from the menu are given a class that changes the color. All of this seems to work well in Chrome and Safari, but in Firefox, the page freezes at the top. I'm wondering if it is looping through some code incessantly...essentially freezing the window.

Here is my code.

$.localScroll({
    onBefore: function() {
        $('body').data('scroll-executing', true);

    },
    onAfter: function() {
        $('body').data('scroll-executing', false);
        $(window).trigger("scroll");
    }
});

$(window).scroll(function () {
    if ($(this).scrollTop() > 259) {
        $('.nav').addClass("f-nav");
    } else {
        $('.nav').removeClass("f-nav");
    }
});

$(window).scroll(function() { 
    if ($('body').data('scroll-executing')) {
        return;
    }
    // find the a with class 'active' and remove it
    $("a").removeClass('active');
    // get the amount the window has scrolled
    var scroll = $(window).scrollTop();
    // add the 'active' class to the correct #nav based on the scroll amount

    if (scroll > 2150) {
        $("#nav_3").removeClass('active');
        $("#nav_5").attr('class', 'active');
        setHash("contact");

    } else if (scroll > 1300) {
        $("#nav_2").removeClass('active');
        $("#nav_3").attr('class', 'active');
        setHash("portfolio");

    } else if (scroll > 400) {
        $("#nav_2").attr('class', 'active');
        setHash("about");

    } else if (scroll <= 380) { //when I remove this section, the problem goes away.
        $("#nav_1").attr('class', 'active');
        setHash("home");
    }

});

I forgot to add the setHash definition. Here it is.

setHash = function(hash) {
    var scrollmem = $('body').scrollTop();
    window.location.hash = hash;
    $('html,body').scrollTop(scrollmem);
}

I also noticed that the CPU goes up to 100%, and I can't seem to figure out why.

The problem is in the third section of code beginning with else if (scroll <= 380). I figured that out by process of elimination. Can anybody see it looping or doing something that will never end...or would explain why firefox freezes at the top of the page?

I'm new to all of this...I just picked up jquery in the past few days, and I have basically been googling a lot and adapting code so that it fits what I need.

Any help will be greatly appreciate.

Answer 1

Executing too much code on scroll event is overkill, on each scroll, browsers trigger the scroll event hundred times, you can consider using a library that have methods like throttle or debounce.

http://documentcloud.github.com/underscore/#throttle

It's a very, very, bad idea to attach handlers to the window scroll event. Depending upon the browser the scroll event can fire a lot and putting code in the scroll callback will slow down any attempts to scroll the page (not a good idea). Any performance degradation in the scroll handler(s) as a result will only compound the performance of scrolling overall. Instead it's much better to use some form of a timer to check every X milliseconds OR to attach a scroll event and only run your code after a delay (or even after a given number of executions - and then a delay). http://ejohn.org/blog/learning-from-twitter/

Answer 2

Basically you are executing too much in the scroll event. As @undefined said, the browser calls this a lot more than you might think. Some tips:

When inside a function that will be called many times, have your jQuery objects already created. That way, each time the function is called, it's not recreating the same jQuery object.

var nav2 = $("#nav_2");
$(window).scroll(function() {
   ...
   nav2.removeClass('active');
   ...
});

Similarly, when called over and over again, adding and removing classes can consume a lot of your processing cycles - Especially when you are accessing a whole class of elements, like in $('.nav').addClass("f-nav");.

Instead, try to add/remove the classes only if they don't exist. Something like:

var alreadyAdded = false;
var alreadyRemoved = false;
$(window).scroll(function () {
    if ($(this).scrollTop() > 259) {
      if(!alreadyAdded){
        $('.nav').addClass("f-nav");
        alreadyAdded = true;
        alreadyRemoved = false;
      }
    } else if(!alreadyRemoved) {
        $('.nav').removeClass("f-nav");
        alreadyAdded = false;
        alreadyRemoved = true;
    }
});

All that said, it will still probably scroll slowly with all this code attached to the scroll event. Maybe there is another way you can get the same effect. If firefox is freezing, I can only imagine how slow this must be in IE.