Issue(s) when removing DataTables from the DOM on IE8

Issue(s) when removing DataTables from the DOM on IE8

kevindentekevindente Posts: 3Questions: 0Answers: 0
edited July 2011 in Bug reports
We have a problem with DataTables running on IE8 (and IE9 in compat mode). We get errors out of jQuery's $.data cleanup code when the DataTables DOM nodes are being removed. I traced it down to the fact that DataTables uses .cloneNode() in various spots, and .cloneNode() on IE8 duplicates expando properties. In this case the expando property being duplicated is the property that jQuery is using to track data items. The result is that both DOM nodes point to the same data object. When jQuery is cleaning them up it removes the first data object from its cache (for the first node), and then tries to remove the same data object again (for the cloned node), which throws an error.

I've actually experienced this problem in 2 different scenarios:
1) With my own data objects stored on the original TABLE element (before applying .dataTable). In this case, dataTable calls cloneNode on the original table element for building the header and footer parts of the table.

2) With dataTables own event registrations. When sorting is enabled, _fnSortAttachListener attaches a click handler to the header cell. This cell ends up getting cloned with cloneNode later on, and again both copies end up pointing to the same data object. When jQuery tries to clean up the second copy, boom.

I've worked around scenario 1 by temporarily removing the data object before applying dataTable(). Ugly, but it seems to work.

Scenario 2 seems to require a change to the DataTables code to fix. I experimented with a fix by replace some of the calls to cloneNode() with jQuery's $.clone() function. AFAIK, this handles duplicating the data and event registrations correctly. Unfortunately, I don't think I can use this same trick to fix scenario 1, since AFAICT there's no non-recusive jQuery clone function (I could be wrong here).

I don't know DataTables code very well, so if someone who does is inclined to try to fix this, I would be very happy. :) Otherwise, I'll see if I can muddle my way through.

Replies

  • allanallan Posts: 63,689Questions: 1Answers: 10,500 Site admin
    Interesting one! The reason I ended up using cloneNode is because I didn't want a deep clone in most of the places I've used it (its also a bit faster since there is no library overhead). I wonder, therefore, if the correct way of handling this, is to do a check and remove for any jQuery data attributes on the cloned node, just after doing a clone.

    Are you able to link me to a simple example which shows the problem?

    Allan
  • kevindentekevindente Posts: 3Questions: 0Answers: 0
    I'm working on putting together a simple repro for this. It's turning out to be trickier than I thought - KnockoutJS may be playing into this too.

    Note that in the problematic case here (scenario 2, with the table headers), you ARE doing a deep clone. So perhaps switching to jQuery's $.clone would be a good idea there. When I tried replacing 'nTheadSize = o.nTHead.cloneNode(true);' and 'nTfootSize = o.nTFoot.cloneNode(true);' with corresponding $.clone() calls, the error went away. There's a couple other places that do deep clones too, but I haven't experimented with those so far.
  • kevindentekevindente Posts: 3Questions: 0Answers: 0
    Sorry, one correction. It's not the duplicate deleting of the cached entry by jQuery that's the problem. The issue is with jQuery UI. It installs it's own cleanData method which overrides jQuery's, and in that method it attempts to raise an event. Raising the event accesses the data item, which has already been deleted.

    The root cause remains the same - the duplicated expandos.
  • skhilkoskhilko Posts: 1Questions: 0Answers: 0
    I've faced exactly the same problem, and thought it could be fixed on jQuery side. So I've submitted a bug here http://bugs.jquery.com/ticket/10061 A simple check would be only for good.
    I'm overriding $.data function in my app for now with the proposed solution. It works fine.
This discussion has been closed.