javascript/jquery leaking and crashing browser

Question

Let me just preface by saying it's actually my crappy code that's leaking and crashing my browser, I just thought I better make the languages being used as clear as I could from the outset.

I have a test page here and the javascript can be found here. My problem is that when I try and drag and drop either one of the red pieces more than a few times it sucks up all browser resources and crashes the browser. I'm fairly certain the culprit is something in the following function in the Tracker() object but I'm absolutely stuck on how to debug this.

My current most likely culprit:

  function register_draggable(ob) {
      ob.config.jqId.draggable({cursor: 'move',
                              grid:[ob.config.size, ob.config.size],
                              containment: '#chessboard',
                              revert: 'invalid',
                              start: function() {
                                check_allowable_moves(ob.config.jqLocation,
                                                      ob.config.jqId,
                                                      ob);
                              },
                              stop: function() {
                                remove_allowable_moves();
                              }
                            });
  }

If anyone could take a quick look and give me any suggestions on what I should be looking for, it would be enormously appreciated.

Solution Turns out register_draggable() was the culprit. I registered a new draggable every time the location of a piece updated and all those draggables on the same object were doing nasty things. Currently I now explicity destroy the old draggable before creating a new one. Current code is

    function register_draggable(ob) {
    ob.config.jqId.draggable('destroy');
    ob.config.jqId.draggable({cursor: 'move',
                              grid:[ob.config.size, ob.config.size],
                              containment: '#chessboard',
                              revert: 'invalid',
                              start: function() {
                                check_allowable_moves(ob.config.jqLocation, 
                                                      ob.config.jqId, 
                                                      ob);
                              },
                              stop: function() {
                                remove_allowable_moves();
                              }
                            });
  }

Answer 1

It feels like some event-handlers are registered multiple times, but I'm unsure. (Reason below.)

---

That doesn't answer the question, but you absolutely should put as much code outside of $(document).ready(…) as possible, in no case put all your code in there as you do now.

I fear that your code is so ineligible that it's too much work to understand it. Could you restructure it (All those function-in-function are really horrible to read.) and add some comments.

Maybe it's just me, but I find it too hard to read and understand. It's surely going to be a disaster to maintain.

Answer 2

I don't think this is actually your problem, but it seems like your making an extra method call on register and check_ allowable_moves

return {
        register_map: function(ob) { map = ob; },
        register_piece: function(ob) {
          ob.config.tracker = this;
          register_draggable(ob);
        },
        register_draggable: function(ob) { register_draggable(ob); },
        check_allowable_moves: function(location, jqPiece, ob) { check_allowable_moves(location, jqPiece, ob); }
      }

can be shortened to

return {
        register_map: function(ob) { map = ob; },
        register_piece: function(ob) {
          ob.config.tracker = this;
          register_draggable(ob);
        },
        register_draggable: register_draggable,
        check_allowable_moves: check_allowable_moves
      }

Also

you are doing a double lookup here:

function remove_allowable_moves() {
        $('.allowable').droppable('destroy');
        $('.allowable').removeClass('allowable');
      }

should be

function remove_allowable_moves() {
        $('.allowable').droppable('destroy')
          .removeClass('allowable');
      }

Also

Whats the purpose of parsing and int into a float? Take off the parseFloat.

var x = parseInt(locs[1]);
var y = parseInt(locs[2]);
var x_min = parseFloat(x)-2;
var y_min = parseFloat(y)-2;

Finally

Why are you re-registering as draggable on drop? This could be the culprit, if your registering the draggable multiple times and only destroying it once.

jqCell.droppable({ accept: '#'+jqPiece.attr('id'),
                  drop: function(ev, ui) {
                    ob.config.jqLocation = $(this);
                    register_draggable(ob);  // why this?
                  }
                });

Other thoughts

Another thing I don't know if its going to help your performance, but it could clean up your code. the jquery selector allows commas so instead of

$('#coord-1-1').doStuff();
$('#coord-1-2').doStuff();
$('#coord-1-3').doStuff();

you could do

$('#coord-1-1, #coord-1-2, #coord-1-3').doStuff();

so your loop would only be concerned with generating the selector string and then you could run you operation on the entire set.

IMO a cleaner init

instead of

var map = new Map('content');
var piece1 = new Piece(map);
var piece2 = new Piece(map);
var tracker = new Tracker;
tracker.register_map(map);
map.render();
piece1.render('coord-4-4', '1');
piece2.render('coord-1-1', '2');
tracker.register_piece(piece1);
tracker.register_piece(piece2);

I'd like to see

$(document).ready(function() {
    $('#content').MapGame({
     pieces : { '1' : 'coord-4-4', '2' : 'coord-1-1' }
    });
});

Now implementing that is a strech from what you have now, but when building a component for jQuery I like to start with a simple init and work from their. Thats one of the big goals of jQuery is to hide all the junk from the user and just let them spin up and instance of your plugin easily.