1.9.x redundant validity check in fnOpen causes failure with row grouping example

carteriii

I'm updating to the latest version and have found that fnOpen contains new code which didn't exist in version 1.8.x and now causes failures when combined with the row grouping example (http://datatables.net/release-datatables/examples/advanced_init/row_grouping.html).

The new code in fnOpen checks that the given row is in the table *AS IT WAS ORIGINALLY CREATED* and returns from the function if it is not.[code]/* Check that the row given is in the table */
var nTableRows = _fnGetTrNodes( oSettings );
if ( $.inArray(nTr, nTableRows) === -1 )
{
return;
}[/code]
This check fails because a new grouping row, added directly with jquery and not with fnAddData, is not included in DataTables' collection of table rows. This was not a problem in earlier versions of DataTables when fnOpen did not include the extra test.

I should point out that the current and previous versions of DataTables still include a test further down in the function which is similar but uses a jquery selector to make sure the row exists in the table, so that test includes any new grouping rows which were added to the table. I believe the proper thing to do may be to only use the test further below, even if that means copying it to execute at the top of the function.[code]/* If the nTr isn't on the page at the moment - then we don't insert at the moment */
var nTrs = $('tr', oSettings.nTBody);
if ( $.inArray(nTr, nTrs) != -1 )
{
$(nNewRow).insertAfter(nTr);
}[/code]

Note that the row grouping example adds a row using plain jquery with the function insertBefore(). Consequently this additional row is not registered with DataTables and does not exist in the master list of table rows returned by _fnGetTrNodes( oSettings ). This means that fnOpen will not work on this newly added row due to the new check in DataTables.fnOpen().

This can be easily confirmed by combining the row grouping example mentioned above with the row details example (http://datatables.net/release-datatables/examples/api/row_details.html). The call to fnOpen fails when clicking on the image on the row created by the grouping.

As a workaround, I was looking at fnAddData to add the grouping row, but that does not provide the formatting flexibility of the grouping row.

How can I get back to the situation which worked just fine in earlier versions of DataTables? Might I be correct that a change is in order, moving up the code at the bottom of fnOpen which looks at all the rows in the table? Why was the extra test added at the top, and was it intentionally different than the test further below?

allan

It sounds like the change i made for 1.9 which introduces this behaviour is doing exactly what was intended with the change (possibly not what you want to hear!). fnOpen is designed only to be called on rows which belong to the DataTables (i.e. they are present in the aoData array) - anything else will throw an error since DataTables tries to attach the newly open row to a parent that doesn't exist.

It sort of worked before, but not entirely, since that attachment didn't occur. It visually might look correct, but the underlaying data model did not support that at all - thus it was unsupported and I prevented it from occurring in 1.9.

I think the underlaying issue here is that DataTables currently doesn't have a good mechanism for grouping - the current grouping options aren't hacks as such, but they also aren't properly baked into the core data model. This needs to be addressed going forward. I think its probably unlikely to be so in v1.10 which already has a firm feature set to be delivered, but it will be something that I look into going forward.

In the mean time, if you want the old behaviour, I'd suggest just modifying DataTables and removing the check you don't want (although remember that it can result is some odd behaviour).

Allan

carteriii

Perhaps the ultimate solution is to implement a function which mimicks the insertBefore() function of jquery but is part of DataTables so that it can track the existence of the row internally. By doing this, it wouldn't matter whether the extra row is specifically for grouping or any other purpose.

fnAddData obviously exists today to add a row with data which is like all other rows, so perhaps there could be a new "fnInsertBefore" or more specifically "fnInsertTrBefore" which just takes a tr, adds it to some internally tracked collection (which could even be totally separate than the current collection), and then calls jquery's insertBefore(). Then your check in fnOpen or elsewhere would know about any new rows which aren't standard "data" rows.

That's just a suggestion. I trust you'll come up with the best approach. I'll modify DataTables for now, since it's been working ok for me for quite some time, but I'll keep my eye on it. Please just consider this for a future release. Thanks.

Tom

allan

That ultimate solution is basically what DataTables does do, but it only does it for rows which are registered with it. To do it with abstract rows would require those rows also to be registered, or there is no way for DataTables to know what to do with them.

Thanks for your thoughts - some interesting ideas. I think the real way to fix this is to get proper grouping in DataTables, which requires a fairly large change to the underlaying data model.

Allan